fix: six measured interaction and a11y defects (dialog Tab, switch names, tooltip, copy failure, checkbox, approval card)

crates/biorouter/src/providers/coding_agent/bridge.rs

@@ -705,10 +705,18 @@ impl BridgeGrant {
         let request = UserActionRequest::ToolApproval(ToolApprovalRequest {
             tool_name: call.name.to_string(),
             arguments: arguments.clone(),
-            prompt: Some(format!(
-                "{} asked to run this through Biorouter.",
-                self.child_label()
-            )),
+            // ⚠ **`prompt` is the inspector's field — "why you are being asked" —
+            // and nothing else may borrow it.** This used to carry "<child> asked
+            // to run this through Biorouter", which is framing, not a finding; the
+            // card reads any prompt as a SECURITY FINDING, so every bridged call
+            // arrived under a warning banner with "Always Allow" withheld. The
+            // card was telling the truth about the field it was given.
+            //
+            // The attribution belongs on the card, but it needs a field of its
+            // own (`requestedBy`) plumbed through `ActionRequiredData` and the
+            // OpenAPI client. Until then it is logged rather than dressed up as
+            // an inspector's verdict.
+            prompt: None,
             risk: Some(self.tool_risks.risk_for(&call.name)),
             preview: crate::conversation::tool_preview::ToolPreview::for_tool_call(
                 &call.name, &arguments,
@@ -729,15 +737,19 @@ impl BridgeGrant {
             UserActionOutcome::Approved { permission } => {
                 tracing::info!(
                     tool_name = %name,
+                    child = self.child_label(),
                     ?permission,
                     "a bridged tool call was approved by the user"
                 );
+                self.record_lasting_decision(&call.name, &permission).await;
                 Ok(())
             }
             // `AlwaysDeny` and `DenyOnce` read the same to the child: it may not
             // run this. The *scope* of the refusal is the permission store's
-            // business, not the child's.
-            UserActionOutcome::Denied { .. } => {
+            // business, not the child's — which is why the store is written
+            // before the refusal is worded.
+            UserActionOutcome::Denied { permission } => {
+                self.record_lasting_decision(&call.name, &permission).await;
                 Err(format!("`{name}` was refused: you did not approve it."))
             }
             other => Err(format!(
@@ -749,6 +761,37 @@ impl BridgeGrant {
         }
     }
 
+    /// Write a decision the user meant to LAST into the permission store.
+    ///
+    /// The outcome's `permission` carries the scope of the answer — its own doc
+    /// says it "distinguishes a one-off from an `AlwaysAllow` the caller may want
+    /// to record" — and on the agent's own path `handle_approved_and_denied_tools`
+    /// records it. The bridge only logged it, so "Always Allow" on a bridged card
+    /// granted a single call and the next identical one asked again: a card
+    /// offering a lasting decision it could not keep. An existing entry was
+    /// always honoured (the permission inspector reads the store before the card
+    /// is ever raised); what was missing was writing one.
+    ///
+    /// A one-off (`AllowOnce` / `DenyOnce`) is deliberately not recorded: that is
+    /// an answer about this call, not a rule.
+    async fn record_lasting_decision(
+        &self,
+        tool_name: &str,
+        permission: &crate::permission::Permission,
+    ) {
+        use crate::config::permission::PermissionLevel;
+        use crate::permission::Permission;
+
+        let level = match permission {
+            Permission::AlwaysAllow => PermissionLevel::AlwaysAllow,
+            Permission::AlwaysDeny => PermissionLevel::NeverAllow,
+            _ => return,
+        };
+        self.inspections
+            .update_permission_manager(tool_name, level)
+            .await;
+    }
+
     /// What to call the child on the approval card.
     ///
     /// The user is being asked to approve a call *they* did not make, so the card

ui/desktop/src/components/MessageCopyLink.test.tsx

@@ -0,0 +1,83 @@
+import { render, screen, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
+
+const mocks = vi.hoisted(() => ({ toastError: vi.fn() }));
+vi.mock('../toasts', () => ({ toastError: mocks.toastError }));
+
+import MessageCopyLink from './MessageCopyLink';
+
+/** No rich node, so the component takes the plain-text path. */
+const noContent = { current: null };
+
+/**
+ * ⚠ **Call this AFTER `userEvent.setup()`.** `setup()` installs a clipboard stub
+ * of its own, so a stub written first is replaced by one whose `writeText`
+ * resolves — and every failure test quietly measures a success.
+ */
+function stubClipboard(writeText: () => Promise<void>) {
+  // `Object.assign` works once and then trips over the prototype getter, which
+  // makes the second test in the file fail for a reason that has nothing to do
+  // with what it is testing.
+  Object.defineProperty(navigator, 'clipboard', {
+    value: { writeText: vi.fn(writeText), write: vi.fn() },
+    configurable: true,
+    writable: true,
+  });
+}
+
+beforeEach(() => {
+  mocks.toastError.mockClear();
+  // The app's own logging stays quiet; these paths log on purpose.
+  vi.spyOn(console, 'error').mockImplementation(() => {});
+});
+
+afterEach(() => vi.restoreAllMocks());
+
+/**
+ * The reported defect: when both clipboard writes fail, the outer catch logged
+ * `'Failed to copy text: '`, the inner catch logged `'Failed to copy text
+ * (fallback): '`, and that was the end of it. `markCopied()` was never reached,
+ * so the button went on saying "Copy" — the user's only way to learn nothing had
+ * been copied was to paste somewhere and find out.
+ */
+describe('MessageCopyLink', () => {
+  it('says so when the clipboard refuses', async () => {
+    const user = userEvent.setup();
+    stubClipboard(() => Promise.reject(new Error('denied')));
+    render(<MessageCopyLink text="hello" contentRef={noContent} />);
+
+    await user.click(screen.getByRole('button', { name: 'Copy message' }));
+
+    await waitFor(() => expect(screen.getByRole('button')).toHaveTextContent('Copy failed'));
+    expect(mocks.toastError).toHaveBeenCalledTimes(1);
+    expect(mocks.toastError.mock.calls[0][0]).toMatchObject({ title: 'Copy failed' });
+  });
+
+  it('leaves the success path alone', async () => {
+    const user = userEvent.setup();
+    stubClipboard(() => Promise.resolve());
+    render(<MessageCopyLink text="hello" contentRef={noContent} />);
+
+    await user.click(screen.getByRole('button', { name: 'Copy message' }));
+
+    await waitFor(() => expect(screen.getByRole('button')).toHaveTextContent('Copied!'));
+    expect(mocks.toastError).not.toHaveBeenCalled();
+  });
+
+  it('still counts a successful fallback as a copy', async () => {
+    // The rich write is what fails here — jsdom has no `ClipboardItem`, which is
+    // the same shape of failure as a browser refusing the `text/html` flavour —
+    // and the plain-text retry succeeds.
+    const user = userEvent.setup();
+    stubClipboard(() => Promise.resolve());
+    const contentRef = { current: document.createElement('div') };
+    contentRef.current.textContent = 'hello';
+    render(<MessageCopyLink text="hello" contentRef={contentRef} />);
+
+    await user.click(screen.getByRole('button', { name: 'Copy message' }));
+
+    await waitFor(() => expect(screen.getByRole('button')).toHaveTextContent('Copied!'));
+    expect(mocks.toastError).not.toHaveBeenCalled();
+  });
+});

ui/desktop/src/components/MessageCopyLink.tsx

@@ -1,15 +1,21 @@
 import React from 'react';
-import { Copy } from './icons/app-icons';
+import { AlertTriangle, Copy } from './icons/app-icons';
 import { MessageMetaAction } from './MessageMeta';
-import { useTransientFlag } from '../hooks/useTransientFlag';
+import { useTransientValue } from '../hooks/useTransientFlag';
+import { toastError } from '../toasts';
 
 interface MessageCopyLinkProps {
   text: string;
   contentRef: React.RefObject<HTMLDivElement | null>;
 }
 
+/** What the clipboard write is asked to put on the clipboard. */
+type CopyOutcome = 'copied' | 'failed';
+
 export default function MessageCopyLink({ text, contentRef }: MessageCopyLinkProps) {
-  const [copied, markCopied] = useTransientFlag(2000);
+  // One transient state with two values rather than two booleans that could
+  // both be true: the button says exactly one thing at a time.
+  const [outcome, markOutcome] = useTransientValue<CopyOutcome>(2000);
 
   const handleCopy = async () => {
     try {
@@ -32,22 +38,43 @@ export default function MessageCopyLink({ text, contentRef }: MessageCopyLinkPro
         await navigator.clipboard.writeText(text);
       }
 
-      markCopied();
+      markOutcome('copied');
+      return;
     } catch (err) {
       console.error('Failed to copy text: ', err);
-      // Fallback to plain text if HTML copy fails
-      try {
-        await navigator.clipboard.writeText(text);
-        markCopied();
-      } catch (fallbackErr) {
-        console.error('Failed to copy text (fallback): ', fallbackErr);
-      }
     }
+
+    // Fallback to plain text if the rich copy failed.
+    try {
+      await navigator.clipboard.writeText(text);
+      markOutcome('copied');
+      return;
+    } catch (fallbackErr) {
+      console.error('Failed to copy text (fallback): ', fallbackErr);
+    }
+
+    // ⚠ Both writes failed, and this is the branch that used to end at a
+    // `console.error` the user cannot see: `markCopied()` was never reached, so
+    // the button went on saying "Copy" and the only way to find out nothing had
+    // been copied was to paste. Say so, in both places the user might be
+    // looking — on the control they pressed, and once in the corner.
+    markOutcome('failed');
+    toastError({
+      title: 'Copy failed',
+      msg: 'Biorouter could not write to the clipboard. Select the message and copy it with your keyboard.',
+    });
   };
 
+  const failed = outcome === 'failed';
+
   return (
-    <MessageMetaAction onClick={handleCopy} icon={<Copy />} aria-label="Copy message">
-      {copied ? 'Copied!' : 'Copy'}
+    <MessageMetaAction
+      onClick={handleCopy}
+      icon={failed ? <AlertTriangle /> : <Copy />}
+      aria-label="Copy message"
+      className={failed ? 'text-text-warning hover:text-text-warning' : undefined}
+    >
+      {outcome === 'copied' ? 'Copied!' : failed ? 'Copy failed' : 'Copy'}
     </MessageMetaAction>
   );
 }

ui/desktop/src/components/ToolCallConfirmation.test.tsx

@@ -334,6 +334,42 @@ describe('ToolCallConfirmation (BR-63)', () => {
     expect(screen.queryByRole('button', { name: /Always Allow/i })).not.toBeInTheDocument();
     expect(screen.getByRole('button', { name: /Deny/i })).toBeInTheDocument();
   });
+
+  /**
+   * The other half of the same rule, and the one that was wrong in practice.
+   *
+   * `prompt` is `approval_prompt_for_request` — an inspector's reason for
+   * escalating — so a card with none is an ordinary permission ask and the user
+   * may grant a lasting permission from it. The coding-agent bridge used to fill
+   * that field with its own framing ("<child> asked to run this through
+   * Biorouter"), so every bridged call arrived under a warning banner with
+   * "Always Allow" withheld: a security verdict the daemon never reached.
+   * `bridge.rs` no longer writes it, and `bridgeApprovalPrompt.test.ts` keeps it
+   * that way.
+   */
+  it('offers "Always Allow" for an ordinary ask, with no warning banner', () => {
+    renderCard({
+      prompt: null,
+      preview: { kind: 'shell', command: 'ls -la', truncated: false },
+    });
+
+    expect(screen.getByRole('button', { name: /Always Allow/i })).toBeInTheDocument();
+    expect(screen.queryByTestId('tool-security-finding')).not.toBeInTheDocument();
+  });
+
+  /**
+   * A blank prompt is not a finding. A daemon sending `" "` — or a future
+   * producer trimming a message to nothing — used to paint an empty warning band
+   * AND take the user's "Always Allow" away, which is the worst of both: a
+   * warning that says nothing, and a permission that cannot be granted because
+   * of it.
+   */
+  it('treats a blank prompt as no finding at all', () => {
+    renderCard({ prompt: '   ' });
+
+    expect(screen.getByRole('button', { name: /Always Allow/i })).toBeInTheDocument();
+    expect(screen.queryByTestId('tool-security-finding')).not.toBeInTheDocument();
+  });
 });
 
 /**

ui/desktop/src/components/ToolCallConfirmation.tsx

@@ -190,6 +190,34 @@ export default function ToolConfirmation({
     return parts.length > 1 ? parts[0] : '';
   }
 
+  // `prompt` is `approval_prompt_for_request` — every reason an INSPECTOR gave
+  // for escalating this call — so its presence is what marks a security finding.
+  //
+  // One derived boolean rather than two independent reads of `prompt`: the
+  // warning banner and the withheld "Always Allow" are two halves of one
+  // decision, and a card that shows the banner while still offering a permanent
+  // grant (or the reverse) is worse than either. Whitespace is not a finding —
+  // a blank prompt used to paint an empty warning band *and* take the user's
+  // "Always Allow" away.
+  //
+  // ⚠ The coding-agent bridge used to put its own framing there ("<child> asked
+  // to run this through Biorouter"), which put every bridged call behind a
+  // warning banner with no way to grant a lasting permission. The card was
+  // reading the field correctly; the producer was misusing it. Fixed in
+  // `bridge.rs::await_approval` and pinned by `bridgeApprovalPrompt.test.ts`.
+  //
+  // ⚠ That guard covers `bridge.rs` and ONLY `bridge.rs` — it string-matches that
+  // one file. Do NOT read it as "nothing but an inspector writes `prompt`": five
+  // other production sites still put framing there, so their cards still draw the
+  // banner and still withhold "Always Allow" —
+  // `extension_manager_extension.rs` (install, delete), `platform_approval.rs`,
+  // `skills_extension.rs` and `bug_report/mod.rs`. Withholding the grant is
+  // arguably wanted for the destructive ones (they carry `requires_user_proof`),
+  // but the *banner* is not: one of them reads "install … from the trusted BAAM
+  // registry" under a warning triangle. Closing that needs a field distinct from
+  // `prompt`, which is a protocol change, so it is a known gap rather than a fix.
+  const securityFinding = typeof prompt === 'string' && prompt.trim().length > 0;
+
   // One cohesive, bordered "permission request" card. A single border wraps the
   // whole element (header + actions) so there are no mismatched borders, it uses
   // the app's standard card tokens + typography, and a gentle slide-in makes it
@@ -201,9 +229,12 @@ export default function ToolConfirmation({
   ) : (
     <>
       <div className="biorouter-message-content text-body overflow-hidden rounded-2xl border border-border-subtle bg-background-default animate-in fade-in slide-in-from-bottom-1 duration-200">
-        {/* Security finding banner, only when the backend flagged one */}
-        {prompt && (
-          <div className="flex items-start gap-2 border-b border-border-subtle bg-background-warning/10 px-4 py-2.5 text-sm text-text-warning">
+        {/* Security finding banner, only when an inspector flagged one */}
+        {securityFinding && (
+          <div
+            data-testid="tool-security-finding"
+            className="flex items-start gap-2 border-b border-border-subtle bg-background-warning/10 px-4 py-2.5 text-sm text-text-warning"
+          >
             <AlertTriangle className="mt-0.5 h-4 w-4 shrink-0" />
             <span>{prompt}</span>
           </div>
@@ -280,8 +311,10 @@ export default function ToolConfirmation({
                 >
                   Allow Once
                 </Button>
-                {/* Only offer "Always Allow" when there's no security finding. */}
-                {!prompt && (
+                {/* Only offer "Always Allow" when there's no security finding. A
+                    permanent grant is not something to decide from a card that
+                    exists because an inspector objected. */}
+                {!securityFinding && (
                   <Button
                     type="button"
                     size="sm"