Make the bell read like a bell, and stop the unlock duplicating a file

Three things from this morning's demo:

The row now carries the kind's own sentence rather than the server's raw
message, the empty panel says the reader is caught up, and the log moves
to a Copy log entry in the overflow menu. Reading the stack trace is the
processor's job, not the bell's.

View in processor opens a new tab when the reader has a loaded workbench
to lose, so a failure mid-upload does not cost them every file they had
open. From the processor, or from an empty workbench, it navigates as it
did.

Decrypt and retry versions the encrypted original in place when the file
is merely closed in the sidebar. The parent stub was read only from the
workbench, so a closed file was adopted as a new one: the user ended up
holding a decrypted copy and the locked original side by side.

frontend/editor/public/locales/en-US/translation.toml

@@ -5067,7 +5067,7 @@ label = "Free credits"
 
 [notifications]
 adoptFailed = "The document was unlocked but could not be opened here. Try the tool directly."
-empty = "Nothing to report."
+empty = "You're all caught up."
 handoffUnavailable = "This browser will not let the processor pass the document to the editor. Open it from the editor instead."
 noDocumentLinked = "This failure is not linked to a specific document, so it cannot be opened or retried here."
 notOnThisDevice = "This document is not on this device, so it cannot be opened or retried here."
@@ -5082,16 +5082,12 @@ unlockedRerunUndelivered = "The document was unlocked and the policy re-run star
 unread = "Unread"
 
 [notifications.action]
+copiedLog = "Copied"
+copyLog = "Copy log"
 failed = "That did not work. Try again in a moment."
 more = "More options"
 unavailable = "Not available for this notification."
 
-[notifications.detail]
-copied = "Copied"
-copy = "Copy error"
-less = "Show less"
-more = "Show full message"
-
 [notifications.section]
 earlier = "Earlier"
 new = "New"
@@ -7434,11 +7430,11 @@ description = "Policy runs that fail will appear here with the actions you can t
 title = "No failures recorded"
 
 [portal.failures.kind.inputPasswordProtected]
-description = "The pipeline could not open the document because it is password-protected. Unlock it and run it again, or skip this file."
+description = "Your file is password protected, so the run could not read it."
 title = "Password-protected document"
 
 [portal.failures.kind.unknown]
-description = "This run failed for a reason Stirling does not yet recognise. The raw message is shown below."
+description = "Something went wrong that Stirling does not recognise yet."
 title = "Unrecognised failure"
 
 [portal.failures.origin]

frontend/editor/src/core/components/notifications/NotificationBell.css

@@ -142,60 +142,6 @@
   overflow-wrap: anywhere;
 }
 
-/* Expanded, the message is the point of the row, so let it run and scroll rather than clamp. */
-.notification-bell__detail--full {
-  display: block;
-  max-height: 10rem;
-  overflow-y: auto;
-  -webkit-line-clamp: none;
-}
-
-/* The message in its own box; copy and expand are corner glyphs, not buttons rivalling the row's actions. */
-.notification-bell__detailbox {
-  grid-column: 1 / -1;
-  position: relative;
-  margin-top: var(--sp-1, 0.25rem);
-  padding: 0.375rem 0.5rem;
-  border: 1px solid var(--c-border-subtle);
-  border-radius: var(--radius-sm, 0.25rem);
-  background: var(--c-surface-sunken, var(--c-bg-raised));
-}
-
-/* Inside the box: drop the row grid placement, and keep the two-line clamp clear of the icons. */
-.notification-bell__detailbox .notification-bell__detail {
-  grid-column: auto;
-  margin: 0;
-  padding-right: 2.75rem;
-}
-
-.notification-bell__detailbox-actions {
-  position: absolute;
-  top: 0.25rem;
-  right: 0.25rem;
-  display: flex;
-  gap: 0.125rem;
-}
-
-.notification-bell__iconbtn {
-  display: inline-flex;
-  align-items: center;
-  justify-content: center;
-  width: 1.25rem;
-  height: 1.25rem;
-  padding: 0;
-  border: none;
-  border-radius: var(--radius-sm, 0.25rem);
-  background: transparent;
-  color: var(--c-text-subtle);
-  cursor: pointer;
-}
-
-.notification-bell__iconbtn:hover,
-.notification-bell__iconbtn:focus-visible {
-  background: var(--c-hover);
-  color: var(--c-text);
-}
-
 /* Why the actions this row could have had are absent. Muted: it explains, it does not warn. */
 .notification-bell__note {
   grid-column: 2;

frontend/editor/src/core/components/notifications/NotificationBell.test.tsx

@@ -65,6 +65,8 @@ vi.mock("react-i18next", () => ({
   useTranslation: () => ({
     // A string fallback, or an options object with defaultValue plus what it interpolates.
     t: (key: string, fallback?: unknown) => {
+      // The kinds' sentences live in the locale files, so one stands in here.
+      if (key.endsWith(".description")) return "Kind description";
       if (typeof fallback === "string") return fallback;
       if (fallback && typeof fallback === "object") {
         const options = fallback as Record<string, unknown>;
@@ -603,25 +605,43 @@ describe("NotificationBell", () => {
     expect(screen.getByLabelText("PDF password")).toBeTruthy();
   });
 
-  it("expands the message without touching the row's actions", async () => {
+  it("reads the kind's own words rather than the raw failure", async () => {
+    // A bell is not a log: the row gets a sentence, the message goes in the menu.
+    const stack = "org.apache.pdfbox.InvalidPasswordException";
     fetchNotifications.mockResolvedValue([
-      notification("a", "Unrecognised failure", {
-        detail: "org.apache.pdfbox.InvalidPasswordException",
+      notification("a", "Password-protected document", {
+        titleKey: "portal.failures.kind.inputPasswordProtected.title",
+        detail: stack,
       }),
     ]);
     render(<NotificationBell />);
     await openPanel();
 
-    const expand = screen.getByRole("button", {
-      name: "Show full message: Unrecognised failure",
-    });
-    fireEvent.click(expand);
+    expect(await screen.findByText("Kind description")).toBeTruthy();
+    expect(screen.queryByText(stack)).toBeNull();
+  });
 
-    expect(
-      screen.getByRole("button", { name: "Show less: Unrecognised failure" }),
-    ).toBeTruthy();
-    expect(
-      screen.getByRole("button", { name: "Copy error: Unrecognised failure" }),
-    ).toBeTruthy();
+  it("keeps the log one click away, for a row whose only extra is the log", async () => {
+    h.specs = { VIEW_FILE: { available: () => true, run: vi.fn() } };
+    const stack = "org.apache.pdfbox.InvalidPasswordException";
+    const clipboard = vi.fn().mockResolvedValue(undefined);
+    Object.assign(navigator, { clipboard: { writeText: clipboard } });
+    fetchNotifications.mockResolvedValue([
+      notification("a", "Unrecognised failure", {
+        detail: stack,
+        actions: [offer("VIEW_FILE", "SECONDARY")],
+      }),
+    ]);
+    render(<NotificationBell />);
+    await openPanel();
+
+    fireEvent.click(
+      await screen.findByRole("button", {
+        name: "More options: Unrecognised failure",
+      }),
+    );
+    fireEvent.click(await screen.findByRole("menuitem", { name: "Copy log" }));
+
+    await waitFor(() => expect(clipboard).toHaveBeenCalledWith(stack));
   });
 });

frontend/editor/src/core/components/notifications/NotificationBell.tsx

@@ -189,7 +189,7 @@ export function NotificationBell() {
 
           {notifications.length === 0 ? (
             <p className="notification-bell__empty">
-              {t("notifications.empty", "Nothing to report.")}
+              {t("notifications.empty", "You're all caught up.")}
             </p>
           ) : (
             <ul className="notification-bell__list">
@@ -262,6 +262,11 @@ interface PasswordPrompt {
  * 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.
  */
+/** The kind's own sentence, sharing the portal's copy. Empty for a kind this build has none for. */
+function summaryKeyOf(titleKey: string): string {
+  return titleKey.replace(/\.title$/, ".description");
+}
+
 function noteFor(
   notification: AppNotification,
   documentState: NotificationDocumentState,
@@ -300,7 +305,7 @@ interface NotificationItemProps {
   onRequestPassword: (prompt: PasswordPrompt) => void;
 }
 
-/** Its own component because the last attempt's message and its expanded state are per-row. */
+/** Its own component because the last attempt's message and the copy state are per-row. */
 function NotificationItem({
   notification,
   unread,
@@ -312,7 +317,6 @@ function NotificationItem({
   const { t } = useTranslation();
   const [message, setMessage] = useState<string | null>(null);
   const [busy, setBusy] = useState<string | null>(null);
-  const [expanded, setExpanded] = useState(false);
   const [copied, setCopied] = useState(false);
 
   const title = t(notification.titleKey, notification.defaultTitle);
@@ -376,6 +380,7 @@ function NotificationItem({
   };
 
   const note = noteFor(notification, documentState, withheldReasonKey, t);
+  const summary = t(summaryKeyOf(notification.titleKey), { defaultValue: "" });
 
   return (
     <li
@@ -398,54 +403,7 @@ function NotificationItem({
         </span>
       )}
 
-      {notification.detail && (
-        <div className="notification-bell__detailbox">
-          {/* Copy and expand sit in the corner of the message, not as buttons of their own, so reading
-              the failure and acting on it do not crowd each other out. */}
-          <span className="notification-bell__detailbox-actions">
-            <button
-              type="button"
-              className="notification-bell__iconbtn"
-              aria-label={`${t("notifications.detail.copy", "Copy error")}: ${title}`}
-              title={
-                copied
-                  ? t("notifications.detail.copied", "Copied")
-                  : t("notifications.detail.copy", "Copy error")
-              }
-              onClick={() => void copyDetail()}
-            >
-              {copied ? <CheckIcon /> : <CopyIcon />}
-            </button>
-            <button
-              type="button"
-              className="notification-bell__iconbtn"
-              aria-expanded={expanded}
-              aria-label={`${
-                expanded
-                  ? t("notifications.detail.less", "Show less")
-                  : t("notifications.detail.more", "Show full message")
-              }: ${title}`}
-              title={
-                expanded
-                  ? t("notifications.detail.less", "Show less")
-                  : t("notifications.detail.more", "Show full message")
-              }
-              onClick={() => setExpanded((wasExpanded) => !wasExpanded)}
-            >
-              {expanded ? <CollapseIcon /> : <ExpandIcon />}
-            </button>
-          </span>
-          <span
-            className={
-              expanded
-                ? "notification-bell__detail notification-bell__detail--full"
-                : "notification-bell__detail"
-            }
-          >
-            {notification.detail}
-          </span>
-        </div>
-      )}
+      {summary && <span className="notification-bell__detail">{summary}</span>}
 
       {note && <span className="notification-bell__note">{note}</span>}
 
@@ -469,7 +427,7 @@ function NotificationItem({
               onRun={() => void run(secondary)}
             />
           )}
-          {overflow.length > 0 && (
+          {(overflow.length > 0 || notification.detail) && (
             <Menu withinPortal position="bottom-end" shadow="md" width={180}>
               <Menu.Target>
                 <Tooltip
@@ -496,6 +454,16 @@ function NotificationItem({
                     {labelOf(offer)}
                   </Menu.Item>
                 ))}
+                {notification.detail && (
+                  <Menu.Item
+                    closeMenuOnClick={false}
+                    onClick={() => void copyDetail()}
+                  >
+                    {copied
+                      ? t("notifications.action.copiedLog", "Copied")
+                      : t("notifications.action.copyLog", "Copy log")}
+                  </Menu.Item>
+                )}
               </Menu.Dropdown>
             </Menu>
           )}
@@ -543,7 +511,6 @@ function ActionButton({
   );
 }
 
-/* Inline so the message box needs no icon dependency: two small glyphs for copy and expand. */
 const ICON_PROPS = {
   width: 14,
   height: 14,
@@ -556,41 +523,6 @@ const ICON_PROPS = {
   "aria-hidden": true,
 };
 
-function CopyIcon() {
-  return (
-    <svg {...ICON_PROPS}>
-      <rect x="9" y="9" width="13" height="13" rx="2" />
-      <path d="M5 15H4a2 2 0 0 1-2-2V4a2 2 0 0 1 2-2h9a2 2 0 0 1 2 2v1" />
-    </svg>
-  );
-}
-
-function CheckIcon() {
-  return (
-    <svg {...ICON_PROPS}>
-      <path d="M20 6 9 17l-5-5" />
-    </svg>
-  );
-}
-
-function ExpandIcon() {
-  return (
-    <svg {...ICON_PROPS}>
-      <path d="m7 15 5 5 5-5" />
-      <path d="m7 9 5-5 5 5" />
-    </svg>
-  );
-}
-
-function CollapseIcon() {
-  return (
-    <svg {...ICON_PROPS}>
-      <path d="m7 20 5-5 5 5" />
-      <path d="m7 4 5 5 5-5" />
-    </svg>
-  );
-}
-
 function MoreIcon() {
   return (
     <svg {...ICON_PROPS} strokeWidth={2.5}>

frontend/editor/src/proprietary/components/notifications/notificationActions.test.tsx

@@ -421,7 +421,9 @@ describe("useNotificationActions", () => {
   it("replaces the encrypted original in place when it is open in the workbench", async () => {
     // The failed document is on screen, so the unlock versions it rather than adding a second copy.
     openFileIds = ["f-1"];
-    openFilesById = { "f-1": { id: "f-1", name: "invoice.pdf", versionNumber: 1 } };
+    openFilesById = {
+      "f-1": { id: "f-1", name: "invoice.pdf", versionNumber: 1 },
+    };
     unlockLocalDocument.mockResolvedValue({
       ok: true,
       files: [
@@ -448,6 +450,30 @@ describe("useNotificationActions", () => {
     ).toBe(true);
   });
 
+  it("versions a document the workbench has closed, rather than adding a copy of it", async () => {
+    // Closed in the sidebar but still on the device: adding here left the user holding both.
+    openFileIds = [];
+    openFilesById = {};
+
+    await registry().DECRYPT_AND_RETRY?.run(policyContext(), "hunter2");
+
+    expect(addFiles).not.toHaveBeenCalled();
+    expect(consumeFiles.mock.calls[0][0]).toEqual(["f-1"]);
+  });
+
+  it("adds the unlocked document when nothing on this device holds the original", async () => {
+    // No stub anywhere: there is no version chain to extend, so adding is all that is left.
+    h.getStirlingFileStub.mockResolvedValue(null);
+
+    await registry().DECRYPT_AND_RETRY?.run(policyContext(), "hunter2");
+
+    expect(consumeFiles).not.toHaveBeenCalled();
+    expect(addFiles.mock.calls[0][1]).toEqual({
+      selectFiles: true,
+      derivedFromTool: true,
+    });
+  });
+
   it("closes the incident only once the document is safely in", async () => {
     // Reported first, then a failed adoption, would leave the row closed with nothing to show.
     retryWithPassword.mockResolvedValue({
@@ -557,6 +583,52 @@ describe("useNotificationActions", () => {
     expect(navigate).toHaveBeenCalledWith("/processor/documents#failures");
   });
 
+  it("opens a new tab rather than costing the reader a loaded workbench", () => {
+    // Navigating away would unload their files, costing them every upload again.
+    openFileIds = ["f-1"];
+    const openTab = vi.spyOn(window, "open").mockReturnValue({} as Window);
+
+    registry().VIEW_IN_PROCESSOR?.run(context());
+
+    expect(openTab).toHaveBeenCalledWith(
+      "/processor/documents#failures",
+      "_blank",
+      "noopener",
+    );
+    expect(navigate).not.toHaveBeenCalled();
+    openTab.mockRestore();
+  });
+
+  it("navigates in place when the tab would be refused", () => {
+    openFileIds = ["f-1"];
+    const openTab = vi.spyOn(window, "open").mockReturnValue(null);
+
+    registry().VIEW_IN_PROCESSOR?.run(context());
+
+    expect(navigate).toHaveBeenCalledWith("/processor/documents#failures");
+    openTab.mockRestore();
+  });
+
+  it("navigates in place from an empty workbench, which costs the reader nothing", () => {
+    const openTab = vi.spyOn(window, "open");
+
+    registry().VIEW_IN_PROCESSOR?.run(context());
+
+    expect(openTab).not.toHaveBeenCalled();
+    expect(navigate).toHaveBeenCalledWith("/processor/documents#failures");
+    openTab.mockRestore();
+  });
+
+  it("navigates in place from the processor, which has no workbench to lose", () => {
+    const openTab = vi.spyOn(window, "open");
+
+    registry(inProcessor).VIEW_IN_PROCESSOR?.run(context());
+
+    expect(openTab).not.toHaveBeenCalled();
+    expect(navigate).toHaveBeenCalledWith("/processor/documents#failures");
+    openTab.mockRestore();
+  });
+
   it("offers the processor link whenever the server did", () => {
     // The server only sends it to someone it will let read the queue.
     expect(
@@ -692,9 +764,9 @@ describe("retrying an attended policy run", () => {
 
   it("unlocks, takes the document in, runs the policy again, then closes the incident", async () => {
     const order: string[] = [];
-    addFiles.mockImplementation(async () => {
+    consumeFiles.mockImplementation(async () => {
       order.push("adopt");
-      return [{ fileId: "f-unlocked" }];
+      return ["f-unlocked"];
     });
     rechainPolicyOnDocument.mockImplementation(async () => {
       order.push("rerun");
@@ -713,14 +785,17 @@ describe("retrying an attended policy run", () => {
     expect(outcome).toEqual({ ok: true });
     // The unlock is the remove-password call on the document the row names, not a stashed endpoint.
     expect(unlockLocalDocument).toHaveBeenCalledWith("f-1", "hunter2");
-    // Added and selected, so the unlocked document is what is on screen once the panel closes. The
-    // encrypted original is left alone: the user never asked to lose it.
-    const [files, options] = addFiles.mock.calls[0];
+    // Versioned onto the encrypted original, so there is one document rather than two.
+    expect(addFiles).not.toHaveBeenCalled();
+    const [inputIds, files, stubs] = consumeFiles.mock.calls[0];
+    expect(inputIds).toEqual(["f-1"]);
     expect((files as File[]).map((file) => file.name)).toEqual(["invoice.pdf"]);
     // derivedFromTool is what stops the adoption starting a SECOND run of this same policy: the
     // dispatch effect in usePolicyAutoRun treats a plain upload as work to enforce. A policy run is
     // a billed automation run, so a double dispatch double-charges and can open a second incident.
-    expect(options).toEqual({ selectFiles: true, derivedFromTool: true });
+    expect(
+      (stubs as Array<{ derivedFromTool?: boolean }>)[0].derivedFromTool,
+    ).toBe(true);
     // Re-submitted under the ORIGINAL reference, so a second failure folds onto this same incident
     // instead of opening a new one about the same document - while the run's output is attributed to
     // the ADOPTED document, which is the one now in front of the user.
@@ -744,13 +819,15 @@ describe("retrying an attended policy run", () => {
     // derivedFromTool above; see the gate's own test in usePolicyAutoRun.chain.test.tsx.
     expect(rechainPolicyOnDocument).toHaveBeenCalledTimes(1);
     expect(rerunPolicy).not.toHaveBeenCalled();
-    expect(addFiles.mock.calls[0][1]).toMatchObject({ derivedFromTool: true });
+    expect(consumeFiles.mock.calls[0][2][0]).toMatchObject({
+      derivedFromTool: true,
+    });
   });
 
   it("still runs when the adoption reports no workspace id, rather than guessing one", async () => {
     // Nothing to attribute the output to, so the run goes untracked rather than being filed against
     // the encrypted original, which would version the wrong document.
-    addFiles.mockResolvedValue([]);
+    consumeFiles.mockResolvedValue([]);
     rechainPolicyOnDocument.mockResolvedValue({ ok: true, tracked: false });
 
     await registry().DECRYPT_AND_RETRY?.run(policyContext(), "hunter2");
@@ -778,7 +855,7 @@ describe("retrying an attended policy run", () => {
         "The document was unlocked and the policy re-run started, but its result cannot be delivered here, so this failure stays open.",
     });
     // Adopted regardless: the password bought them the unlocked document either way.
-    expect(addFiles).toHaveBeenCalled();
+    expect(consumeFiles).toHaveBeenCalled();
     expect(reportNotificationResolved).not.toHaveBeenCalled();
   });
 
@@ -811,7 +888,7 @@ describe("retrying an attended policy run", () => {
   });
 
   it("neither re-runs nor closes the incident when the document cannot be taken in", async () => {
-    addFiles.mockRejectedValue(new Error("quota"));
+    consumeFiles.mockRejectedValue(new Error("quota"));
 
     expect(
       await registry().DECRYPT_AND_RETRY?.run(policyContext(), "hunter2"),
@@ -839,7 +916,7 @@ describe("retrying an attended policy run", () => {
         "The document was unlocked and opened here, but the policy could not be run on it again.",
     });
     // Adopted anyway: the password bought them the unlocked document, and that is theirs to keep.
-    expect(addFiles).toHaveBeenCalled();
+    expect(consumeFiles).toHaveBeenCalled();
     // But nothing is fixed server-side, so the incident stays open.
     expect(reportNotificationResolved).not.toHaveBeenCalled();
   });

frontend/editor/src/proprietary/components/notifications/notificationActions.ts

@@ -86,6 +86,11 @@ function takeSelection(): string | null {
   }
 }
 
+/** False when the browser refused it, so the caller can fall back to navigating in place. */
+function openInNewTab(path: string): boolean {
+  return window.open(withBasePath(path), "_blank", "noopener") !== null;
+}
+
 /**
  * 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.
@@ -415,13 +420,15 @@ export function useNotificationActions(): ClientActionRegistry {
         // 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.
         const unlocked = asFiles(outcome.files ?? []);
-        // The original to version in place, when it is open here. Only a policy names one: a tool
-        // retry has no single input to replace, so it keeps adding its output.
+        // Only a policy names an original to version; a tool retry keeps adding its output.
         const originalId =
           target.kind === "policy" ? (target.policy.fileId as FileId) : null;
-        const parentStub =
-          (originalId && fileStore?.getState().files.byId?.[originalId]) ||
-          null;
+        // Storage too, or a file merely closed in the sidebar ends up decrypted twice over.
+        const parentStub = originalId
+          ? (fileStore?.getState().files.byId?.[originalId] ??
+            (await fileStorage.getStirlingFileStub(originalId)) ??
+            null)
+          : null;
         let adopted: FileId[] = [];
         try {
           adopted = await adopt(fileContext.actions, parentStub, unlocked);
@@ -478,7 +485,14 @@ export function useNotificationActions(): ClientActionRegistry {
       // is in portal/views/Documents, and both lift together.
       available: () => import.meta.env.DEV,
       closesPanel: true,
-      run: () => navigate(FAILURES_DESTINATION),
+      run: () => {
+        // Leaving would cost them a loaded workbench, and every file in it a re-upload.
+        const holdsFiles = (fileStore?.getState().files.ids.length ?? 0) > 0;
+        if (canOpenHere && holdsFiles && openInNewTab(FAILURES_DESTINATION)) {
+          return;
+        }
+        navigate(FAILURES_DESTINATION);
+      },
     };
 
     return {
@@ -487,5 +501,5 @@ export function useNotificationActions(): ClientActionRegistry {
       VIEW_FILE: viewFile,
       VIEW_IN_PROCESSOR: viewInProcessor,
     };
-  }, [canOpenHere, openInWorkbench, fileContext, navigate, t]);
+  }, [canOpenHere, openInWorkbench, fileContext, fileStore, navigate, t]);
 }