review: fix what the pre-review pass surfaced

Functional fixes: - A batch run's success no longer resolves rows for files it silently failed: the continuation's tool arm now requires the row's file to have produced an output, the same proof the policy arm already asked for. Being an input of a successful run proves nothing on its own. - The action registry's useMemo takes aiEnabled as a dependency, so a decrypt-and-retry no longer rejoins the upload chain computed with the pre-load value and drops the AI policies. - The withheld reason can no longer come from an action this build has never heard of: promoteActions takes the build's knowledge as its own predicate, restoring the parent PR's reviewed rule. - A single-file endpoint's retry sends only the row's own document, not the whole stashed batch in one request the server would silently truncate to its first file; a ZIP answer is unpacked rather than adopted as one PDF; and the stash records which endpoint shape it was. - The stash also records the failure's error code, and both consumers refuse a stash another kind's failure wrote: the server keys incidents on kind as well as file, the stash never did, so the newest failure could hand its operation to an older row. - Custom-processor tools are not stashed: a generic re-submission would bypass their endpoint-specific request building, and the PR already claims they get no retry. Builds without notifications stash nothing at all, for the same reason they mount no bell. - A row with no runnable action keeps its error log: the overflow menu no longer hides behind the primary button. Housekeeping: - The retry service reports failure reasons and the component layer words them, so its copy is translated like everything else's. - The inline more-options icon becomes LocalIcon's own. - localFilePresence.ts deleted: notificationRetry.ts is its successor and the merge had left both alive. - Stale comments corrected (RESOLVED is set now; the clipboard fallback; the one-source-today anchor) and a duplicated TODO dropped.
2026-09-03 05:10:16 +03:00 · 2026-08-25 14:08:26 +01:00
parent 2f3c116c2a
commit ce8068ae3b
16 changed files with 327 additions and 143 deletions
@@ -4,8 +4,8 @@ import java.util.Arrays;
 import java.util.List;

 /**
- * Disposition of one recorded failure. {@code RESOLVED} is declared but not set yet (it becomes
- * system-set later); the rollup already defines what a repeat means for it, which is to reopen.
+ * Disposition of one recorded failure. {@code RESOLVED} is system-set when a client reports its own
+ * retry worked, never dispatched as an action; a repeat reopens it.
 */
 public enum FileRunEventStatus {
    NEW(false),
@@ -93,7 +93,8 @@ class NotificationResolveTest {
    @Test
    void theRowsOwnIdIsNotANotificationId() {
        // This mirror exists so no client has to strip the prefix, so an unprefixed id is
-        // refused rather than working by accident because there is only one source today.
+        // refused outright rather than left to work by accident for whichever source it
+        // would happen to reach.
        FileRunEvent event = given(FailureKind.UNKNOWN, ACTOR, "f-1");

        assertThat(statusOf(() -> controller.resolved(event.id())))
@@ -546,7 +546,12 @@ describe("NotificationBell", () => {
    expect(
      screen.queryByRole("button", { name: /VIEW_IN_PROCESSOR|VIEW_FILE/ }),
    ).toBeNull();
-    expect(document.querySelector(".notification-bell__actions")).toBeNull();
+    // The error log stays reachable: a row with nothing left to do still owns its detail.
+    expect(
+      screen.getByRole("button", {
+        name: "More options: Unrecognised failure",
+      }),
+    ).toBeTruthy();
  });

  it("asks for the password in the unlock modal before it retries", async () => {
@@ -3,6 +3,7 @@ import type { TFunction } from "i18next";
 import { useTranslation } from "react-i18next";
 import { Menu, Tooltip } from "@mantine/core";
 import { ActionIcon, Button } from "@app/ui";
+import LocalIcon from "@app/components/shared/LocalIcon";
 import { isResolvableHere } from "@app/hooks/useNotifications";
 import type { NotificationDocumentState } from "@app/hooks/useNotifications";
 import type {
@@ -102,6 +103,9 @@ export function NotificationItem({
      if (!spec) return false;
      return spec.available(context);
    },
+    // The withheld reason may come from an action this device cannot perform right now, but
+    // never from one this build could not have rendered at all.
+    (offer) => registry[offer.id] !== undefined,
  );

  const labelOf = (offer: NotificationActionOffer) =>
@@ -142,7 +146,7 @@ export function NotificationItem({
      await navigator.clipboard.writeText(notification.detail);
      setCopied(true);
    } catch {
-      // No clipboard permission, and the message is on screen and selectable anyway.
+      // No clipboard permission. Nothing worth an error of its own: the copy simply stays unoffered.
    }
  };

@@ -175,16 +179,19 @@ export function NotificationItem({
      {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
-          out of the way so a row of near-equal buttons never competes for the click. */}
-      {primary && (
+          out of the way so a row of near-equal buttons never competes for the click. The menu is not
+          gated on a button existing: a row with no runnable action still owns its error log. */}
+      {(primary || notification.detail) && (
        <span className="notification-bell__actions">
-          <ActionButton
-            variant="primary"
-            rowTitle={title}
-            label={labelOf(primary)}
-            busy={busy === primary.id}
-            onRun={() => void run(primary)}
-          />
+          {primary && (
+            <ActionButton
+              variant="primary"
+              rowTitle={title}
+              label={labelOf(primary)}
+              busy={busy === primary.id}
+              onRun={() => void run(primary)}
+            />
+          )}
          {secondary && (
            <ActionButton
              variant="secondary"
@@ -207,7 +214,7 @@ export function NotificationItem({
                    className="notification-bell__more"
                    aria-label={`${t("notifications.action.more", "More options")}: ${title}`}
                  >
-                    <MoreIcon />
+                    <LocalIcon icon="more-horiz" width={14} height={14} />
                  </ActionIcon>
                </Tooltip>
              </Menu.Target>
@@ -277,25 +284,3 @@ function ActionButton({
    </Button>
  );
 }
-
-const ICON_PROPS = {
-  width: 14,
-  height: 14,
-  viewBox: "0 0 24 24",
-  fill: "none",
-  stroke: "currentColor",
-  strokeWidth: 2,
-  strokeLinecap: "round" as const,
-  strokeLinejoin: "round" as const,
-  "aria-hidden": true,
-};
-
-function MoreIcon() {
-  return (
-    <svg {...ICON_PROPS} strokeWidth={2.5}>
-      <circle cx="5" cy="12" r="0.5" />
-      <circle cx="12" cy="12" r="0.5" />
-      <circle cx="19" cy="12" r="0.5" />
-    </svg>
-  );
-}
@@ -94,10 +94,15 @@ const RUNNABLE = new Set([
 /** The predicate the bell supplies: a known id, on a device that can act on it. */
 const canRun = (action: NotificationActionOffer) => RUNNABLE.has(action.id);

+/** The build's knowledge alone, which is what gates a withheld reason. */
+const knowsAction = (action: NotificationActionOffer) =>
+  RUNNABLE.has(action.id);
+
 function promoted(list: NotificationActionOffer[]) {
  const { primary, secondary, overflow, withheldReasonKey } = promoteActions(
    list,
    canRun,
+    knowsAction,
  );
  return {
    primary: primary?.id ?? null,
@@ -224,6 +229,7 @@ describe("promoteActions", () => {
    const { primary, secondary, overflow } = promoteActions(
      password("DECRYPT_AND_RETRY", "RETRY", "VIEW_FILE", "VIEW_IN_PROCESSOR"),
      inProcessor,
+      knowsAction,
    );

    expect(primary?.id).toBe("VIEW_IN_PROCESSOR");
@@ -238,6 +244,7 @@ describe("promoteActions", () => {
    const { primary, overflow, withheldReasonKey } = promoteActions(
      unknown("RETRY", "VIEW_FILE"),
      () => false,
+      knowsAction,
    );

    expect(primary).toBeNull();
@@ -258,7 +265,7 @@ describe("promoteActions", () => {
  });

  it("has nothing to promote when nothing survives", () => {
-    expect(promoteActions([], () => true)).toEqual({
+    expect(promoteActions([], () => true, () => true)).toEqual({
      primary: null,
      secondary: null,
      overflow: [],
@@ -266,6 +273,25 @@ describe("promoteActions", () => {
    });
  });

+  it("never explains the row with an action this build has never heard of", () => {
+    // The server ships a new action, disabled with a reason, to a client that predates it.
+    // That client could never have drawn the button, so the reason is not its row's story.
+    const list = [
+      offer("QUARANTINE", "RESOLUTION", {
+        enabled: false,
+        disabledReasonKey: NO_DOCUMENT,
+      }),
+      ...unknown("VIEW_IN_PROCESSOR"),
+    ];
+
+    expect(promoted(list)).toEqual({
+      primary: "VIEW_IN_PROCESSOR",
+      secondary: null,
+      overflow: [],
+      withheldReasonKey: null,
+    });
+  });
+
  it("takes the reason from the best action lost, not the first declared", () => {
    // Two refusals, one row: the reader gets the one attached to the action they would have reached
    // for first.
@@ -34,7 +34,9 @@ export interface PromotedActions {
 *
 * Every offer the bell is given is one this client runs itself, so each is asked past
 * `canRenderClientAction`: whether this build knows the id, and whether this device can currently
- * perform it.
+ * perform it. `knowsAction` asks only the first half, and gates the withheld reason: an action this
+ * device cannot perform right now still explains the row, but one this build has never heard of
+ * cannot - a reason about a button that could never have been drawn is not this row's explanation.
 *
 * A dropped action leaves no hole, and a disabled one is dropped too: a button that can never work is
 * false hope. Its reason comes back instead, for the row to say in words.
@@ -42,6 +44,7 @@ export interface PromotedActions {
 export function promoteActions(
  offers: readonly NotificationActionOffer[],
  canRenderClientAction: (offer: NotificationActionOffer) => boolean,
+  knowsAction: (offer: NotificationActionOffer) => boolean,
 ): PromotedActions {
  const ranked = offers
    .map((offer, declaredAt) => ({ offer, declaredAt }))
@@ -55,8 +58,9 @@ export function promoteActions(

  // The best one withheld, so a row explains itself once rather than once per lost action.
  const withheldReasonKey =
-    ranked.find((offer) => !offer.enabled && offer.disabledReasonKey)
-      ?.disabledReasonKey ?? null;
+    ranked.find(
+      (offer) => !offer.enabled && offer.disabledReasonKey && knowsAction(offer),
+    )?.disabledReasonKey ?? null;

  const renderable = ranked.filter(
    (offer) => offer.enabled && canRenderClientAction(offer),
@@ -22,12 +22,14 @@ import {
 } from "@app/types/fileContext";
 import { FILE_EVENTS } from "@app/services/errorUtils";
 import {
+  errorCodeOf,
  reportToolFailure,
  wasCancelled,
 } from "@app/services/failureReporting";
 import { stashRetryPayload } from "@app/services/notificationRetry";
 import { refreshNotificationsNow } from "@app/hooks/useNotifications";
 import { useResolutionContinuation } from "@app/hooks/tools/shared/useResolutionContinuation";
+import { useNotificationsAvailable } from "@app/components/notifications/useNotificationsAvailable";
 import { zipFileService } from "@app/services/zipFileService";
 import { getFilenameWithoutExtension } from "@app/utils/fileUtils";
 import {
@@ -127,6 +129,7 @@ export const useToolOperation = <TParams>(
  const { checkCredits } = useCreditCheck(config.operationType, endpointString);
  const willUseCloud = useWillUseCloud(endpointString);
  const continueResolutions = useResolutionContinuation();
+  const notificationsAvailable = useNotificationsAvailable();

  // Track last operation for undo functionality
  const lastOperationRef = useRef<{
@@ -646,16 +649,28 @@ export const useToolOperation = <TParams>(

        // Keep what a retry would need, since the report itself carries no
        // operation and answers 204. Gated on the reporter's own cancellation
-        // test so the two cannot disagree about what counts as a failure, and on
-        // there being an endpoint: a custom processor has nothing to re-submit to.
-        if (!wasCancelled(error) && runtimeEndpoint) {
-          void stashRetryPayload({
-            operation: config.operationType,
-            endpoint: runtimeEndpoint,
-            params: params as Record<string, unknown>,
-            fileIds: validFiles.map((file) => file.fileId),
-            recordedAt: Date.now(),
-          });
+        // test so the two cannot disagree about what counts as a failure; on the
+        // tool not being a custom processor, whose endpoint-specific request
+        // building a generic re-submission would bypass; and on this build having
+        // notifications at all, so a build with no bell does not fill a stash
+        // nothing can ever read.
+        if (
+          !wasCancelled(error) &&
+          runtimeEndpoint &&
+          config.toolType !== ToolType.custom &&
+          notificationsAvailable
+        ) {
+          void errorCodeOf(error).then((errorCode) =>
+            stashRetryPayload({
+              operation: config.operationType,
+              endpoint: runtimeEndpoint,
+              params: params as Record<string, unknown>,
+              fileIds: validFiles.map((file) => file.fileId),
+              multiFile: config.toolType === ToolType.multiFile,
+              errorCode,
+              recordedAt: Date.now(),
+            }),
+          );
        }

        const errorMessage =
@@ -686,6 +701,7 @@ export const useToolOperation = <TParams>(
      willUseCloud,
      checkCredits,
      continueResolutions,
+      notificationsAvailable,
    ],
  );

@@ -17,7 +17,6 @@ import {
 * 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";

@@ -1,36 +0,0 @@
-import { beforeEach, describe, expect, it, vi } from "vitest";
-import "fake-indexeddb/auto";
-
-/**
- * Tests for the one thing the bell asks about a failed document here: whether it is
- * still in this browser, which is what decides if it can be opened.
- */
-
-const getStirlingFileStub = vi.fn();
-
-vi.mock("@app/services/fileStorage", () => ({
-  fileStorage: {
-    getStirlingFileStub: (...args: unknown[]) => getStirlingFileStub(...args),
-  },
-}));
-
-const { hasLocalFile } = await import("@app/services/localFilePresence");
-
-beforeEach(() => {
-  getStirlingFileStub.mockReset().mockResolvedValue(null);
-});
-
-describe("hasLocalFile", () => {
-  it("is false once the document has left this browser", async () => {
-    getStirlingFileStub.mockResolvedValue(null);
-
-    await expect(hasLocalFile("f-1")).resolves.toBe(false);
-    await expect(hasLocalFile(null)).resolves.toBe(false);
-  });
-
-  it("is true while the document is still stored here", async () => {
-    getStirlingFileStub.mockResolvedValue({ id: "f-1", name: "doc.pdf" });
-
-    await expect(hasLocalFile("f-1")).resolves.toBe(true);
-  });
-});
@@ -1,18 +0,0 @@
-import { fileStorage } from "@app/services/fileStorage";
-import type { FileId } from "@app/types/file";
-
-/** Whether the document is still in this browser. The id is this workspace's own, so only it can say. */
-export async function hasLocalFile(fileId: string | null): Promise<boolean> {
-  if (!isUsableId(fileId)) return false;
-
-  try {
-    const stub = await fileStorage.getStirlingFileStub(fileId as FileId);
-    return stub !== null;
-  } catch {
-    return false;
-  }
-}
-
-function isUsableId(fileId: string | null | undefined): fileId is string {
-  return typeof fileId === "string" && fileId.trim() !== "";
-}
@@ -41,6 +41,8 @@ function payload(overrides: Partial<Record<string, unknown>> = {}) {
    endpoint: "/api/v1/security/remove-password",
    params: {},
    fileIds: ["f-1"],
+    multiFile: false,
+    errorCode: "E004",
    recordedAt: 1_000,
    ...overrides,
  } as Parameters<typeof stashRetryPayload>[0];
@@ -82,6 +84,8 @@ describe("the retry stash", () => {
      endpoint: "/api/v1/security/remove-password",
      params: { onlyPages: "1-3" },
      fileIds: ["f-1", "f-2"],
+      multiFile: false,
+      errorCode: "E004",
      recordedAt: 1_000,
    });
    // Every file in the run gets a record, so the bell can retry from any of them.
@@ -213,7 +217,8 @@ describe("retryWithPassword", () => {
    const result = await retryWithPassword(payload(), "hunter2");

    expect(result.ok).toBe(false);
-    expect(result.message).toBeTruthy();
+    // The reason, not words: the component layer owns the wording, having `t`.
+    expect(result.reason).toBe("fileMissing");
    expect(post).not.toHaveBeenCalled();
  });

@@ -313,7 +318,7 @@ describe("unlockLocalDocument", () => {
    const result = await unlockLocalDocument("f-1", "hunter2");

    expect(result.ok).toBe(false);
-    expect(result.message).toBeTruthy();
+    expect(result.reason).toBe("fileMissing");
    expect(post).not.toHaveBeenCalled();
  });

@@ -4,6 +4,7 @@ import {
  indexedDBManager,
  type DatabaseConfig,
 } from "@app/services/indexedDBManager";
+import { zipFileService } from "@app/services/zipFileService";
 import type { FileId } from "@app/types/file";
 import type { ToolEndpoint } from "@app/types/toolApiTypes";

@@ -11,8 +12,9 @@ import type { ToolEndpoint } from "@app/types/toolApiTypes";
 * What the notification bell needs to offer "Retry" or "Decrypt and retry" on a
 * failure the editor reported. The server keeps none of it: the report drops the
 * operation and answers 204, so this lives here, keyed on the opaque `fileId` it was
- * filed against. Last-write-wins per fileId, matching the server's actor|kind|file
- * dedup: one file failing two operations is one incident with one retry button.
+ * filed against. Last-write-wins per fileId, which is narrower than the server's
+ * dedup (that one also keys on the failure kind): see {@link stashMatchesKind} for
+ * how a consumer tells whether the surviving stash belongs to a given row.
 */
 export interface RetryPayload {
  /** tool/endpoint identifier, e.g. "remove-password" */
@@ -22,9 +24,41 @@ export interface RetryPayload {
  /** the tool parameters as submitted */
  params: Record<string, unknown>;
  fileIds: string[];
+  /** Whether the endpoint takes the whole batch in one call, or one file per call. */
+  multiFile: boolean;
+  /** The failure's error code, so a stash can be matched to the row's kind. */
+  errorCode: string | null;
  recordedAt: number;
 }

+/**
+ * The error codes the named failure kinds claim, mirrored from the server's
+ * {@code FailureKind} declarations. Used only to tell whether the one stash a file
+ * carries belongs to a given row: the server keys incidents on kind as well as file,
+ * so one file can have two open rows while this stash holds only the newest failure.
+ */
+const KIND_ERROR_CODES: Record<string, string> = {
+  INPUT_PASSWORD_PROTECTED: "E004",
+};
+
+/**
+ * Whether a stashed failure is the one a row of this kind describes. A named kind
+ * owns exactly its claimed code; every other kind owns whatever no named kind
+ * claims. A stash written before {@code errorCode} existed matches nothing named,
+ * failing closed rather than retrying the wrong operation.
+ */
+export function stashMatchesKind(
+  kindId: string,
+  payload: RetryPayload,
+): boolean {
+  const claimed = KIND_ERROR_CODES[kindId];
+  if (claimed) return payload.errorCode === claimed;
+  return (
+    payload.errorCode === null ||
+    !Object.values(KIND_ERROR_CODES).includes(payload.errorCode)
+  );
+}
+
 /**
 * Its own database rather than a store on `stirling-pdf-files`: that schema has
 * shipped at v9, and adding a store there means a version bump plus an upgrade path
@@ -99,6 +133,10 @@ export async function loadRetryPayload(
    endpoint: record.endpoint,
    params: record.params ?? {},
    fileIds: record.fileIds ?? [fileId],
+    // Older records predate these fields; both defaults fail closed (one file per
+    // call, matching no named kind).
+    multiFile: record.multiFile ?? false,
+    errorCode: record.errorCode ?? null,
    recordedAt: record.recordedAt,
  };
 }
@@ -124,10 +162,21 @@ export interface RetryOutputFile {
  filename: string;
 }

+/**
+ * Why a retry could not run, for the component layer to word: the wording belongs
+ * up there, which has `t`. `serverMessage` means {@link PasswordRetryOutcome.message}
+ * carries the server's own words, which pass through untranslated on purpose.
+ */
+export type PasswordRetryFailure =
+  | "notRetryable"
+  | "fileMissing"
+  | "serverMessage";
+
 /** What a password-carrying call comes back with. `files` only ever on success. */
 export interface PasswordRetryOutcome {
  ok: boolean;
-  message?: string;
+  reason?: PasswordRetryFailure;
+  message?: string | null;
  files?: RetryOutputFile[];
 }

@@ -152,30 +201,34 @@ export async function unlockLocalDocument(
 * what it produced. The password is appended to a single request and then out of
 * scope: never stashed, never logged, never in the message returned here.
 *
+ * A single-file endpoint was called once per file by the original run, so the retry
+ * sends only `forFileId`, the document the row is about; a multi-file endpoint gets
+ * the whole stashed batch back, exactly as it was submitted.
+ *
 * `files` is returned rather than adopted because every file operation goes through
 * FileContext, which a service cannot reach.
 */
 export async function retryWithPassword(
  payload: RetryPayload,
  password: string,
+  forFileId: string | null = null,
 ): Promise<PasswordRetryOutcome> {
  if (!payload.endpoint) {
-    return { ok: false, message: "This operation cannot be retried." };
+    return { ok: false, reason: "notRetryable", message: null };
  }

-  return postWithPassword(
-    payload.endpoint,
-    payload.params,
-    payload.fileIds,
-    password,
-  );
+  const fileIds = payload.multiFile
+    ? payload.fileIds
+    : [forFileId && payload.fileIds.includes(forFileId) ? forFileId : payload.fileIds[0]];
+
+  return postWithPassword(payload.endpoint, payload.params, fileIds, password);
 }

 /** Shared by both callers above, so a password reaches the network from one place only. */
 async function postWithPassword(
  endpoint: string,
  params: Record<string, unknown>,
-  requestedFileIds: string[],
+  requestedFileIds: (string | null | undefined)[],
  password: string,
 ): Promise<PasswordRetryOutcome> {
  const fileIds = requestedFileIds.filter(isUsableId);
@@ -189,11 +242,7 @@ async function postWithPassword(
  // getStirlingFiles drops what it cannot find, so a short result means an input is
  // gone. Resolved rather than thrown: the caller shows this next to the notification.
  if (files.length === 0 || files.length !== fileIds.length) {
-    return {
-      ok: false,
-      message:
-        "This file is no longer stored in this browser, so it cannot be retried here.",
-    };
+    return { ok: false, reason: "fileMissing", message: null };
  }

  try {
@@ -204,18 +253,39 @@ async function postWithPassword(
    });
    return {
      ok: true,
-      files: [
-        {
-          blob: response.data,
-          filename: filenameOf(response.headers, files[0].name),
-        },
-      ],
+      files: await asOutputFiles(
+        response.data,
+        filenameOf(response.headers, files[0].name),
+      ),
    };
  } catch (error) {
-    return { ok: false, message: messageOf(error) };
+    return { ok: false, reason: "serverMessage", message: messageOf(error) };
  }
 }

+/**
+ * The response as adoptable documents. A multi-output run answers with a ZIP, which
+ * must not land in the workbench pretending to be one PDF, so it is unpacked here
+ * the same way the tool pipeline unpacks it.
+ */
+async function asOutputFiles(
+  blob: Blob,
+  filename: string,
+): Promise<RetryOutputFile[]> {
+  if (await zipFileService.isZipResponse(blob)) {
+    const extracted = await zipFileService.extractPdfFiles(
+      new File([blob], filename),
+    );
+    if (extracted.success && extracted.extractedFiles.length > 0) {
+      return extracted.extractedFiles.map((file) => ({
+        blob: file,
+        filename: file.name,
+      }));
+    }
+  }
+  return [{ blob, filename }];
+}
+
 /**
 * The name the server gave the output, falling back to the input's: a caller adopting an
 * unnamed blob would put a file called "blob" in the user's workbench.
@@ -314,15 +384,16 @@ function prunedBelow(value: unknown, depth: number): unknown {
  return kept;
 }

-/** What the user saw. Never carries the password: it is not interpolated here. */
-function messageOf(error: unknown): string {
+/**
+ * What the server said, or null when it said nothing usable (the caller words that
+ * case itself). Never carries the password: it is not interpolated here.
+ */
+function messageOf(error: unknown): string | null {
  const response = (error as { response?: { data?: unknown } })?.response?.data;
  if (typeof response === "string" && response.trim() !== "") return response;

  const message = (error as { message?: unknown })?.message;
-  return typeof message === "string" && message.trim() !== ""
-    ? message
-    : "Retrying the operation failed.";
+  return typeof message === "string" && message.trim() !== "" ? message : null;
 }

 async function writeRecords(records: StoredRetryRecord[]): Promise<void> {
@@ -15,7 +15,12 @@ import type { NotificationActionContext } from "@core/components/notifications/n

 const retryWithPassword = vi.fn();
 const unlockLocalDocument = vi.fn();
-vi.mock("@app/services/notificationRetry", () => ({
+vi.mock("@app/services/notificationRetry", async (importOriginal) => ({
+  // The real stashMatchesKind: it is pure, and the guard it implements is part of
+  // what these tests exercise.
+  ...(await importOriginal<
+    typeof import("@app/services/notificationRetry")
+  >()),
  retryWithPassword: (...args: unknown[]) => retryWithPassword(...args),
  unlockLocalDocument: (...args: unknown[]) => unlockLocalDocument(...args),
 }));
@@ -144,6 +149,8 @@ function context(
      endpoint: "/api/v1/security/remove-password",
      params: {},
      fileIds: ["f-1"],
+      multiFile: false,
+      errorCode: "E004",
      recordedAt: 0,
    },
    ...overrides,
@@ -279,6 +286,8 @@ describe("useNotificationActions", () => {
          endpoint: "/api/v1/quarantine",
          params: {},
          fileIds: ["f-1"],
+          multiFile: false,
+          errorCode: "E004",
          recordedAt: 0,
        },
      }),
@@ -386,6 +395,8 @@ describe("useNotificationActions", () => {
    expect(retryWithPassword).toHaveBeenCalledWith(
      expect.objectContaining({ endpoint: "/api/v1/security/remove-password" }),
      "hunter2",
+      // The row's own document, so a single-file endpoint is not handed the whole batch.
+      "f-1",
    );
    expect(outcome).toEqual({ ok: false, message: "Wrong" });
  });
@@ -666,6 +677,8 @@ describe("retrying an attended policy run", () => {
          endpoint: "/api/v1/security/remove-password",
          params: {},
          fileIds: ["f-1"],
+          multiFile: false,
+          errorCode: "E004",
          recordedAt: 0,
        },
      }),
@@ -18,7 +18,9 @@ import { EDITOR_BASENAME } from "@app/routes/editorBasename";
 import { fileStorage } from "@app/services/fileStorage";
 import {
  retryWithPassword,
+  stashMatchesKind,
  unlockLocalDocument,
+  type PasswordRetryOutcome,
  type RetryOutputFile,
  type RetryPayload,
 } from "@app/services/notificationRetry";
@@ -142,7 +144,11 @@ function retryTargetOf(context: NotificationActionContext): RetryTarget | null {
    };
  }

-  return retryPayload ? { kind: "tool", payload: retryPayload } : null;
+  // The stash is one record per file while the server keeps one incident per kind per
+  // file, so a stash written by a different kind's failure is not this row's to run.
+  return retryPayload && stashMatchesKind(notification.kindId, retryPayload)
+    ? { kind: "tool", payload: retryPayload }
+    : null;
 }

 /** The documents a password-carrying call produced, as files the workbench can take. */
@@ -310,6 +316,27 @@ export function useNotificationActions(): ClientActionRegistry {
      ),
    });

+    /**
+     * A failed unlock in the reader's words. The service reports why and this layer words it,
+     * because the wording belongs where `t` lives; only the server's own message passes through.
+     */
+    const unlockFailure = (
+      outcome: PasswordRetryOutcome,
+    ): ClientActionOutcome => {
+      if (outcome.reason === "fileMissing") {
+        return {
+          ok: false,
+          message: t(
+            "notifications.notOnThisDevice",
+            "This document is not on this device, so it cannot be opened or retried here.",
+          ),
+        };
+      }
+      if (outcome.reason === "notRetryable") return unavailable();
+      // The server's own words, or nothing: the row falls back to its generic failure line.
+      return { ok: false, message: outcome.message ?? undefined };
+    };
+
    /**
     * What a policy re-run amounted to, in the reader's terms. A rejection after the unlock reads
     * differently from one before it, so the reader is not left thinking their password was wrong.
@@ -412,10 +439,14 @@ export function useNotificationActions(): ClientActionRegistry {
        // endpoint for a policy, since a locked input is fixed the same way whatever was reading it.
        const outcome =
          target.kind === "tool"
-            ? await retryWithPassword(target.payload, password)
+            ? await retryWithPassword(
+                target.payload,
+                password,
+                context.notification.fileId,
+              )
            : await unlockLocalDocument(target.policy.fileId, password);
        // A wrong password lands here, carrying the server's own words, which the row shows.
-        if (!outcome.ok) return outcome;
+        if (!outcome.ok) return unlockFailure(outcome);

        // It unlocked, so the user must end up holding it. A failed adoption fails the whole action:
        // claiming success and dropping the result leaves them nothing for the password they typed.
@@ -501,5 +532,5 @@ export function useNotificationActions(): ClientActionRegistry {
      VIEW_FILE: viewFile,
      VIEW_IN_PROCESSOR: viewInProcessor,
    };
-  }, [canOpenHere, openInWorkbench, fileContext, fileStore, navigate, t]);
+  }, [aiEnabled, canOpenHere, openInWorkbench, fileContext, fileStore, navigate, t]);
 }
@@ -27,7 +27,11 @@ vi.mock("@app/hooks/useNotifications", () => ({
 }));

 const loadRetryPayload = vi.fn();
-vi.mock("@app/services/notificationRetry", () => ({
+vi.mock("@app/services/notificationRetry", async (importOriginal) => ({
+  // The real stashMatchesKind: pure, and part of the behaviour under test.
+  ...(await importOriginal<
+    typeof import("@app/services/notificationRetry")
+  >()),
  loadRetryPayload: (fileId: string) => loadRetryPayload(fileId),
 }));

@@ -370,6 +374,74 @@ describe("useResolutionContinuation", () => {
    expect(reportNotificationResolved).not.toHaveBeenCalled();
  });

+  it("does not resolve a file its batch run failed for", async () => {
+    // A batch can succeed for one input and fail for another without ever reaching the
+    // failure path. Being an input of a successful run proves nothing; producing an
+    // output does.
+    fetchNotifications.mockResolvedValue({
+      notifications: [toolRow()],
+      viewerReviewsTeam: false,
+    });
+    loadRetryPayload.mockResolvedValue({
+      operation: "compress",
+      endpoint: "/api/v1/misc/compress-pdf",
+      params: {},
+      fileIds: ["f-tool", "f-other"],
+      multiFile: false,
+      errorCode: null,
+      recordedAt: 0,
+    });
+
+    continuation()({
+      operation: "compress",
+      inputFileIds: ["f-tool", "f-other"],
+      // Only the other file produced an output; f-tool failed again, silently.
+      outputs: [
+        {
+          file: new File(["pdf"], "other.pdf"),
+          fileId: "f-other-out",
+          sourceFileId: "f-other",
+        },
+      ],
+    });
+
+    await waitFor(() => expect(fetchNotifications).toHaveBeenCalled());
+    expect(reportNotificationResolved).not.toHaveBeenCalled();
+  });
+
+  it("does not resolve a row whose stash another kind's failure wrote", async () => {
+    // One stash per file, one incident per kind per file: the password failure's stash
+    // overwrote the compress one, so the compress row may not be resolved against it.
+    fetchNotifications.mockResolvedValue({
+      notifications: [toolRow()],
+      viewerReviewsTeam: false,
+    });
+    loadRetryPayload.mockResolvedValue({
+      operation: "removePassword",
+      endpoint: "/api/v1/security/remove-password",
+      params: {},
+      fileIds: ["f-tool"],
+      multiFile: false,
+      errorCode: "E004",
+      recordedAt: 0,
+    });
+
+    continuation()({
+      operation: "removePassword",
+      inputFileIds: ["f-tool"],
+      outputs: [
+        {
+          file: new File(["pdf"], "unlocked.pdf"),
+          fileId: "f-out",
+          sourceFileId: "f-tool",
+        },
+      ],
+    });
+
+    await waitFor(() => expect(fetchNotifications).toHaveBeenCalled());
+    expect(reportNotificationResolved).not.toHaveBeenCalled();
+  });
+
  it("does not resolve a tool failure from a different operation's success", async () => {
    fetchNotifications.mockResolvedValue({
      notifications: [toolRow()],
@@ -6,7 +6,10 @@ import {
  reportNotificationResolved,
  type AppNotification,
 } from "@app/services/notifications";
-import { loadRetryPayload } from "@app/services/notificationRetry";
+import {
+  loadRetryPayload,
+  stashMatchesKind,
+} from "@app/services/notificationRetry";
 import { rechainPolicyOnDocument } from "@app/services/notificationPolicyRetry";
 import type {
  SucceededToolRun,
@@ -129,8 +132,15 @@ async function continueRow(

  // A tool failure: resolved when the operation that failed succeeds on the same document.
  if (!row.fileId) return false;
+  // Succeeded FOR THIS FILE: a batch can succeed for one input and fail for another
+  // without ever reaching the failure path, so being an input of a successful run is
+  // not enough - the file must have produced an output.
+  if (!outputFor(row.fileId, run)) return false;
  const stash = await loadRetryPayload(row.fileId);
  if (!stash || stash.operation !== run.operation) return false;
+  // One stash per file but one incident per kind per file: a stash another kind's
+  // failure wrote is not evidence about this row.
+  if (!stashMatchesKind(row.kindId, stash)) return false;
  if (!row.actions.some((a) => a.enabled && a.id === "RETRY")) return false;
  return reportNotificationResolved(row.id);
 }