Stirling-Tools/Stirling-PDF
1
0
Fork 0
mirror of https://github.com/Stirling-Tools/Stirling-PDF.git synced 2026-09-03 05:10:16 +03:00
Code Issues Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'origin/feature/failure-notifications' into feature/policy-decrypt-retry

Browse Source 
# Conflicts:
#	app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionId.java
#	app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureKind.java
#	app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventService.java
#	app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventView.java
#	app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationController.java
#	app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationService.java
#	app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationSource.java
#	app/proprietary/src/test/java/stirling/software/proprietary/failure/FailureKindTest.java
#	app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventServiceTest.java
#	frontend/editor/src/core/components/notifications/NotificationBell.test.tsx
#	frontend/editor/src/core/components/notifications/NotificationBell.tsx
#	frontend/editor/src/core/components/notifications/notificationActions.ts
#	frontend/editor/src/core/hooks/useNotifications.test.ts
#	frontend/editor/src/core/hooks/useNotifications.ts
#	frontend/editor/src/core/services/localFilePresence.ts
#	frontend/editor/src/core/services/notifications.ts
#	frontend/editor/src/proprietary/components/notifications/notificationActions.ts
This commit is contained in:
EthanHealy01
2026-08-20 03:04:03 +01:00
parent 6539b98c64 a75a959b22
commit 106eb464ed
47 changed files with 260 additions and 654 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionException.java
+7 -24
View File
@@ -4,20 +4,14 @@ import org.springframework.http.HttpStatus;
import lombok.Getter;
/**
* Why an action could not be dispatched. Thrown carrying a {@link Reason} rather than an HTTP
* status, so the service stays web-agnostic and only a controller ever reaches for {@link
* #statusOf}.
*/
/** Carries a {@link Reason} rather than an HTTP status, so the service stays web-agnostic. */
@Getter
public class FailureActionException extends RuntimeException {
public enum Reason {
/**
* No such event, it belongs to another team, or the caller's team did not resolve. One
* reason for all three, so the response does not vary with which it was. Unrelated to
* {@link FailureKind#UNKNOWN}, which is an unclassified failure rather than a refused
* action.
* No such event, another team's, or an unresolved team: one reason, so the answer cannot
* vary.
*/
EVENT_NOT_FOUND,
@@ -25,16 +19,11 @@ public class FailureActionException extends RuntimeException {
ACTION_NOT_RECOGNISED,
/**
* The action exists but this kind does not declare it, so an incoherent pairing (releasing
* a document whose destination is what failed) cannot be dispatched even by hand. Reachable
* as of the kinds that stopped offering {@link FailureActionId#ACKNOWLEDGE}.
* The action exists but this kind does not offer it, so it cannot be dispatched by hand.
*/
ACTION_NOT_DECLARED,
/**
* The kind offers it, but the client is what runs it (see {@link
* FailureActionId.Execution#CLIENT}), so it is refused rather than half-performed.
*/
/** Offered, but the client is what runs it, so refused rather than half-performed. */
ACTION_NOT_DISPATCHABLE,
/** The event is already closed, so no further transition is possible. */
@@ -47,20 +36,14 @@ public class FailureActionException extends RuntimeException {
this(reason, message, null);
}
/** For a refusal that follows from a lower-level failure, so its stack is not dropped. */
public FailureActionException(Reason reason, String message, Throwable cause) {
super(message, cause);
this.reason = reason;
}
/**
* The HTTP status each refusal reason answers with. Lives with the reasons it maps, so the two
* surfaces that dispatch actions, the failure queue and the notification bell, cannot drift
* apart and a client's error handling does not depend on which one it called.
*
* <p>A closed row is a conflict rather than a bad request: the request was well-formed and
* would have been valid a moment earlier. A missing row is a 404 whether it never existed,
* belongs to another team or is a colleague's, so trying does not confirm which.
* Lives with the reasons it maps, so every surface that dispatches an action answers alike. A
* closed row is a conflict, not a bad request: it was well-formed and valid a moment earlier.
*/
public static HttpStatus statusOf(Reason reason) {
return switch (reason) {
app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionId.java
+9 -24
View File
@@ -3,25 +3,16 @@ package stirling.software.proprietary.failure;
import lombok.Getter;
/**
* The actions a {@link FailureKind} may declare, and which side of the wire runs each one.
*
* <p>The dispositions are the server's: they change how the event is shown and touch nothing else,
* which is what makes them valid for every kind including {@link FailureKind#UNKNOWN}. Everything
* else is the client's, because the server holds an opaque id for the document and nothing more.
*
* <p>Client actions are still declared here rather than invented by each client, so the server
* keeps deciding what a kind offers, in what order and under what label.
* The actions a {@link FailureKind} may declare. Client actions are declared here rather than
* invented per client, so the server keeps deciding what a kind offers, in what order and labelled
* how.
*/
@Getter
public enum FailureActionId {
/**
* "Seen, and I own it." No kind offers this any more, but rows shipped before that are already
* {@code ACKNOWLEDGED} and must stay readable and closable.
*/
/** No kind offers this any more, but rows already {@code ACKNOWLEDGED} must stay closable. */
ACKNOWLEDGE(Execution.SERVER, "Acknowledge"),
/** "No remediation will happen; close it." See {@link DismissAction}. */
DISMISS(Execution.SERVER, "Dismiss"),
/** Run the failed operation again on the document the client still holds. */
@@ -30,21 +21,15 @@ public enum FailureActionId {
/** Ask the owner for the password, unlock the document in their client, then retry. */
DECRYPT_AND_RETRY(Execution.CLIENT, "Decrypt and retry"),
/** Open the document this incident is about. Only its owner's client can resolve the id. */
/** Only the owner's client can resolve the id. */
VIEW_FILE(Execution.CLIENT, "View file"),
/** Open the run behind it, for whoever reviews the team rather than owns the document. */
VIEW_IN_PROCESSOR(Execution.CLIENT, "View in processor");
/**
* Which side of the wire runs the action. The dispatch endpoint refuses a {@code CLIENT} id, so
* "the client does this one" is enforced rather than merely documented.
*/
/** Dispatch refuses a {@code CLIENT} id, so this is enforced rather than merely documented. */
public enum Execution {
/**
* Dispatchable, and {@link FailureActionRegistry} requires a {@link FailureAction} bean.
*/
/** {@link FailureActionRegistry} requires a {@link FailureAction} bean for these. */
SERVER,
/**
@@ -55,7 +40,7 @@ public enum FailureActionId {
private final Execution execution;
/** English fallback, used when the client has no translation for the action's label key. */
/** English fallback, for a client with no translation for the label key. */
private final String defaultLabel;
FailureActionId(Execution execution, String defaultLabel) {
@@ -63,7 +48,7 @@ public enum FailureActionId {
this.defaultLabel = defaultLabel;
}
/** Whether the server runs it itself, which is also whether it can be dispatched. */
/** Also whether it can be dispatched. */
public boolean runsOnServer() {
return execution == Execution.SERVER;
}
app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionRegistry.java
+3 -6
View File
@@ -17,8 +17,8 @@ import lombok.extern.slf4j.Slf4j;
* point: because kinds declare action ids as data, one could name an action nobody implements,
* which would otherwise show up as a button that 400s rather than as a failed boot.
*
* <p>Only {@link FailureActionId.Execution#SERVER} ids belong here. A client action has no bean by
* design, and a bean for one is refused outright because dispatch could never reach it.
* <p>Only {@link FailureActionId.Execution#SERVER} ids belong here: a bean for a client action is
* refused, because dispatch could never reach it.
*/
@Slf4j
@Service
@@ -49,10 +49,7 @@ public class FailureActionRegistry {
}
}
/**
* Fail fast if any kind declares a server action with no handler, naming every gap rather than
* the first, so one boot tells you everything that is missing.
*/
/** Names every gap rather than the first, so one boot tells you everything that is missing. */
@PostConstruct
void verifyEveryDeclaredActionHasAHandler() {
List<String> gaps =
app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureAudience.java
+3 -8
View File
@@ -1,20 +1,15 @@
package stirling.software.proprietary.failure;
/**
* Who an offered action is for. The read scope has already decided the caller may see the incident;
* this decides which of its actions are theirs to take.
*
* <p>The distinction is possession, not seniority: a reviewer clearing up after a colleague cannot
* supply that colleague's password or reach a document only their browser holds.
* Who an offered action is for, the read scope having already decided they may see the incident.
* The distinction is possession, not seniority: a reviewer cannot reach a document only its owner
* holds.
*/
public enum FailureAudience {
/** The person whose work failed, whose own client still holds the document. */
OWNER,
/** Anyone who triages the team's incidents, whoever hit them. */
TEAM_REVIEWER,
/** Everyone the incident is shown to at all. */
ANYONE_WHO_SEES
}
app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureKind.java
+4 -11
View File
@@ -28,14 +28,8 @@ import lombok.Getter;
* The registry of failure kinds, described as data: a stable id, i18n keys and an English fallback
* like {@code ExceptionUtils.ErrorCode}, plus the facets a review surface needs.
*
* <p>Actions are declared here but run elsewhere: a server action in a {@link FailureAction} bean
* resolved by id, a client action in the browser that holds the document. Either way a new kind
* ships as a registry entry plus copy. Two members today: {@link #UNKNOWN} gives every failed run a
* record, and kinds get promoted out of it as production shows what occurs.
*
* <p>Each offer also says who it is for and where the kind wants it, because the same incident is
* read by the person who hit it and by whoever reviews after them: only the owner can supply a
* password, only a reviewer wants the run.
* <p>A new kind ships as a registry entry plus copy. Each offer says who it is for and where the
* kind wants it, since one incident is read both by whoever hit it and by whoever reviews after.
*/
@Getter
public enum FailureKind {
@@ -114,9 +108,8 @@ public enum FailureKind {
}
/**
* One action this kind offers: who it is for, where it wants to sit, and the key to label it
* by. One ordered list rather than ids plus parallel maps of audiences, slots and label
* overrides, which could disagree with each other.
* One ordered list rather than ids plus parallel maps of audiences, slots and labels, which
* could disagree with each other.
*
* @param labelKeySuffix key under {@code portal.failures.action.}, or null for the generic
* label
app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventRepository.java
+2 -10
View File
@@ -98,16 +98,8 @@ public interface FileRunEventRepository extends JpaRepository<FileRunEventEntity
* Close the incidents about documents their owner deleted from the editor: the queue is what
* needs attention, and a document that no longer exists needs none.
*
* <p>Restricted to that owner's own rows. File ids are minted by the client, so scoping on team
* alone would let one caller close a colleague's incidents by naming ids.
*
* <p>Scoped by the ABSENCE OF A SOURCE rather than by origin, which is what tells the two id
* spaces in {@code fileId} apart. An editor report and an attended policy run both carry the id
* the client minted for its own document, so both are closable by the client that holds it; a
* source-fed run carries a one-way hash of a path or key that was never on any device, so no
* client can legitimately name it. Keying on {@code origin = TOOL} instead left a user's own
* policy failures in the queue after they deleted the very document those failures were about,
* because the run had been recorded by the processor rather than reported by the editor.
* <p>Scoped by the absence of a source rather than by origin: a source-fed run's {@code fileId}
* is a hash no client can name. Narrowed to the owner's own rows, since clients mint the ids.
*/
@Modifying(clearAutomatically = true)
@Transactional
app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventService.java
+15 -33
View File
@@ -22,8 +22,7 @@ import stirling.software.proprietary.policy.config.PolicyManagementAuthority;
* enabled so single-user deployments keep working. When the team cannot be resolved the caller
* reads nothing; see {@link #readScope()}.
*
* <p>Seeing an incident and being able to act on it are separate questions: the read scope decides
* the first, {@link #availableActions} the second.
* <p>The read scope decides who sees an incident; {@link #availableActions} decides who may act.
*/
@Slf4j
@Service
@@ -135,8 +134,8 @@ public class FileRunEventService {
// failures they caused. Someone who fixes their own problem should not have to ask a leader
// to clear the row.
//
// Audience decides what is offered, not what may be dispatched, so this read scope is the
// whole gate. A server action aimed at OWNER alone would need its own guard here.
// Audience decides what is offered, not what may be dispatched, so this scope is the whole
// gate. A server action aimed at OWNER alone would need its own guard here.
FileRunEvent event = requireVisible(eventId);
FailureActionId resolvedId = parseActionId(actionId);
@@ -148,8 +147,7 @@ public class FileRunEventService {
FailureActionException.Reason.ACTION_NOT_DECLARED,
"Kind " + event.kind().getId() + " does not offer action " + resolvedId);
}
// Declared, and still not the server's to run: without this a client could post VIEW_FILE
// and be answered as though something had happened, when nothing here has the file.
// Without this a client could post VIEW_FILE and be answered as though something happened.
if (!resolvedId.runsOnServer()) {
throw new FailureActionException(
FailureActionException.Reason.ACTION_NOT_DISPATCHABLE,
@@ -193,10 +191,7 @@ public class FileRunEventService {
FileRunEventStatus.open());
}
/**
* The event, if this caller may act on it at all. Reported as "no such event" rather than a
* refusal, so a member cannot learn that a colleague's incident exists by trying to close it.
*/
/** "No such event" rather than a refusal, so trying does not confirm a colleague's exists. */
private FileRunEvent requireVisible(String eventId) {
ReadScope scope = readScope();
if (!scope.permitted()) {
@@ -212,7 +207,6 @@ public class FileRunEventService {
FailureActionException.Reason.EVENT_NOT_FOUND, "No such event: " + eventId);
}
/** Whose incident this is, as far as the caller is concerned. See {@link Ownership}. */
public Ownership ownershipOf(FileRunEvent event) {
if (event.actor() == null) {
return Ownership.UNOWNED;
@@ -222,22 +216,18 @@ public class FileRunEventService {
}
/**
* Which of an event's declared actions this caller is offered, and which are usable right now,
* so the client never renders a button that would be refused. An action outside the caller's
* audience is dropped rather than disabled: a greyed-out "Decrypt and retry" would read as a
* permission problem when it is simply not their document.
* Offers resolved for one caller, so no client renders a button that would be refused. Outside
* their audience is dropped, not disabled: greyed out would read as a permission problem.
*/
public List<AvailableAction> availableActions(FileRunEvent event) {
Ownership ownership = ownershipOf(event);
boolean reviewsTeam = reviewsTeam();
boolean closed = event.status().terminal();
// Nobody's client is holding the document, so an owner action has nothing to act on. A
// login-disabled deployment is excluded: its rows are unowned only because it has no users,
// and its one operator owns everything they can see.
// Login disabled is excluded: its rows are unowned only for want of users, and its one
// operator owns everything they can see.
boolean unattended = enforced() && ownership == Ownership.UNOWNED;
// No document reference at all, so an owner action has nothing to name even when the owner
// is right here holding the file. Answered here rather than left to the client, which would
// otherwise report "not on this device" about a document the row never identified.
// Answered here, or the client reports "not on this device" about a document the row never
// identified in the first place.
boolean documentless = event.fileId() == null || event.fileId().isBlank();
return event.kind().getOfferedActions().stream()
.filter(offer -> offeredTo(offer.audience(), ownership, reviewsTeam))
@@ -256,10 +246,7 @@ public class FileRunEventService {
offer.id(), offer.labelKey(), offer.slot(), reason == null, reason);
}
/**
* Why an offer cannot be taken right now, or null when it can. Closed wins over everything,
* then the owner-only reasons, most specific first.
*/
/** Closed wins over everything, then the owner-only reasons, most specific first. */
private static String disabledReasonFor(
FailureAudience audience, boolean closed, boolean unattended, boolean documentless) {
if (closed) {
@@ -274,11 +261,7 @@ public class FileRunEventService {
return documentless ? DOCUMENTLESS_REASON_KEY : null;
}
/**
* Whether an offer aimed at {@code audience} is this caller's to take. An unattended incident
* has no owner, so its reviewer inherits the owner's actions; otherwise nobody is offered them
* at all.
*/
/** An unattended incident has no owner, so its reviewer inherits the owner's actions. */
private static boolean offeredTo(
FailureAudience audience, Ownership ownership, boolean reviewsTeam) {
return switch (audience) {
@@ -289,7 +272,7 @@ public class FileRunEventService {
};
}
/** Whether the caller triages the whole team's incidents, rather than only their own. */
/** Whether the caller triages the team's incidents, not only their own. Login disabled: all. */
public boolean reviewsTeam() {
return !enforced() || policyManagementAuthority.canEditPolicies();
}
@@ -365,8 +348,7 @@ public class FileRunEventService {
}
/**
* One action as offered to one caller about one event, with its availability resolved. {@code
* slot} is the kind's placement intent, carried through for the client to make the final call.
* One action offered to one caller, availability resolved. {@code slot} is placement intent.
*/
public record AvailableAction(
FailureActionId id,
app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventView.java
+2 -5
View File
@@ -61,10 +61,8 @@ public record FileRunEventView(
}
/**
* One button, as offered to this caller about this row. {@code defaultLabel} and {@code
* execution} are here for the reason {@code defaultTitle} is on the row: a client can then
* render, and route, an action it was never built with. {@code slot} is placement intent; see
* {@link FailureActionSlot}.
* {@code defaultLabel} and {@code execution} let a client render and route an action it was
* never built with. {@code slot} is placement intent; see {@link FailureActionSlot}.
*/
public record ActionView(
String id,
@@ -75,7 +73,6 @@ public record FileRunEventView(
boolean enabled,
String disabledReasonKey) {
/** Public because the notification bell projects the same resolved offers. */
public static ActionView of(FileRunEventService.AvailableAction action) {
return new ActionView(
action.id().name(),
app/proprietary/src/main/java/stirling/software/proprietary/failure/Ownership.java
+3 -7
View File
@@ -1,21 +1,17 @@
package stirling.software.proprietary.failure;
/**
* Whose incident this is, from the point of view of whoever is reading it. Derived on read and
* never persisted: the same row is {@code MINE} to the member who hit it and {@code THEIRS} to the
* leader reviewing after them, so a stored answer would be wrong for everyone but one person.
* Whose incident this is, from the reader's point of view. Derived on read, never persisted: one
* row is {@code MINE} to whoever hit it and {@code THEIRS} to the leader reviewing after them.
*/
public enum Ownership {
/** The caller's own failure: their work, and their client holding the document. */
MINE,
/** A colleague's, visible because the caller reviews the team. */
THEIRS,
/**
* Nobody's. An unattended run failed, with a folder, bucket or webhook as its only attribution,
* so there is no owner to hand the resolution to.
* An unattended run: a folder, bucket or webhook is its only attribution, so there is no owner.
*/
UNOWNED
}
app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationController.java
+4 -11
View File
@@ -20,13 +20,8 @@ import lombok.RequiredArgsConstructor;
import stirling.software.proprietary.failure.FailureActionException;
/**
* The caller's notifications. Open to any authenticated user, unlike the failure endpoints it draws
* on: each source scopes its own rows and resolves its own actions, so a member is told about their
* own failures and a leader about their team's.
*
* <p>Every action a notification offers is one the client runs on its own device, so the only write
* here is the client reporting that its own retry worked. Ids are prefixed with their source, so
* the bell is never given the producing row's id; see {@link NotificationSource}.
* Open to any authenticated user, unlike the failure endpoints it draws on: each source scopes its
* own rows. Every action runs on the client's own device, so the only write is it reporting a fix.
*/
@RestController
@RequestMapping("/api/v1/notifications")
@@ -35,7 +30,6 @@ import stirling.software.proprietary.failure.FailureActionException;
@Tag(name = "Notifications", description = "Things worth telling the caller about")
public class NotificationController {
/** One page of a bell. Enough to fill a panel; the badge counts what it is given. */
private static final int DEFAULT_LIMIT = 20;
private static final int MAX_LIMIT = 100;
@@ -81,9 +75,8 @@ public class NotificationController {
}
/**
* Wrapped rather than a bare array so paging or a total can be added without breaking clients.
* {@code viewerReviewsTeam} lets the client filter a member's list; see {@link
* NotificationService#callerReviewsTeam()}.
* Wrapped so paging or a total can be added without breaking clients. {@code viewerReviewsTeam}
* is what lets the client filter a member's list.
*/
public record NotificationsResponse(
List<NotificationView> notifications, boolean viewerReviewsTeam) {}
app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationService.java
+6 -20
View File
@@ -11,14 +11,8 @@ import stirling.software.proprietary.failure.FileRunEventService;
import stirling.software.proprietary.failure.FileRunEventView;
/**
* Assembles the caller's notifications from whatever produces them, and routes a client's report of
* its own fix back to whichever source produced the row. Derived on read rather than stored: there
* is one source today, and a table would need a write path, a retention story and a per-user read
* model before it earned itself.
*
* <p>Who sees what is decided by each source rather than here. {@link FileRunEventService} already
* scopes its reads and resolves each row's actions against that reader, so this cannot widen either
* by accident.
* Derived on read rather than stored: one source today, and a table would need a write path,
* retention and a per-user read model first. Each source scopes its own rows, so this cannot widen.
*/
@Service
@RequiredArgsConstructor
@@ -26,10 +20,7 @@ public class NotificationService {
private final FileRunEventService fileRunEvents;
/**
* The caller's notifications, newest first. Only open failures: a dismissed or resolved one has
* been dealt with and is not news.
*/
/** Newest first, and only open failures: one already dealt with is not news. */
public List<NotificationView> list(int limit) {
return fileRunEvents.list(null, null, limit).stream().map(this::fromFailure).toList();
}
@@ -43,10 +34,8 @@ public class NotificationService {
}
/**
* Record that the client's own retry of this notification worked, and return it as it now
* stands. Takes the prefixed id even though nobody pressed a button: it is still a call made
* from the bell, and the bell holds no raw failure id, so it cannot reach a failure endpoint
* even by accident.
* Record that the client's own retry of this notification worked. Takes the prefixed id, so the
* bell cannot reach a failure endpoint even by accident.
*
* @throws IllegalArgumentException if the id names no source this build has
* @throws stirling.software.proprietary.failure.FailureActionException if the source refuses,
@@ -68,9 +57,7 @@ public class NotificationService {
"Not a notification id: " + notificationId));
}
/**
* One failure as a notification, with its row id prefixed on the way out and never sent bare.
*/
/** Prefixes the row id on the way out, so it is never sent bare. */
private NotificationView fromFailure(FileRunEvent event) {
return new NotificationView(
NotificationSource.FAILURE.qualify(event.id()),
@@ -89,7 +76,6 @@ public class NotificationService {
event.occurrences(),
event.createdAt(),
event.lastSeenAt(),
// The failure surface's own offers, filtered to the ones the client itself runs.
// A disposition such as Dismiss belongs to the review surface, not the bell.
fileRunEvents.availableActions(event).stream()
.filter(action -> !action.id().runsOnServer())
app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationSource.java
+2 -10
View File
@@ -5,26 +5,18 @@ import java.util.Locale;
import java.util.Optional;
/**
* Which subsystem produced a notification. One member today; the point of the field is that a
* client already branches on it, so a second source needs no client change to be ignorable.
*
* <p>Also the routing table for a notification id: every id is prefixed with its source, so an id
* handed back to a notification endpoint says which subsystem to ask, and a client never holds the
* producing row's own id.
* Which subsystem produced a notification. Every id is prefixed with it, so a client never holds
* the producing row's own id and cannot reach that source's endpoints by accident.
*/
public enum NotificationSource {
/** A recorded run failure: see {@code stirling.software.proprietary.failure}. */
FAILURE;
private static final char SEPARATOR = ':';
/** The prefix an id from this source carries, including the separator. */
public String prefix() {
return name().toLowerCase(Locale.ROOT) + SEPARATOR;
}
/** One of this source's own row ids, as the client sees it. */
public String qualify(String sourceRowId) {
return prefix() + sourceRowId;
}
app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationView.java
+2 -29
View File
@@ -10,35 +10,8 @@ import stirling.software.proprietary.failure.FileRunEventView;
import stirling.software.proprietary.failure.Ownership;
/**
* One thing worth telling the caller about, flattened to what a bell needs to render it and act on
* it. Sources project onto this rather than exposing their own shape, so a client renders and
* triages a notification without knowing which subsystem produced it.
*
* <p>The facets read as failure vocabulary because failures are the only source so far. A later
* source projects onto the same fields or leaves them null; what a client must not do is infer the
* source from which fields are populated, which is what {@code source} is for.
*
* @param id unique across every source, so a client can key and de-duplicate on it alone. Prefixed
* with its source, and the only handle the notification endpoints accept
* @param source which subsystem produced it, for grouping and for the click-through
* @param kindId the source's own id for what happened, so a client can group or route on it
* @param origin what was running when it happened, e.g. one tool or a policy
* @param ownership whether this is the caller's own to act on, derived per read
* @param severity how loudly to show it
* @param status where the underlying row stands, so a bell can show what has already been dealt
* with
* @param titleKey i18n key for the headline, with {@code defaultTitle} as the English fallback
* @param detail the one-line body, as the source recorded it
* @param fileId opaque reference to the document, never a name. Two id spaces share it, and {@code
* sourceId} is what tells them apart; see {@code PolicyEngine#runPolicy}
* @param sourceId which folder, bucket or webhook fed the failing run, and null for an attended run
* the caller started themselves
* @param policyId which policy the failing run belonged to; null for work with no policy around it
* @param occurrences how many times this same thing has happened, at least 1
* @param createdAt when it first happened; the client orders and marks unread on this
* @param lastSeenAt when it last happened, equal to {@code createdAt} for a one-off
* @param actions the same resolved offers the failure surface renders, already filtered to this
* caller, so the bell needs no rules of its own
* A source's row flattened to what a bell renders. {@code fileId} is an opaque reference, never a
* name, and two id spaces share it: {@code sourceId} tells them apart.
*/
public record NotificationView(
String id,
app/proprietary/src/main/java/stirling/software/proprietary/policy/controller/PolicyController.java
+3 -7
View File
@@ -680,13 +680,9 @@ public class PolicyController {
}
/**
* The document reference to record against a failure of this run: the caller's own opaque id,
* but only when the run carries exactly one primary document. A recorded incident has a single
* file reference, so naming one document out of several would attribute the failure to
* whichever happened to be bound first, which is worse than naming none.
*
* <p>Counted off the resolved inputs rather than the raw multipart list, so an empty part
* cannot make a single-document run look like two.
* Only for a single-document run: an incident holds one file reference, so naming one of
* several would attribute the failure to whichever bound first. Counted off resolved inputs,
* not parts.
*/
private static String documentReferenceFor(PolicyRunFiles files, PolicyInputs inputs) {
String fileId = files.getFileId();
app/proprietary/src/main/java/stirling/software/proprietary/policy/controller/PolicyRunFiles.java
+2 -8
View File
@@ -31,14 +31,8 @@ public class PolicyRunFiles {
private List<NamedAsset> assets = new ArrayList<>();
/**
* The caller's own opaque reference to the document it is running on, recorded against any
* failure of this run so the client that filed it can resolve the row back to that document.
* Without it an attended failure names no document, and every action that needs the bytes is
* unreachable for the one person holding them.
*
* <p>Opaque by contract and never a name: the server stores it, hands it back and reads nothing
* out of it. Only honoured for a single-document run, see {@code
* PolicyController#documentReferenceFor}.
* Recorded against any failure of this run, so the client can resolve the row back to its
* document. Opaque by contract, never a name, and only honoured for a single-document run.
*/
@Schema(
description =
app/proprietary/src/main/java/stirling/software/proprietary/policy/engine/PolicyEngine.java
+2 -6
View File
@@ -151,12 +151,8 @@ public class PolicyEngine {
* As {@link #runPolicy(Policy, PolicyInputs, PolicyProgressListener)}, recording which source
* fed the run and its opaque reference to the document. The first says where an unattended
* failure came from; the second says which document, and is what lets the same document failing
* again fold into one incident.
*
* <p>An attended run (a user's upload) has no source, and that absence is the discriminator for
* the two id spaces {@code fileIdentity} carries: with no source it is the reference the client
* minted for its own document, otherwise a source's one-way hash of a path or key. Either way
* this engine only carries it, and reads nothing out of it.
* again fold into one incident. With no source {@code fileIdentity} is the client's own
* reference, with one it is that source's hash; this engine only carries it either way.
*/
public PolicyRunHandle runPolicy(
Policy policy,
app/proprietary/src/main/java/stirling/software/proprietary/policy/engine/PolicyRunner.java
+2 -7
View File
@@ -121,13 +121,8 @@ public class PolicyRunner {
* The supplied documents are still counted against the virtual {@link EditorSource}, scoped to
* the policy's team, so the Sources overview reports the whole team's editor throughput.
*
* <p>Attended: no source fed this run, which is what later tells its recorded failures apart
* from an unattended sweep's.
*
* @param documentReference the caller's own opaque reference to the single document it is
* running on, or null when it supplied none (or supplied several). Passed through
* untouched, and lands where an unattended run's {@link ResolvedInput#forFile} identity
* does; see {@code PolicyEngine#runPolicy} for how the two are told apart.
* @param documentReference the caller's own opaque reference to the single document it runs on,
* or null when it supplied none or several. Passed through untouched.
*/
public PolicyRunHandle runWith(
Policy policy,
app/proprietary/src/test/java/stirling/software/proprietary/failure/CheckConstrainedEnumsTest.java
+4 -9
View File
@@ -9,11 +9,8 @@ import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
/**
* Pins the five enums that {@code file_run_events} stores as strings behind CHECK constraints. The
* shipped migration spells out the permitted values, so adding one to {@code status}, {@code
* origin}, {@code stage}, {@code severity} or {@code scope} is a schema change dressed up as a Java
* change: it compiles, then fails against a real database on the row that most needed recording.
* Reordering is free, since values are persisted by name.
* Pins the five enums {@code file_run_events} stores behind CHECK constraints: adding a value is a
* schema change dressed as a Java one, compiling here and failing against a real database.
*/
class CheckConstrainedEnumsTest {
@@ -36,8 +33,7 @@ class CheckConstrainedEnumsTest {
@Test
@DisplayName("the facets added since are derived, not stored")
void nothingAddedToTheModelReachedTheTable() throws Exception {
// Audience, slot, execution and ownership are resolved per reader, so a column for any of
// them would hold the wrong answer for everybody but one person.
// Resolved per reader, so a column would hold the wrong answer for all but one person.
List<Class<?>> persisted =
Arrays.stream(FileRunEventEntity.class.getDeclaredFields())
.filter(field -> !field.isSynthetic())
@@ -51,8 +47,7 @@ class CheckConstrainedEnumsTest {
FailureActionId.class,
FailureActionId.Execution.class,
Ownership.class);
// kind_id stays a plain varchar with no CHECK, which is what lets a new kind ship without a
// migration while the five columns above cannot.
// A plain varchar with no CHECK, which is what lets a new kind ship without a migration.
assertThat(FileRunEventEntity.class.getDeclaredField("kindId").getType())
.isEqualTo(String.class);
}
app/proprietary/src/test/java/stirling/software/proprietary/failure/FailureKindTest.java
+6 -10
View File
@@ -35,10 +35,7 @@ import stirling.software.common.util.ExceptionUtils;
*/
class FailureKindTest {
/**
* One expected offer in full, rather than four separate extracting() assertions, so a
* declaration that pairs the right action with the wrong audience cannot pass.
*/
/** In full, so a declaration pairing the right action with the wrong audience cannot pass. */
private static FailureKind.OfferedAction offered(
FailureActionId id,
FailureAudience audience,
@@ -123,8 +120,8 @@ class FailureKindTest {
@ParameterizedTest
@EnumSource(FailureKind.class)
void offersEachActionAtMostOnce(FailureKind kind) {
// Declaration order is the client's tie-break, so the same action twice would be two
// buttons with one meaning, and labelKeyFor would silently answer for the first.
// The same action twice would be two buttons with one meaning, and labelKeyFor would
// answer for the first.
assertThat(kind.getActions()).doesNotHaveDuplicates();
}
@@ -320,8 +317,8 @@ class FailureKindTest {
@Test
void noKindOffersAcknowledgeAnyMore() {
// Kept in the vocabulary because rows are already ACKNOWLEDGED, and those must stay
// readable. Nothing offers it, so nothing can dispatch it either.
// Kept in the vocabulary for rows already ACKNOWLEDGED; offered by nothing, so
// dispatchable by nothing.
for (FailureKind kind : FailureKind.values()) {
assertThat(kind.declares(FailureActionId.ACKNOWLEDGE))
.as("%s offers ACKNOWLEDGE", kind.getId())
@@ -331,8 +328,7 @@ class FailureKindTest {
@Test
void everyKindLabelsItsActionsWithTheSharedWordingToday() {
// The per-kind override still exists for wording that reads badly in context; nothing
// needs it now that Dismiss sits in an overflow menu, where the shared word is right.
// The per-kind override still exists for wording that reads badly in context.
for (FailureKind kind : FailureKind.values()) {
for (FailureActionId action : kind.getActions()) {
assertThat(kind.labelKeyFor(action))
app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventServiceTest.java
+17 -31
View File
@@ -85,9 +85,7 @@ class FileRunEventServiceTest {
class Acknowledge {
/**
* No kind offers ACKNOWLEDGE any more, so it cannot be dispatched; the bean stays for rows
* that are already {@code ACKNOWLEDGED}. Exercised directly so their transition stays
* covered.
* No kind offers it, so it cannot be dispatched; exercised directly for rows that have it.
*/
private FileRunEvent acknowledge(FileRunEvent event, String actor) {
return new AcknowledgeAction(store).execute(event, Map.of(), actor);
@@ -128,8 +126,6 @@ class FileRunEventServiceTest {
@Test
void anAlreadyAcknowledgedRowStaysReadableAndClosable() {
// The reason the id and its bean stay: a row in this state predates the retry actions
// and must not become a row nobody can act on.
FileRunEvent event = given(FailureKind.INPUT_PASSWORD_PROTECTED, TEAM, "f1");
acknowledge(event, ACTOR);
@@ -301,8 +297,7 @@ class FileRunEventServiceTest {
@Test
void anActionTheClientRunsIsRefusedRatherThanPretendedTo() {
// UNKNOWN offers VIEW_FILE, and the server has neither the document nor the tool.
// Answering 200 here would tell the client something happened when nothing did.
// Answering 200 would tell the client something happened when nothing did.
FileRunEvent event = given(FailureKind.UNKNOWN, TEAM, "f1");
assertThatThrownBy(() -> service.dispatch(event.id(), "VIEW_FILE", Map.of()))
@@ -316,8 +311,7 @@ class FileRunEventServiceTest {
@Test
void everyClientActionIsRefusedWhicheverKindDeclaresIt() {
// Asserted over the vocabulary rather than one id, so an action added on the client
// side cannot arrive dispatchable because nobody thought to test it.
// Over the whole vocabulary, so a client action added later cannot arrive dispatchable.
for (FailureKind kind : FailureKind.values()) {
FileRunEvent event = given(kind, TEAM, "f-" + kind.getId());
for (FailureActionId action : kind.getActions()) {
@@ -406,8 +400,7 @@ class FileRunEventServiceTest {
@Test
void theSameRowIsMineToOnePersonAndTheirsToAnother() {
// Why it is derived and not stored: one stored answer would be wrong for everyone but
// the person it was stored for.
// Why it is derived: a stored answer would be wrong for everyone but one person.
FileRunEvent event = givenHitBy(ACTOR, FailureKind.UNKNOWN, TEAM, "f1");
assertThat(service.ownershipOf(event)).isEqualTo(Ownership.MINE);
@@ -446,8 +439,7 @@ class FileRunEventServiceTest {
@Test
void aReviewerReadingAColleaguesIsNotOfferedTheDocumentTheyDoNotHave() {
// Dropped rather than disabled: a greyed-out "View file" would read as the
// reviewer's permission problem, when it is simply not their document.
// Dropped, not disabled: greyed out would read as their permission problem.
FileRunEvent theirs =
givenHitBy(
"colleague@example.com",
@@ -476,8 +468,8 @@ class FileRunEventServiceTest {
@Test
void inheritedOwnerActionsComeBackDisabledWithTheReasonWhy() {
// The file was fed by a folder, bucket or webhook, and re-running it from there is work
// that has not landed. Said out loud rather than offered as a button that does nothing.
// No browser holds a source-fed file, so it is stated rather than offered as a dead
// button.
FileRunEvent unattended =
givenHitBy(null, FailureKind.INPUT_PASSWORD_PROTECTED, TEAM, "f1");
@@ -509,9 +501,8 @@ class FileRunEventServiceTest {
@Test
void theOwnersActionsAreDisabledWhenTheRowNamesNoDocument() {
// A row with no document id has nothing to name even for the owner holding the file.
// Answered here rather than left to the client, which would otherwise report it "not on
// this device" while it sits in their own workbench.
// Answered here, or the client calls it "not on this device" while it sits in their
// own workbench.
FileRunEvent documentless =
givenHitBy(ACTOR, FailureKind.INPUT_PASSWORD_PROTECTED, TEAM, null);
@@ -540,8 +531,7 @@ class FileRunEventServiceTest {
@Test
void aMemberIsNotOfferedTheOwnerActionsOnAnUnattendedRow() {
// The inheritance is the reviewer's, not everybody's. A member has no claim on a run
// nobody attended, and cannot see one in the first place.
// The inheritance is the reviewer's: a member has no claim on a run nobody attended.
when(authority.canEditPolicies()).thenReturn(false);
FileRunEvent unattended =
givenHitBy(null, FailureKind.INPUT_PASSWORD_PROTECTED, TEAM, "f1");
@@ -551,8 +541,8 @@ class FileRunEventServiceTest {
@Test
void aLoginDisabledOperatorKeepsTheirOwnActions() {
// Its rows are unowned because it has no users, not because nothing attended them, and
// the one operator is holding the document. Disabling their retry would be wrong.
// Unowned for want of users, not because nothing attended: the one operator holds the
// file.
ApplicationProperties props = new ApplicationProperties();
props.getSecurity().setEnableLogin(false);
FileRunEventService unsecured =
@@ -765,16 +755,14 @@ class FileRunEventServiceTest {
complete.verifyEveryDeclaredActionHasAHandler();
for (FailureActionId id : FailureActionId.values()) {
// Only the server's actions need a handler: the rest are run by the client, which
// is why the boot check no longer asks about them.
// Only server actions need a handler, which is why the boot check ignores the rest.
assertThat(complete.find(id).isPresent()).isEqualTo(id.runsOnServer());
}
}
@Test
void doesNotAskForAHandlerForAnActionTheClientRuns() {
// Every kind declares client actions, so a registry holding only the server's two must
// still boot; otherwise every client action would need an empty handler beside it.
// Otherwise every client action would need an empty handler beside it.
FailureActionRegistry serverOnly =
new FailureActionRegistry(
List.of(new AcknowledgeAction(store), new DismissAction(store)));
@@ -785,16 +773,14 @@ class FileRunEventServiceTest {
@Test
void refusesAHandlerForAnActionTheClientRuns() {
// It could never be reached: dispatch refuses the id before resolving a handler. A bean
// that is never called is worse than no bean, because it reads as though it were.
// Dispatch refuses the id before resolving a handler, so the bean reads as live and is
// not.
assertThatThrownBy(() -> new FailureActionRegistry(List.of(new ClientSideAction())))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("VIEW_FILE");
}
/**
* A handler for an action the client runs, which is exactly what must not be registered.
*/
/** A handler for a client action, which is exactly what must not be registered. */
private static final class ClientSideAction implements FailureAction {
@Override
app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventStoreDbTest.java
+4 -8
View File
@@ -241,9 +241,8 @@ class FileRunEventStoreDbTest {
store.record(
RecordFailure.forEditor(
FailureKind.UNKNOWN, TEAM, "owner@example.com", "f-1", "boom"));
// A policy the user's own upload triggered. Recorded by the processor, but about the very
// document they just deleted, and carrying the id their client minted for it. Keying on
// origin left these behind: the user deleted the file and the failure stayed in the queue.
// Recorded by the processor, about the document they just deleted. Keying on origin left
// these in the queue.
FileRunEvent myPolicyRun =
store.record(failure(FailureKind.UNKNOWN, TEAM, "owner@example.com", "f-1"));
FileRunEvent theirs =
@@ -278,11 +277,8 @@ class FileRunEventStoreDbTest {
@Test
@DisplayName("a source-fed incident survives a client naming its file id")
void markFilesRemovedLeavesSourceFedRowsAlone() {
// The two id spaces meet here. A source-fed run's fileId is a one-way hash of a path or key
// that was never on any device, so no client can legitimately claim to have deleted it.
// With login disabled the actor is null on both sides, so the actor clause matches and the
// absence of a source is the only thing standing between a local delete and a sweep's
// incidents.
// With login disabled the actor is null on both sides, so the absence of a source is all
// that stands between a local delete and a sweep's incidents.
FileRunEvent sweep =
store.record(
new RecordFailure(
app/proprietary/src/test/java/stirling/software/proprietary/failure/NotificationProjectionTest.java
+8 -13
View File
@@ -23,9 +23,8 @@ import stirling.software.proprietary.notification.NotificationView;
import stirling.software.proprietary.policy.config.PolicyManagementAuthority;
/**
* What the bell is given to render. Lives beside the failure tests because the invariants are
* failure invariants: the bell is never handed a raw event id, and it is offered only the actions
* the client itself runs, resolved for this reader by the same service that scopes the queue.
* What the bell is given to render: never a raw event id, and only the actions the client itself
* runs, resolved for this reader by the same service that scopes the queue.
*/
@ExtendWith(MockitoExtension.class)
class NotificationProjectionTest {
@@ -96,12 +95,10 @@ class NotificationProjectionTest {
assertThat(notification.status()).isEqualTo(FileRunEventStatus.NEW);
assertThat(notification.fileId()).isEqualTo("f-1");
assertThat(notification.policyId()).isNull();
// No source fed it, which is how the client knows the fileId above is one of its own
// references and worth looking up locally.
// How the client knows the fileId above is one of its own and worth looking up.
assertThat(notification.sourceId()).isNull();
assertThat(notification.defaultTitle()).isNotBlank();
// The failure queue's own resolved offers, minus the ones the server runs: a bell
// offering different client actions from the queue would be a bell that lies.
// The queue's own offers minus the server's: a bell offering different ones would lie.
assertThat(notification.actions())
.containsExactlyElementsOf(
FileRunEventView.of(mine, failures.availableActions(mine))
@@ -116,8 +113,7 @@ class NotificationProjectionTest {
@Test
void offersNoActionTheServerRunsBecauseDispositionsBelongToTheQueue() {
// The bell routes the user to the right surface; deciding a failure's fate (dismissing
// it) happens on the review surface, so no disposition button reaches the panel.
// Deciding a failure's fate belongs to the review surface, not the panel.
given(FailureKind.INPUT_PASSWORD_PROTECTED, ACTOR, "f-1");
assertThat(controller.list(null).notifications().getFirst().actions())
@@ -127,8 +123,8 @@ class NotificationProjectionTest {
@Test
void namesTheSourceThatFedAnUnattendedRunSoItsFileIdIsNotMistakenForAClientsOwn() {
// Only a source-fed run's fileId is a hash, so the source is the discriminator: without
// it a client would look up a hash it can never resolve and call the document missing.
// Without the source a client looks up a hash it can never resolve and calls it
// missing.
store.record(
RecordFailure.forRun(
FailureKind.INPUT_PASSWORD_PROTECTED,
@@ -148,8 +144,7 @@ class NotificationProjectionTest {
@Test
void aColleaguesNotificationOffersTheReviewersActionsOnly() {
// The bell shows a leader their team's failures, so the audience filtering has to reach
// it: no offering someone a document they do not have.
// A leader sees the team's failures, so audience filtering has to reach the bell too.
given(FailureKind.INPUT_PASSWORD_PROTECTED, "colleague@example.com", "f-1");
assertThat(controller.list(null).notifications().getFirst().actions())
app/proprietary/src/test/java/stirling/software/proprietary/failure/PolicyFailureOwnershipTest.java
+11 -26
View File
@@ -55,15 +55,9 @@ import stirling.software.proprietary.policy.store.PolicyStore;
import tools.jackson.databind.json.JsonMapper;
/**
* What a reader is offered on a real recorded row, with every collaborator between the failing tool
* call and the offered actions being the real one. That {@code actor} names whoever triggered the
* run is settled upstream by {@code PolicyFailureAttributionTest}; what is pinned here is what the
* reader's relationship to that actor then entitles them to.
*
* <p>Ownership is derived per reader rather than stored, so the same row answers differently to the
* person holding the document and to whoever reviews after them. Both directions are asserted,
* because an action offered to the wrong one is either a button that cannot work or a document
* handed to someone who should not have it.
* What a reader is offered on a real recorded row, every collaborator being the real one. Both
* directions are asserted: offered to the wrong reader is either a dead button or a leaked
* document.
*/
@ExtendWith(MockitoExtension.class)
class PolicyFailureOwnershipTest {
@@ -150,9 +144,7 @@ class PolicyFailureOwnershipTest {
TEAM);
}
/**
* Run the shared policy so its single tool step fails, as {@code triggeredBy} (null = sweep).
*/
/** Fails the policy's single tool step as {@code triggeredBy} (null = sweep). */
private void runAndFail(String triggeredBy, String sourceId, String fileIdentity)
throws Exception {
when(internalApiClient.post(eq(ROTATE), any())).thenThrow(new RuntimeException("boom"));
@@ -183,10 +175,8 @@ class PolicyFailureOwnershipTest {
}
/**
* Read the single recorded row as {@code reader}, who is a plain member of the team. Lenient
* because a leader's read scope and an UNOWNED ownership check both answer without asking who
* is reading, so whether the name is consulted is the behaviour under test rather than a
* mistake.
* Lenient because a leader's scope and an UNOWNED check both answer without asking who reads,
* so whether the name is consulted is the behaviour under test.
*/
private FileRunEvent asMember(String reader) {
lenient().when(userService.getCurrentUsername()).thenReturn(reader);
@@ -231,9 +221,7 @@ class PolicyFailureOwnershipTest {
void thePolicyOwnerIsNotHandedADocumentSheNeverTouched() throws Exception {
runAndFail("bob", null, "bob-doc-1");
// Alice owns the policy and pays for the run, but her browser has no copy of Bob's
// file. Offering her the document produced a button that vanished client-side with no
// explanation.
// She owns the policy and pays for the run, and still has no copy of Bob's file.
FileRunEvent theirs = asReviewer("alice");
assertThat(service.ownershipOf(theirs)).isEqualTo(Ownership.THEIRS);
assertThat(offeredTo(theirs)).doesNotContain(FailureActionId.VIEW_FILE);
@@ -243,8 +231,7 @@ class PolicyFailureOwnershipTest {
void theReviewerIsStillOfferedWhatReviewingNeeds() throws Exception {
runAndFail("bob", null, "bob-doc-1");
// Not her document, but still her team's incident: she gets the run and a way to close
// the row.
// Not her document, still her team's incident.
assertThat(offeredTo(asReviewer("alice")))
.contains(FailureActionId.VIEW_IN_PROCESSOR, FailureActionId.DISMISS);
}
@@ -260,9 +247,7 @@ class PolicyFailureOwnershipTest {
FileRunEvent unattended = asReviewer("alice");
assertThat(service.ownershipOf(unattended)).isEqualTo(Ownership.UNOWNED);
// The inheritance and its copy are reachable for the exact scenario they exist for: no
// browser holds this document, so the offer is stated and disabled rather than silently
// dropped.
// No browser holds this document, so the offer is stated and disabled, not dropped.
assertThat(offeredTo(unattended)).contains(FailureActionId.VIEW_FILE);
assertThat(service.availableActions(unattended))
.filteredOn(action -> action.id() == FailureActionId.VIEW_FILE)
@@ -277,8 +262,8 @@ class PolicyFailureOwnershipTest {
@Test
void thePolicyOwnerDoesNotInheritItAsHerOwn() throws Exception {
// Alice is billed for the sweep, and that must not become ownership: she is offered the
// owner actions here only as the team's reviewer, and disabled, not as MINE.
// Being billed for the sweep must not become ownership: she gets these as reviewer
// only.
runAndFail(null, "src-watched-folder", "file-hash-1");
assertThat(service.ownershipOf(asReviewer("alice"))).isNotEqualTo(Ownership.MINE);
app/proprietary/src/test/java/stirling/software/proprietary/failure/PolicyFailureRecorderTest.java
+3 -8
View File
@@ -383,12 +383,8 @@ class PolicyFailureRecorderTest {
@Test
void theSameDocumentFailingInTwoAttendedRunsIsOneIncident() {
// WHAT THE DOCUMENT REFERENCE BUYS. A password-protected input is FILE-scoped, and
// every
// upload is a new run, so with no reference the run id stands in for the document and
// the
// same broken file re-uploaded reads as a second incident instead of a second
// occurrence.
// Every upload is a new run, so with no reference the run id stands in for the document
// and the same broken file reads as a second incident rather than a second occurrence.
when(policyStore.get("policy-1")).thenReturn(Optional.of(policy("policy-1", TEAM)));
recorder.recordRunFailure(
@@ -403,8 +399,7 @@ class PolicyFailureRecorderTest {
@Test
void twoDocumentsFailingTheSameWayStaySeparateIncidents() {
// The other half of the rule: folding is per document, so two broken uploads are still
// two rows and neither is credited with the other's occurrence.
// Folding is per document, so neither row is credited with the other's occurrence.
when(policyStore.get("policy-1")).thenReturn(Optional.of(policy("policy-1", TEAM)));
recorder.recordRunFailure(
app/proprietary/src/test/java/stirling/software/proprietary/failure/RecordFailurePrivacyTest.java
+2 -4
View File
@@ -57,10 +57,8 @@ class RecordFailurePrivacyTest {
@Test
void theRunRequestThatSuppliesADocumentReferenceCarriesNoNameEither() {
// An attended run sends its own opaque reference to the document so a failure of it can be
// resolved back to that document. The same discipline applies at the door as in the row: an
// id and nothing else, or a document name reaches the wire on the way to a table that
// deliberately has nowhere to put it.
// The same discipline at the door as in the row: an id and nothing else, or a document name
// reaches a table that deliberately has nowhere to put it.
assertThat(List.of(PolicyRunFiles.class.getDeclaredFields()))
.extracting(Field::getName)
.contains("fileId")
app/proprietary/src/test/java/stirling/software/proprietary/policy/controller/PolicyControllerTest.java
+6 -11
View File
@@ -86,8 +86,7 @@ class PolicyControllerTest {
@Mock private ProcessedLedger processedLedger;
// Real, not mocked: the run endpoints spool their uploads through it, and a test that supplies
// files needs them to actually land somewhere.
// Real, not mocked: the run endpoints spool uploads through it.
private final TempFileManager tempFileManager =
new TempFileManager(new TempFileRegistry(), new ApplicationProperties());
@@ -669,9 +668,7 @@ class PolicyControllerTest {
@DisplayName("runStoredPolicy")
class RunStoredPolicy {
/**
* The run files an editor sends: the documents, plus its own id for a single one of them.
*/
/** What an editor sends: the documents, plus its own id for a single one of them. */
private PolicyRunFiles filesWith(String fileId, int documents) {
PolicyRunFiles files = new PolicyRunFiles();
files.setFileId(fileId);
@@ -689,7 +686,6 @@ class PolicyControllerTest {
return files;
}
/** The document reference the run was actually started with. */
private String documentReferenceOf(PolicyRunFiles files) throws Exception {
Policy p = policy("a", 1L);
when(policyStore.get("a")).thenReturn(Optional.of(p));
@@ -724,8 +720,8 @@ class PolicyControllerTest {
@Test
@DisplayName("records the caller's own id for a single-document run")
void carriesTheCallersDocumentReference() throws Exception {
// The point of the whole field: a failure of this run names a document the client that
// started it can resolve, so the actions that need the bytes are reachable.
// The point of the field: a failure names a document the client that started it can
// resolve.
assertThat(documentReferenceOf(filesWith("editor-file-1", 1)))
.isEqualTo("editor-file-1");
}
@@ -733,15 +729,14 @@ class PolicyControllerTest {
@Test
@DisplayName("records nothing when the run carries several documents")
void refusesToGuessWhichOfSeveralDocumentsItIs() throws Exception {
// One incident, one file reference: naming one of several would attribute the
// failure to whichever document happened to be bound first.
// One incident, one reference: naming one of several would attribute it to whichever
// bound first.
assertThat(documentReferenceOf(filesWith("editor-file-1", 3))).isNull();
}
@Test
@DisplayName("records nothing when the caller sent no id")
void toleratesACallerThatSendsNoReference() throws Exception {
// Every other client (and every older one) keeps working exactly as before.
assertThat(documentReferenceOf(filesWith(null, 1))).isNull();
}
frontend/editor/src/core/components/notifications/BellIcon.tsx
+2 -3
View File
@@ -1,7 +1,6 @@
/**
* Outlined bell. The bundled Material Symbols set only carries the filled variant, and a filled
* bell reads as a permanently-ringing one, so this mirrors the portal's own {@code BellIcon}
* rather than using it: core cannot import from portal.
* The bundled Material Symbols set only carries the filled variant, which reads as permanently
* ringing. Mirrors the portal's own icon rather than importing it: core cannot reach into portal.
*/
export function BellIcon({ size = 18 }: { size?: number }) {
return (
frontend/editor/src/core/components/notifications/NotificationBell.test.tsx
+9 -16
View File
@@ -36,8 +36,7 @@ vi.mock("@app/services/notifications", () => ({
},
}));
// The document lookups read IndexedDB, which jsdom has none of. Answered here so a row's
// availability is a fact of the test rather than of the environment.
// IndexedDB, which jsdom has none of. Answered here so availability is a fact of the test.
const h = vi.hoisted(() => ({
hasLocalFile: true,
retryPayload: { operation: "removePassword" } as unknown,
@@ -57,16 +56,14 @@ vi.mock("@app/services/notificationRetry", () => ({
loadRetryPayload: () => Promise.resolve(h.retryPayload),
}));
// Stands in for the layer that owns the destinations. Core's own registry is empty, so without
// this there are no client actions to test.
// Core's own registry is empty, so without this there are no client actions to test.
vi.mock("@app/components/notifications/notificationActions", () => ({
useNotificationActions: () => h.specs,
}));
vi.mock("react-i18next", () => ({
useTranslation: () => ({
// Mirrors i18next closely enough for this component: a string fallback, or an options object
// carrying defaultValue plus the values it interpolates.
// A string fallback, or an options object with defaultValue plus what it interpolates.
t: (key: string, fallback?: unknown) => {
if (typeof fallback === "string") return fallback;
if (fallback && typeof fallback === "object") {
@@ -180,8 +177,7 @@ describe("NotificationBell", () => {
render(<NotificationBell />);
await openPanel();
// Opening is what marks them read: waiting for the close would leave the badge lit
// while the user is looking at the list.
// Opening marks them read: waiting for the close would leave the badge lit.
await waitFor(() => expect(screen.queryByText("2")).toBeNull());
});
@@ -200,8 +196,7 @@ describe("NotificationBell", () => {
});
it("keeps the division on screen after opening marks them read", async () => {
// The boundary is frozen on open. Read live it would collapse the moment the badge cleared,
// taking the divider with it while the user was still looking at the list.
// Frozen on open: read live it would collapse the moment the badge cleared.
markReadThrough(AT.b);
fetchNotifications.mockResolvedValue([
notification("a"),
@@ -298,7 +293,7 @@ describe("NotificationBell", () => {
render(<NotificationBell />);
await openPanel();
// Named for the row they belong to: every button in the list says the same thing.
// Named for their row: every button in the list says the same thing.
for (const id of ["VIEW_IN_PROCESSOR", "VIEW_FILE"])
expect(
screen.getByRole("button", { name: `${id}: Unrecognised failure` }),
@@ -450,8 +445,7 @@ describe("NotificationBell", () => {
});
it("claims nothing about a device for a row it never looks up", async () => {
// A source-fed run's fileId is a server-side identity that was never on any device, so it is not
// probed. Absent lookups must not read as an absent document, and the server said nothing either.
// Never on any device, so never probed, and an absent lookup is not an absent document.
h.hasLocalFile = false;
fetchNotifications.mockResolvedValue([
notification("a", "Password-protected document", {
@@ -471,8 +465,7 @@ describe("NotificationBell", () => {
});
it("renders no button for an action the server would refuse, and says why in words", async () => {
// A greyed button on a failure it can never work for is false hope, so the next action takes the
// row and the reason becomes its note.
// A greyed button that can never work is false hope, so the reason becomes the row's note.
h.specs = {
VIEW_FILE: { available: () => true, run: vi.fn() },
VIEW_IN_PROCESSOR: { available: () => true, run: vi.fn() },
@@ -525,7 +518,7 @@ describe("NotificationBell", () => {
render(<NotificationBell />);
await openPanel();
// The message and its reading aids remain, so the row still reads as a row.
// The message and its chips remain, so the row still reads as a row.
expect(screen.getByText("Unrecognised failure")).toBeTruthy();
expect(
screen.getByText("Not available for this notification."),
frontend/editor/src/core/components/notifications/NotificationBell.tsx
+17 -35
View File
@@ -33,11 +33,8 @@ import type { NotificationDocumentState } from "@app/hooks/useNotifications";
import "@app/components/notifications/NotificationBell.css";
/**
* The bell and its panel. Lives in core because both shells mount it and the dependency only runs
* one way: the portal may import from core, never the reverse.
*
* <p>Renders whatever the server sends without knowing which subsystem produced it, or what any of its
* actions mean, so a new source or failure kind needs no change here.
* Renders whatever the server sends without knowing which subsystem produced it or what its actions
* mean, so a new source or failure kind needs no change here. In core because both shells mount it.
*/
export function NotificationBell() {
const { t } = useTranslation();
@@ -48,16 +45,12 @@ export function NotificationBell() {
const container = useRef<HTMLDivElement>(null);
const headingId = useId();
/**
* The first notification the user had already seen when they opened the panel, which is where the
* new ones stop. Held as an id rather than a count because opening marks everything read, so a
* live count would collapse to zero and take the divider with it while they were reading.
*
* An id also survives the list changing underneath: one arriving on a poll lands above the
* divider, where it belongs, instead of shifting a frozen index onto the wrong row.
* Where the new ones stop, frozen on open. An id rather than a count because opening marks
* everything read, and because one arriving on a poll must land above the divider, not shift it.
*/
const [firstSeenId, setFirstSeenId] = useState<string | null>(null);
// Fixed to the viewport, positioned from the trigger. The workbench bar clips its overflow, so
// an absolutely positioned panel is cut off by its own toolbar.
// Fixed to the viewport: the workbench bar clips its overflow, so an absolutely positioned panel
// would be cut off by its own toolbar.
const [anchor, setAnchor] = useState<{ top: number; right: number } | null>(
null,
);
@@ -121,12 +114,11 @@ export function NotificationBell() {
};
}, [open]);
// Opening is what marks them read, not closing: the user has seen them by then, and waiting
// until close would leave the badge lit while they are looking at the list.
// Opening marks them read, not closing: waiting would leave the badge lit while they read.
const toggle = () => {
setOpen((wasOpen) => {
if (!wasOpen) {
// Read the boundary before marking, or there is nothing left to read.
// Before marking, or there is nothing left to read.
setFirstSeenId(notifications[unreadCount]?.id ?? null);
markAllSeen();
}
@@ -135,12 +127,8 @@ export function NotificationBell() {
};
/**
* How many of the listed notifications count as new, for this reading of the panel. Everything
* above it is new, everything from it down is earlier.
*
* No boundary id means everything was new when the panel opened, so the whole list is. A boundary
* that has since left the list (dismissed elsewhere, its document deleted) leaves nothing to
* divide on, and reads as "none new" rather than guessing at a row.
* How many count as new. No boundary id means all of them were; one that has since left the list
* leaves nothing to divide on, so it reads as none rather than guessing at a row.
*/
const boundaryIndex = firstSeenId
? notifications.findIndex((notification) => notification.id === firstSeenId)
@@ -215,8 +203,8 @@ export function NotificationBell() {
/>
</li>
)}
{/* Only where there is something on both sides of it: a lone "Earlier" heading
over the whole list says nothing the empty badge has not already said. */}
{/* Only with something on both sides: a lone "Earlier" over everything says
nothing the empty badge has not. */}
{index === dividedAt && dividedAt > 0 && (
<li aria-hidden>
<DividerWithText
@@ -271,9 +259,8 @@ interface PasswordPrompt {
}
/**
* The row's one line of explanation, if it has earned one. The server's reason for what it withheld
* wins, being about this failure rather than this browser; otherwise only what we actually know, so a
* row we never look up is never called absent.
* The server's reason wins, being about the failure rather than this browser. Otherwise only what we
* actually looked up, so a row we never probed is never called absent.
*/
function noteFor(
notification: AppNotification,
@@ -313,10 +300,7 @@ interface NotificationItemProps {
onRequestPassword: (prompt: PasswordPrompt) => void;
}
/**
* One row. Its own component because the message its last attempt came back with and whether that
* message is expanded are per-row state the panel cannot hold.
*/
/** Its own component because the last attempt's message and its expanded state are per-row. */
function NotificationItem({
notification,
unread,
@@ -387,7 +371,7 @@ function NotificationItem({
await navigator.clipboard.writeText(notification.detail);
setCopied(true);
} catch {
// No clipboard permission. The message is on screen and selectable, so this needs no error.
// No clipboard permission, and the message is on screen and selectable anyway.
}
};
@@ -463,7 +447,6 @@ function NotificationItem({
</div>
)}
{/* Actions were taken away from this row, so say why rather than leaving a bare row. */}
{note && <span className="notification-bell__note">{note}</span>}
{/* Two buttons at most, then a menu: the row's own answer, one runner-up, and the rest tucked
@@ -551,8 +534,7 @@ function ActionButton({
fontSize="xs"
className="notification-bell__cta"
disabled={busy}
// Every row's buttons say the same thing, so the label alone would not tell a screen reader which
// failure it acts on.
// Every row's buttons read alike, so the label alone would not say which failure this acts on.
aria-label={`${label}: ${rowTitle}`}
onClick={onRun}
>
frontend/editor/src/core/components/notifications/notificationActions.ts
+8 -20
View File
@@ -2,12 +2,10 @@ import type { AppNotification } from "@app/services/notifications";
import type { RetryPayload } from "@app/services/notificationRetry";
/**
* What this client can do about a notification, keyed by the action id the server offered. Keyed by
* action rather than by row because the server decides which actions a failure kind offers: adding a
* kind is no frontend change, and adding a button is one entry here.
* Keyed by action rather than by row, because the server decides what a kind offers: adding a kind is
* no frontend change, and adding a button is one entry here.
*/
/** Everything an action needs to decide whether it can run, and to run. */
export interface NotificationActionContext {
notification: AppNotification;
/** Whether the document is still in this browser, which is what most actions hinge on. */
@@ -16,44 +14,34 @@ export interface NotificationActionContext {
retryPayload: RetryPayload | null;
}
/**
* How a client action reports back. `void` means it did what it said, usually a navigation; a failed
* outcome carries the message the row shows, because an action that quietly does nothing leaves the
* user guessing.
*/
/** `void` means it did what it said; a failed outcome carries the message the row shows. */
export interface ClientActionOutcome {
ok: boolean;
message?: string;
}
export interface ClientActionSpec {
/** Whether this device can perform it right now. Asked per row, never during a request. */
/** Asked per row, never during a request. */
available(context: NotificationActionContext): boolean;
/** `password` is only ever passed for a spec that asked for one. May answer synchronously. */
run(
context: NotificationActionContext,
password?: string,
): ClientActionOutcome | void | Promise<ClientActionOutcome | void>;
/** Collect a password in the row before running. Never stored, never logged. */
/** Collect a password before running. Never stored, never logged. */
needsPassword?: boolean;
/** Whether the panel should get out of the way, because the destination is behind it. */
/** Whether the panel should get out of the way, the destination being behind it. */
closesPanel?: boolean;
}
/**
* Indexed by action id. An id with no entry is skipped rather than rendered unwired: the server can
* ship a new failure kind, with new actions, long before a client knows what they mean.
*/
/** An id with no entry is skipped rather than rendered unwired. */
export type ClientActionRegistry = Readonly<
Record<string, ClientActionSpec | undefined>
>;
const NONE: ClientActionRegistry = {};
/**
* Nothing is offered here: every destination a notification could point at ships in a higher layer, so
* a build without them has nowhere to send anyone, so its rows carry no buttons.
*/
/** Every destination ships in a higher layer, so this build's rows carry no buttons. */
export function useNotificationActions(): ClientActionRegistry {
return NONE;
}
frontend/editor/src/core/contexts/FileContext.tsx
+2 -5
View File
@@ -611,11 +611,8 @@ function FileContextInner({
// Remove from memory and cleanup resources
lifecycleManager.removeFiles(fileIds, stateRef);
// Only a real delete closes a failure. Most callers pass false and mean "take it out of
// the workbench": closing a tab, unchecking it in the file manager, swapping which files
// are open. The document is still on the device and its failures still need attention, so
// telling the server it was deleted would quietly clear the user's own notifications while
// they worked. Fire-and-forget, so a server that cannot be told never blocks the delete.
// Only a real delete closes a failure: most callers pass false and mean "take it out of the
// workbench", leaving the document, and its failures, very much alive.
if (deleteFromStorage !== false) {
void reportFilesRemoved(fileIds);
}
frontend/editor/src/core/contexts/NavigationContext.tsx
+2 -3
View File
@@ -133,9 +133,8 @@ const NavigationStateContext = createContext<
NavigationContextStateValue | undefined
>(undefined);
/**
* Exported for the same reason as {@link FileActionsContext}: a component that
* mounts in both shells has to ask whether these actions are available rather
* than assume it, and {@link useNavigationActions} throws when they are not.
* Exported like {@link FileActionsContext}: a component mounting in both shells must ask whether
* these exist, and {@link useNavigationActions} throws when they do not.
*/
export const NavigationActionsContext = createContext<
NavigationContextActionsValue | undefined
frontend/editor/src/core/contexts/file/removeFiles.reporting.test.tsx
+5 -11
View File
@@ -7,11 +7,8 @@ import type { FileContextActions } from "@app/types/fileContext";
import type { FileId } from "@app/types/file";
/**
* Which removals tell the server a document is gone.
*
* `removeFiles` serves two jobs the caller distinguishes only by `deleteFromStorage`: deleting a
* document, and taking one out of the workbench. Reporting both closes the user's own failure
* notifications as they open and close files, which is not a decision they made.
* `removeFiles` deletes a document or merely takes it out of the workbench, told apart only by
* `deleteFromStorage`. Reporting both closed the user's own notifications as they opened files.
*/
const reportFilesRemoved = vi.fn();
@@ -20,8 +17,7 @@ vi.mock("@app/services/failureReporting", () => ({
reportToolFailure: vi.fn(),
}));
// A real delete reaches IndexedDB, which jsdom has none of. Only the delete itself is stubbed: the
// point of these tests is which branch runs, so the branch has to be allowed to finish.
// IndexedDB, which jsdom has none of. Stubbed so the delete branch can run to the end.
vi.mock("@app/services/fileStorage", () => ({
// FileContext subscribes to this to drop files whose bytes are unreadable.
onRecordUnreadable: () => () => {},
@@ -68,8 +64,7 @@ describe("removeFiles and the failure queue", () => {
});
it("says nothing when the file is only closed in the workbench", async () => {
// Closing a tab, unchecking it in the file manager, swapping which files are open: all of them
// pass false and leave the document on the device, so its failures still need attention.
// Closing a tab or unchecking it leaves the document on the device, failures and all.
setup();
await act(async () => {
@@ -80,8 +75,7 @@ describe("removeFiles and the failure queue", () => {
});
it("treats an unspecified removal as a delete, the way the storage path does", async () => {
// Same default as the IndexedDB branch below it: absent means delete, only an explicit false
// means keep.
// Same default as the IndexedDB branch: only an explicit false means keep.
setup();
await act(async () => {
frontend/editor/src/core/hooks/tools/shared/useToolOperation.ts
+1 -2
View File
@@ -611,8 +611,7 @@ export const useToolOperation = <TParams>(
// Report it so a leader sees the failure too, then carry on with the user's
// own error handling. Fire-and-forget: the reporter swallows its own errors.
// Chained rather than fired alongside: the re-read has to happen after the row exists, or
// it finds the list exactly as it was.
// Chained, not fired alongside: the re-read must happen after the row exists.
void reportToolFailure({
operation: config.operationType,
error,
frontend/editor/src/core/hooks/useNotifications.test.ts
+4 -8
View File
@@ -6,10 +6,8 @@ import type {
} from "@app/services/notifications";
/**
* The bell is mounted several times over (floated over an empty workbench, inside the workbench bar,
* again in the portal shell) and all of them show the same thing, so what is pinned here is that
* they share one read of it: one poll, one set of document lookups, one read marker, and no timer
* left running once the last of them has gone.
* The bell is mounted several times over, so what is pinned here is that they share one read: one
* poll, one set of lookups, one marker, and no timer left running once the last has gone.
*/
const fetchNotifications = vi.fn();
@@ -18,8 +16,7 @@ vi.mock("@app/services/notifications", () => ({
fetchNotifications: (...args: unknown[]) => fetchNotifications(...args),
}));
// The document lookups read IndexedDB, which jsdom has none of. Counted here so that "resolved once
// per list, not once per row" is observable.
// Counted here so "resolved once per list, not once per row" is observable.
const hasLocalFile = vi.fn((_fileId: string) => Promise.resolve(true));
const loadRetryPayload = vi.fn((_fileId: string) => Promise.resolve(null));
@@ -215,8 +212,7 @@ describe("useNotifications", () => {
await waitFor(() => expect(first.result.current.unreadCount).toBe(2));
expect(second.result.current.unreadCount).toBe(2);
// Async, because the store tells its subscribers on a microtask: a bell marks the list read
// from inside its own state updater, so it cannot re-render its neighbours from there.
// Async because subscribers are told on a microtask: a bell marks the list read while rendering.
await act(async () => first.result.current.markAllSeen());
expect(first.result.current.unreadCount).toBe(0);
frontend/editor/src/core/hooks/useNotifications.ts
+20 -61
View File
@@ -10,23 +10,14 @@ import {
} from "@app/services/notificationRetry";
/**
* The caller's notifications, refreshed on a timer because they arrive from background work rather
* than from anything the user just did.
* One polled store for however many bells are mounted. A module store rather than a context because
* the portal mounts its bell as a sibling of AppProviders, so there is no single tree to provide in.
*
* One set of data for however many bells are on screen. The bell is mounted more than once, so the
* list, the document lookups and the read marker live in the module-level store below and each mount
* merely subscribes; otherwise two badges can disagree while reading the same localStorage key.
*
* A module-level store rather than a React context because there is no single tree to hang a provider
* in: the portal mounts its bell as a sibling of AppProviders. Each bell still owns its own open state.
*
* TODO: read state is tracked here, in the browser, as the time of the newest notification the user
* has seen. That is enough for one device and one person, but it does not survive a cache clear
* and does not follow them to another browser. When notifications become a server-side concept
* with their own table, move this there: the server can then record which user has read which
* notification, durably and per user, and this hook just renders what it is told.
* TODO: read state lives in this browser, so it does not survive a cache clear or follow the user to
* another one. It belongs on the server once notifications have a table of their own.
*/
// TODO: read state is per-browser. Move it server-side when notifications get their own table.
const POLL_INTERVAL_MS = 30_000;
const SEEN_STORAGE_KEY = "stirling.notifications.readThroughAt";
@@ -41,8 +32,7 @@ function readReadThrough(): number | null {
const stored = Number(window.localStorage.getItem(SEEN_STORAGE_KEY));
return Number.isFinite(stored) && stored > 0 ? stored : null;
} catch {
// Private mode, or storage disabled. Everything then reads as unseen, which errs towards
// showing the user their failures rather than hiding them.
// Private mode: everything reads as unseen, which errs towards showing failures.
return null;
}
}
@@ -51,14 +41,10 @@ function writeReadThrough(at: number): void {
try {
window.localStorage.setItem(SEEN_STORAGE_KEY, String(at));
} catch {
// Nothing to do: the marker simply will not persist across a reload.
// The marker just will not survive a reload.
}
}
/**
* What this browser holds for the document a notification is about, which decides what can actually run
* here. Resolved for the list rather than per row, since the answers come from IndexedDB.
*/
export interface NotificationDocumentState {
hasLocalFile: boolean;
retryPayload: RetryPayload | null;
@@ -70,23 +56,16 @@ const NO_DOCUMENT: NotificationDocumentState = {
};
/**
* Whether this browser could resolve the document a notification names, which is the one place that
* rule is stated.
*
* Two id spaces share the one fileId field. An attended run reports the id its editor minted, so a
* lookup here can succeed; an unattended run reports its source's one-way hash of a path or key, which
* was never on any device. The discriminator is the absence of a source rather than the origin, since
* an attended policy run now sends a client reference too.
* Whether this browser could resolve the document a row names. Two id spaces share `fileId`: an
* attended run reports the id its editor minted, a source-fed one a hash that was never on a device.
*/
export function isResolvableHere(notification: AppNotification): boolean {
// Coalesced, so a row from a server that does not send the field reads as "no source".
return (notification.sourceId ?? null) === null;
}
/** Everything the bells read. Replaced wholesale, never mutated, so it can be a snapshot. */
interface NotificationsSnapshot {
notifications: AppNotification[];
/** Keyed by fileId, so several rows about the same document cost one pair of lookups. */
/** Keyed by fileId, so several rows about one document cost one lookup. */
documents: Record<string, NotificationDocumentState>;
/** Everything up to and including this time has been read. Epoch millis, never a row id. */
readThroughAt: number | null;
@@ -101,9 +80,8 @@ const NOTHING_LOADED: NotificationsSnapshot = {
let snapshot: NotificationsSnapshot = NOTHING_LOADED;
const subscribers = new Set<() => void>();
let pollTimer: number | null = null;
/** The read in progress, if any, so that everyone who asks while it runs joins it. */
let inFlight: Promise<void> | null = null;
/** Bumped whenever polling starts or stops, so a read from a finished cycle cannot write. */
/** Bumped when polling starts or stops, so a read from a finished cycle cannot write. */
let cycle = 0;
let notifyQueued = false;
@@ -112,10 +90,8 @@ function getSnapshot(): NotificationsSnapshot {
}
/**
* New snapshot now, subscribers told on a microtask. Deferred because a bell marks the list read from
* inside its own state updater, i.e. while rendering, and re-rendering every other bell from there is
* the render-phase update React refuses. A microtask is still the same tick, so no bell paints a badge
* it should have dropped. Coalesced, so a burst of writes costs one round of re-renders.
* Subscribers told on a microtask: a bell marks the list read from inside its own state updater, and
* re-rendering the others from there is the render-phase update React refuses.
*/
function publish(next: NotificationsSnapshot): void {
snapshot = next;
@@ -166,11 +142,7 @@ async function read(forCycle: number): Promise<void> {
publish({ ...snapshot, notifications: visible, documents });
}
/**
* Read the list once, however many callers want it: a caller arriving mid-read joins the read already
* running. An action's refresh can then be answered by a read that started just before it landed, which
* the next poll corrects.
*/
/** A caller arriving mid-read joins the one already running. */
function load(): Promise<void> {
if (inFlight) return inFlight;
const pending = read(cycle).finally(() => {
@@ -182,8 +154,7 @@ function load(): Promise<void> {
function startPolling(): void {
cycle += 1;
// Re-read from disk rather than trusting what the last bell left in memory: another tab may have
// moved the marker on since.
// From disk, not memory: another tab may have moved the marker on.
snapshot = { ...NOTHING_LOADED, readThroughAt: readReadThrough() };
pollTimer = window.setInterval(() => void load(), POLL_INTERVAL_MS);
void load();
@@ -194,14 +165,13 @@ function stopPolling(): void {
window.clearInterval(pollTimer);
pollTimer = null;
}
// Anything still in flight belongs to a cycle nobody is watching: drop its result, and do not let
// the next bell join it.
// Drop anything in flight: its cycle has nobody watching it.
cycle += 1;
inFlight = null;
snapshot = NOTHING_LOADED;
}
/** Polling lives exactly as long as there is a bell to show it: one timer, no leak. */
/** Polling lives exactly as long as there is a bell to show it. */
function subscribe(onStoreChange: () => void): () => void {
subscribers.add(onStoreChange);
if (subscribers.size === 1) startPolling();
@@ -211,7 +181,6 @@ function subscribe(onStoreChange: () => void): () => void {
};
}
/** Everything currently listed becomes read, for every bell at once. */
function markAllSeen(): void {
// The newest time in the list, not the first row's, so a re-sorted list cannot under-mark.
const newest = Math.max(
@@ -229,13 +198,8 @@ function refresh(): void {
}
/**
* Re-read now, for a caller that has just caused a notification to exist. Without it the person who
* triggered a failure waits up to a whole poll interval to be told about their own action, which
* reads as the app not having noticed. Everyone else's failures still arrive on the poll, which is
* what it is for.
*
* A no-op when no bell is mounted: there is nobody to tell, and the next mount reads for itself.
* Safe to call after a report that was refused, since the re-read simply finds nothing new.
* Re-read now, for a caller that just caused a notification: without it the person who triggered a
* failure waits a whole poll interval to hear about their own action. A no-op with no bell mounted.
*/
export function refreshNotificationsNow(): void {
if (subscribers.size === 0) return;
@@ -244,16 +208,11 @@ export function refreshNotificationsNow(): void {
export interface NotificationsState {
notifications: AppNotification[];
/**
* How many are newer than the last one the user looked at: the badge, and the boundary the panel
* divides new from earlier on. Read before {@link markAllSeen}, which zeroes it.
*/
/** Read before {@link markAllSeen}, which zeroes it. */
unreadCount: number;
/** What this device holds for a row's document. Answers for a document it knows nothing about. */
documentStateFor: (
notification: AppNotification,
) => NotificationDocumentState;
/** Call when the user opens the panel: everything currently listed becomes read. */
markAllSeen: () => void;
refresh: () => void;
}
frontend/editor/src/core/routes/portalBasename.ts
+2 -4
View File
@@ -7,9 +7,7 @@
export const PORTAL_BASENAME = "/processor";
/**
* Fragment identifying the recorded-failures section of the portal's Documents
* view. Here for the same reason as the basename: whoever links to that section
* and whoever renders it are in different layers, and neither should have to
* import the other to agree on the anchor.
* The recorded-failures section of the portal's Documents view. Here because whoever links to it and
* whoever renders it are in different layers.
*/
export const PORTAL_FAILURES_ANCHOR = "failures";
frontend/editor/src/core/services/notifications.ts
+11 -35
View File
@@ -1,60 +1,40 @@
import apiClient from "@app/services/apiClient";
/**
* The caller's notifications. Derived server-side from whatever produces them, so this client
* needs to know nothing about failures, policies or any later source: it renders what it is given
* and branches on `source` only when it wants to.
*/
// Derived server-side from whatever produces them, so this client knows nothing about failures.
const NOTIFICATIONS_PATH = "/api/v1/notifications";
/** Which subsystem produced a notification. Widen as the server gains sources. */
export type NotificationSource = "FAILURE";
export type NotificationSeverity = "ERROR" | "WARNING" | "INFO";
/** What was running when it failed. */
export type NotificationOrigin = "TOOL" | "POLICY" | "PIPELINE";
/**
* Whose document it is, from this reader's point of view. `UNOWNED` is an unattended run: nobody has
* the file, so nothing that needs the bytes can be offered.
*/
/** From this reader's point of view. `UNOWNED` is an unattended run: nobody holds the file. */
export type NotificationOwnership = "MINE" | "THEIRS" | "UNOWNED";
/**
* How much of the row an action has earned. The server ranks by what the action does, not by where it
* ends up on screen; `promoteActions` turns a slot into a button or a menu entry.
* How much of the row an action has earned. The server ranks by what it does, not by where it lands;
* `promoteActions` turns a slot into a button or a menu entry.
*/
export type NotificationActionSlot = "RESOLUTION" | "SECONDARY" | "OVERFLOW";
/**
* One action as offered for one notification, all of them run by this client on its own device.
* `id` is a plain string rather than a union because the server may know actions this build does
* not, and the client skips the ones it cannot perform.
*/
/** `id` is an open string, not a union: the server may know actions this build does not. */
export interface NotificationActionOffer {
id: string;
labelKey: string;
/** English fallback, for a build with no copy for `labelKey`. */
defaultLabel: string;
slot: NotificationActionSlot;
/** False means it cannot work for this row. The bell renders no button and states the reason
* instead; the portal's queue still shows it disabled. */
/** False renders no button in the bell, and a disabled one in the portal's queue. */
enabled: boolean;
disabledReasonKey: string | null;
}
export interface AppNotification {
/**
* Unique across sources, so it can be keyed and compared on its own. Prefixed with its source
* (`failure:<uuid>`), so it is the id the notification endpoints take and never one a per-source
* endpoint would accept.
*/
/** Prefixed with its source (`failure:<uuid>`), so it is never an id a per-source endpoint takes. */
id: string;
source: NotificationSource;
/** Which failure kind, e.g. `INPUT_PASSWORD_PROTECTED`. An open string: the server adds kinds
* without waiting for a client that knows them. */
/** Open string, e.g. `INPUT_PASSWORD_PROTECTED`: the server adds kinds without a client change. */
kindId: string;
origin: NotificationOrigin;
ownership: NotificationOwnership;
@@ -63,12 +43,9 @@ export interface AppNotification {
titleKey: string;
defaultTitle: string;
detail: string | null;
/**
* Opaque reference to the document, resolvable only by the client that stored it. Two id spaces
* share this field, and `sourceId` says which: see `isResolvableHere` in `useNotifications`.
*/
/** Two id spaces share this field, and `sourceId` says which: see `isResolvableHere`. */
fileId: string | null;
/** Which folder, bucket or webhook fed the failing run, and null for an attended run. */
/** Which folder, bucket or webhook fed the run, and null for an attended one. */
sourceId: string | null;
policyId: string | null;
occurrences: number;
@@ -94,8 +71,7 @@ export interface FetchedNotifications {
/**
* Newest first. Empty rather than throwing: a bell that cannot load is an empty bell, not an error.
* {@code viewerReviewsTeam} defaults to true when absent, so a missing field never hides more than
* intended.
* `viewerReviewsTeam` defaults to true, so a missing field never hides more than intended.
*/
export async function fetchNotifications(
limit = 20,
frontend/editor/src/core/tests/helpers/api-stubs.ts
+2 -4
View File
@@ -274,10 +274,8 @@ export async function mockAppApis(
route.fulfill({ json: [] }),
);
// The notification bell polls GET /api/v1/notifications from every shell it
// is mounted in, starting on load. The hook swallows the failure and shows an
// empty bell, but the browser still logs the failed request itself, which the
// console-hygiene guard counts. Empty list, for the same reason as policies.
// The bell polls this on load. The hook swallows the failure, but the browser still logs the
// request, which the console-hygiene guard counts.
await page.route("**/api/v1/notifications*", (route: Route) =>
route.fulfill({ json: { notifications: [] } }),
);
frontend/editor/src/core/types/fileContext.ts
+2 -3
View File
@@ -330,9 +330,8 @@ export interface FileContextActions {
selectFiles?: boolean;
skipUploadTracking?: boolean;
/**
* Mark every added file as produced in-app rather than uploaded, which is what stops the
* policy auto-run enforcing an upload policy on it. Set by anything adding a file that has
* already been through a policy or a tool: policy output delivery, and the bell's unlock.
* Produced in-app rather than uploaded, which stops the policy auto-run enforcing an upload
* policy on it. Set by anything adding a file already through a policy or a tool.
*/
derivedFromTool?: boolean;
},
frontend/editor/src/portal/components/failures/FileRunEventList.tsx
+2 -4
View File
@@ -35,10 +35,8 @@ export function FileRunEventList() {
const section = useRef<HTMLElement>(null);
const { hash, key } = useLocation();
// Arriving from a notification, which links to this section by fragment. The browser only
// honours a fragment on a real page load, and this is a client-side route change, so scroll
// it into view here. Keyed on the navigation as well as the fragment: clicking a second
// notification while already parked here changes neither the path nor the hash.
// A fragment is only honoured on a real page load, not a client-side route change. Keyed on the
// navigation too: a second notification changes neither the path nor the hash.
useEffect(() => {
if (hash !== `#${PORTAL_FAILURES_ANCHOR}`) return;
section.current?.scrollIntoView({ behavior: "smooth", block: "start" });
frontend/editor/src/proprietary/components/notifications/notificationActions.test.tsx
+12 -21
View File
@@ -9,9 +9,8 @@ import type {
import type { NotificationActionContext } from "@core/components/notifications/notificationActions";
/**
* What this build can actually do about a failure, and where each action sends the reader. The bell hangs
* in the editor and in the processor, and only the editor has a file context above it, so the two shells
* are the interesting cases: selecting the document directly, or handing it over.
* Where each action sends the reader. Only the editor has the workbench contexts above it, so the two
* shells are the interesting cases: opening the document, or handing it over.
*/
const retryWithPassword = vi.fn();
@@ -46,8 +45,7 @@ vi.mock("react-router-dom", async () => ({
useNavigate: () => navigate,
}));
// No i18n instance is initialised here, so the real hook would return bare keys. The plugin is
// stubbed too because the workbench contexts below reach `core/i18n`, which registers it on import.
// No i18n instance here, and the plugin is stubbed because the contexts below reach `core/i18n`.
vi.mock("react-i18next", () => ({
useTranslation: () => ({
t: (_key: string, fallback: string) => fallback,
@@ -55,8 +53,7 @@ vi.mock("react-i18next", () => ({
initReactI18next: { type: "3rdParty", init: () => {} },
}));
// The document lookup reads IndexedDB, which jsdom has none of. Answered here so that whether the
// bytes are present is a fact of the test rather than of the environment.
// IndexedDB, which jsdom has none of. Answered here so presence is a fact of the test.
const h = vi.hoisted(() => ({
stub: { id: "f-1" } as unknown,
getStirlingFileStub: vi.fn(),
@@ -125,7 +122,6 @@ function notification(
};
}
/** An action the server offered, enabled: what it does with it is the client's decision. */
function offer(id: string): NotificationActionOffer {
return {
id,
@@ -306,8 +302,7 @@ describe("useNotificationActions", () => {
});
it("leaves View file as the only usable offer when the server offers actions this build cannot run", () => {
// The server can ship new kinds with new actions ahead of the clients that understand them, so
// an id this build wires nothing for drops out rather than rendering dead.
// An id this build wires nothing for drops out rather than rendering dead.
const actions = registry();
const usable = [offer("QUARANTINE"), offer("VIEW_FILE")].filter(
(candidate) => actions[candidate.id]?.available(context()) ?? false,
@@ -319,16 +314,14 @@ describe("useNotificationActions", () => {
it("opens the document into the viewer when an editor is above", async () => {
await registry().VIEW_FILE?.run(context());
// Selecting alone would show nothing: the workbench does not hold the file yet, and it keeps
// whatever view it was already on.
// Selecting alone shows nothing: the workbench holds neither the file nor the viewer yet.
expect(addStirlingFileStubs).toHaveBeenCalledWith([h.stub]);
expect(setActiveFileId).toHaveBeenCalledWith("f-1");
expect(setWorkbench).toHaveBeenCalledWith("viewer");
});
it("stays where it is rather than routing through the role-based root", async () => {
// "/" decides a landing page from the reader's role, so navigating there reads as the app
// reloading and can land them somewhere other than their document.
// "/" lands on a page chosen by the reader's role, which reads as the app reloading.
await registry().VIEW_FILE?.run(context());
expect(window.location.pathname).toBe("/");
@@ -358,11 +351,11 @@ describe("useNotificationActions", () => {
it("hands the document over to the editor when there is no workbench above it", async () => {
await registry(inProcessor).VIEW_FILE?.run(context());
// Nothing to open into, so the intent outlives the navigation that mounts the editor.
// The intent outlives the navigation that mounts the editor.
expect(
window.sessionStorage.getItem("stirling.notifications.pendingSelection"),
).toBe("f-1");
// The editor's own URL, not "/", which would hand the reader to the role router instead.
// The editor's own URL, not the role router at "/".
expect(window.location.pathname).toBe("/editor");
});
@@ -526,8 +519,7 @@ describe("useNotificationActions", () => {
});
it("says it cannot hand the document over rather than navigating to nothing", async () => {
// Storage refused, so nothing would be selected on arrival: the row reports it and stays put.
// On the prototype: jsdom's storage object is a proxy, so an own-property spy does not take.
// Spied on the prototype: jsdom's storage is a proxy, so an own-property spy does not take.
const setItem = vi
.spyOn(Storage.prototype, "setItem")
.mockImplementation(() => {
@@ -541,7 +533,7 @@ describe("useNotificationActions", () => {
message:
"This browser will not let the processor pass the document to the editor. Open it from the editor instead.",
});
// Still on the page it started on, so the failure is visible rather than mysterious.
// Still on the page it started on, so the failure is visible.
expect(window.location.pathname).toBe("/");
setItem.mockRestore();
});
@@ -566,8 +558,7 @@ describe("useNotificationActions", () => {
});
it("offers the processor link whenever the server did", () => {
// The server only sends it to someone it will let read the queue, so there is nothing left
// to gate here.
// The server only sends it to someone it will let read the queue.
expect(
registry(inProcessor).VIEW_IN_PROCESSOR?.available(
context({ hasLocalFile: false, retryPayload: null }),
frontend/editor/src/proprietary/components/notifications/notificationActions.ts
+16 -49
View File
@@ -55,37 +55,23 @@ export {
};
/**
* What this build can do about a failure notification: unlock the document and take the result, run the
* failing work again, open the tool that failed, or go to the incident in the processor. "Run it again"
* means two different things, so see {@link RetryTarget}.
* What this build can do about a failure notification: unlock the document and take the result, run
* the failing work again, open the tool that failed, or go to the incident in the processor.
*
* THE SHELL PROBLEM. The portal mounts as a sibling of the route that renders `AppProviders` (see
* `proprietary/App.tsx`), so in the processor shell there is no FileContext, ToolWorkflowContext or
* NavigationContext above this hook. Three things below are shaped by that and marked where they
* appear: file state is reached through the raw context so its absence is a value rather than a thrown
* error; a document the processor cannot select is handed over through session storage; and editor
* destinations are reached by pushing the URL and announcing it. All three go away when the portal
* route moves inside `AppProviders`.
* The portal mounts as a sibling of `AppProviders`, so in the processor shell none of the workbench
* contexts exist above this hook. That is why contexts are read raw and a document is handed over.
*/
/**
* The document a notification's actions are about, waiting for an editor to pick it up. Written only
* when there is no file context to select it directly (shell problem, point 2). Session-scoped and
* one-shot: it is a click the user just made, not state worth keeping.
*/
const HANDOFF_KEY = "stirling.notifications.pendingSelection";
/** The recorded-failures section of the processor, which is as precise as this link gets. */
const FAILURES_DESTINATION = `${PORTAL_BASENAME}/documents#${PORTAL_FAILURES_ANCHOR}`;
/** False when this browser will not store it, which the caller must not paper over. */
/** False when storage refused it: navigating anyway lands the user in an editor with nothing open. */
function stashSelection(fileId: string): boolean {
try {
window.sessionStorage.setItem(HANDOFF_KEY, fileId);
return true;
} catch {
// Private mode, or storage disabled. Navigating anyway would land the user in the editor with
// nothing selected and no idea why, so the caller reports it instead.
return false;
}
}
@@ -101,10 +87,8 @@ function takeSelection(): string | null {
}
/**
* Go to an editor path from either shell. Not the router's `navigate` (shell problem, point 3): the
* editor reads its tool out of the URL on mount and on a history pop, and a router push is neither, so
* the address would change and the workbench would not. Pushing plus announcing covers both readers,
* the same way `settingsNavigation` opens a settings section.
* Not the router's `navigate`: the editor reads its tool from the URL on mount and on a history pop,
* and a router push is neither, so the address would change and the workbench would not.
*/
function goToEditor(path: string): void {
window.history.pushState({}, "", withBasePath(path));
@@ -220,9 +204,8 @@ async function adopt(
export function useNotificationActions(): ClientActionRegistry {
const { t } = useTranslation();
const navigate = useNavigate();
// Raw contexts, not the hooks that wrap them (shell problem, point 1): in the processor shell
// there is no provider above the bell, and a hook that insists on one would take the whole panel
// down with it. All four are present together or not at all, which is what `canOpenHere` means.
// Raw, because the hooks that wrap these throw when there is no provider, and in the processor
// shell there is none. All four are present together or not at all.
const fileContext = useContext(FileActionsContext);
const fileStore = useContext(FileStoreContext);
const navigation = useContext(NavigationActionsContext);
@@ -233,13 +216,8 @@ export function useNotificationActions(): ClientActionRegistry {
const aiEnabled = useAiEngineEnabled();
/**
* Open a document the way the file sidebar does, rather than merely selecting it: a selected id
* that is not in the workbench shows nothing, and the workbench keeps whatever view it was on.
* So the stub is added if the workbench does not already hold it, made the active file, and the
* viewer brought to the front.
*
* False when this shell cannot do it, or when the document is no longer in storage: the caller
* turns that into the row's message instead of a navigation that appears to do nothing.
* Opens the way the file sidebar does. Selecting alone shows nothing: an id the workbench does not
* hold has nothing to render, and the workbench keeps whatever view it was on.
*/
const openInWorkbench = useCallback(
async (fileId: string): Promise<boolean> => {
@@ -261,8 +239,7 @@ export function useNotificationActions(): ClientActionRegistry {
[fileContext, fileStore, navigation, viewer],
);
// Pick up a document handed over by the other shell (shell problem, point 2). One-shot: the
// handoff is read and cleared, so a later render cannot reopen a file the user has moved on from.
// One-shot: read and cleared, so a later render cannot reopen a file the user has moved on from.
useEffect(() => {
if (!canOpenHere) return;
const fileId = takeSelection();
@@ -275,21 +252,13 @@ export function useNotificationActions(): ClientActionRegistry {
): Promise<ClientActionOutcome | void> => {
if (!fileId) return;
// Already in the shell that owns the workbench, so open it in place. Navigating as well
// would send the user through the role-based router at "/", which reads as the app
// reloading itself and lands them wherever their role says, not on their document.
// In place, with no navigation: "/" is the role-based router, so going there reads as the app
// reloading and lands the user wherever their role says rather than on their document.
if (canOpenHere) {
// No message of its own: the row falls back to "that did not work", which is the whole
// truth here. The document was in storage a moment ago or the button would not be on
// screen, so a failure now is a race rather than something the user can act on.
return (await openInWorkbench(fileId)) ? undefined : { ok: false };
}
// The processor shell has no workbench to open into, so hand the document over and go to
// the editor's own URL. EDITOR_BASENAME rather than "/" for the same reason as above.
if (!stashSelection(fileId)) {
// Nothing would be opened on arrival, so say so here rather than navigate to a page
// that looks like it worked.
return {
ok: false,
message: t(
@@ -505,10 +474,8 @@ export function useNotificationActions(): ClientActionRegistry {
};
const viewInProcessor: ClientActionSpec = {
// The server only offers this to someone it will let read the queue, so audience is already
// settled. What is left is whether the destination exists: the failures section it lands on is
// mounted in dev only until failures get their own review screen, so in a build this would
// navigate nowhere. Both gates lift together, and the other one is in portal/views/Documents.
// Its destination is dev-only until failures get a review screen; the other half of this gate
// is in portal/views/Documents, and both lift together.
available: () => import.meta.env.DEV,
closesPanel: true,
run: () => navigate(FAILURES_DESTINATION),
frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.chain.test.tsx
+4 -11
View File
@@ -91,8 +91,7 @@ describe("auto-run ordered chaining", () => {
// The first policy (order 0) runs on the upload; the second waits for the chain.
expect(runStored).toHaveBeenCalledTimes(1);
// The workspace id goes with the run: a failure of it is then recorded against a document this
// browser can resolve, which is what makes the notification about it actionable.
// Recorded against a document this browser can resolve, which is what makes its failure actionable.
expect(runStored).toHaveBeenCalledWith(
"backend-sec",
[{ size: 100 }],
@@ -126,8 +125,7 @@ describe("auto-run ordered chaining", () => {
await vi.advanceTimersByTimeAsync(1);
});
// The next policy (order 1) fires on the first policy's output, not the original - and reports
// that output's own workspace id, since that is the document now in front of the user.
// Fires on the first policy's output and reports that output's own id, not the original's.
expect(runStored).toHaveBeenCalledWith(
"backend-cls",
[{ size: 100 }],
@@ -160,13 +158,8 @@ describe("auto-run ordered chaining", () => {
});
it("never dispatches on a file marked derivedFromTool", async () => {
// THE GATE OTHER CODE RELIES ON. A file added programmatically after it has already been through a
// policy or a tool carries this flag, and this effect must leave it alone. A policy run is a BILLED
// automation run, so both callers charge the customer for work nobody asked for if this stops
// holding: policy output delivery (`importOutputs`) would re-enforce a policy on its own output
// forever, and the bell's "Decrypt and retry" (`notificationActions.adopt`) would have the adoption
// of an unlocked document fire the whole upload chain on it. If this test fails, fix the gate rather
// than the test.
// A policy run is billed, so this gate is what stops `importOutputs` re-enforcing a policy on
// its own output forever. If this fails, fix the gate rather than the test.
setFileStubs([
{ id: "file-1", name: "unlocked.pdf", derivedFromTool: true },
]);
frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.ts
+3 -5
View File
@@ -251,8 +251,7 @@ export function usePolicyAutoRun(): void {
dispatchKey(finished.categoryId, finished.fileId),
);
}
// A run that failed has just recorded an incident against the person watching it, so read the
// list rather than leaving them to wait out a poll interval for news of their own upload.
// Read now rather than leaving them a poll interval to hear about their own upload.
if (view.status === "FAILED") refreshNotificationsNow();
const code = view.errorCode;
if (code !== "PAYG_LIMIT_REACHED" && code !== "FEATURE_DEGRADED") return;
@@ -933,9 +932,8 @@ async function runPolicyOnFile(
await acquireDispatchSlot(priority);
try {
const target = resolvePolicyRunTarget();
// Hand the workspace id over with the run, so a failure of it is recorded against a document this
// browser can resolve and the notification can offer to open or unlock it. One file per run here,
// which is the only shape the server records a reference for.
// Recorded against a document this browser can resolve. One file per run, which is the only
// shape the server keeps a reference for.
const runId = await runStoredPolicy(backendId, [file], fileId);
// recordRunStart marks this (policy, file) dispatched as it records the run.
recordRunStart({
frontend/editor/src/proprietary/services/policyApi.test.ts
+2 -3
View File
@@ -1,9 +1,8 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
/**
* What the run request puts on the wire. Only the document reference is pinned, because it is the one
* field with a rule attached: the server records it against any failure of the run, so a filename here
* would put a document name into a table that deliberately has nowhere to keep one.
* Only the document reference is pinned, being the one field with a rule attached: a filename here
* would reach a table that deliberately has nowhere to keep one.
*/
const post = vi.fn().mockResolvedValue({ data: { jobId: "run-1" } });
frontend/editor/src/proprietary/services/policyApi.ts
+2 -5
View File
@@ -61,11 +61,8 @@ export async function reorderPolicies(orderedIds: string[]): Promise<void> {
}
/**
* Run a stored policy by id on the supplied files; returns the run id.
*
* `fileId` is this workspace's own opaque id for the document being run. The server records it against
* any failure of the run, which is the only way an attended failure can name a document this browser can
* resolve. Only honoured for a single-document run, and never a filename.
* Run a stored policy by id; returns the run id. `fileId` is this workspace's own opaque id, recorded
* against any failure of the run. Only honoured for a single-document run, and never a filename.
*/
export async function runStoredPolicy(
id: string,