Repository navigation
feat(desktop): approval card for parked device tool calls (#20938, PR 2) - #20997
Open
sauranshbhardwaj wants to merge 8 commits into
Open
sauranshbhardwaj wants to merge 8 commits into
sauranshbhardwaj wants to merge 8 commits into
Conversation
Collaborator
|
Reviewed the PR-2-only changes on head Verified strengths
For maintainer attention
Thank you for the exceptionally detailed PR description and the UI evidence — it made reviewing this stack feasible. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
Git-on-my-level
added
security-review
Contributor
Author
|
Thanks, and glad the description and evidence made the stack reviewable.
|
2 tasks done
…ool policy A dispatch_required decision for a sensitive on-device tool now has a policy-owned shape for what the user is asked: the capability, operation and resource the grant would be matched against, a bounded display preview of the input (attachment paths reduced to a file name, text and scripts capped at the 4 KiB card bound with a truncation flag the broker fails closed on), and the answers. Deny is the default; allow_once is always offered; allow_session is offered only when there is an exact resource for a grant to cover and carries plain words for what it would cover, so a script grant is shown as covering only that exact script. The 180s expiry matches the mobile device-tool transport and stays under the pi-mono long tool wait minus the Swift executor timeout, so a late answer cannot run an effect the model already reported as failed. Only this module decides the question and the options; adapters and the model never choose allow or deny.
sauranshbhardwaj
force-pushed
the
feat/desktop-approval-card
branch
from
October 10, 2026 07:22
1158b11 to
ebb2128
Compare
Collaborator
|
Reviewed the PR-2-specific changes (the stacked diff includes #20973/#20975 until they land). This renders the approval card for parked device tool calls, and the engineering holds together well:
Notes for a maintainer before merge:
No security or supply-chain concerns found in static review. Because this decides how users approve message sends, Messages/Mail reads, screenshots, and AppleScript execution — and it sits mid-stack — it needs human maintainer sign-off before merge rather than an automated one. This review was produced by AI automation on behalf of the maintainers. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
Contributor
Author
|
Thanks for the review. On the notes for maintainers:
|
sauranshbhardwaj
force-pushed
the
feat/desktop-approval-card
branch
from
October 10, 2026 20:57
ebb2128 to
a795670
Compare
Contributor
Author
|
One fix since the review, caught by a whole-suite Swift run on the stack:
The UI evidence images predate the darker grey. I'll recapture them on the next packaged run. |
… approval.requested send_message, run_applescript, read_message_history, list_message_chats and list_mail_messages used to fail with approval_required whenever no scoped grant existed, and nothing in the app could create one. capture_screen did not fail at all: it took and shared a full-screen screenshot with no approval, although its own description promised one, so it now parks the same way under the stable resource "screen". A screenshot holds whatever is on screen, including windows the person keeps out of capture elsewhere, so each one is allowed once: its card offers no session grant, no grant row covers a capture, and the kernel refuses to mint a grant the card did not offer. The realtime voice screenshot tool is no longer advertised to pi-mono, where it sat ungated in every typed chat, pill and workstream run; realtime voice runs still get it through their surface projection. The capability broker now prepares the ledger row and inserts one approval dispatch bound to that exact invocation (id, input hash, daemon boot epoch) in the same transaction, the kernel moves the run and attempt to waiting_approval and appends run.waiting_approval plus approval.requested, and the broker refuses to dispatch the invocation until the dispatch is resolved. resolve_desktop_dispatch gains the two user answers: decision allow without a grant admits exactly this invocation once and mints nothing; allow with a grant whose runId is null mints a session-scoped grant through the existing insertGrant path, capped at 24 hours. Deny, cancellation, expiry, run termination and owner change fail the prepared invocation closed as failed, never outcome_unknown, each with its own approval.resolved, and the run resumes so the model receives approval_denied. A new kernel operation, endChatApprovalScope, ends what a chat granted when the person clears it: the main chat keeps one kernel session across clears, so it closes every approval still parked in that session and revokes every grant the person minted for it, with one grants.revoked audit event. Startup reconciliation expires any pending bound approval and records approval.resolved, so a restart never replays a parked call. Every relay-callable tool is now classified on purpose (named in a policy set or declared unbundled, never by the read-only default), and a test holds the approval set to every relay-callable tool in a sensitive bundle, with each deliberate exception named and justified, so a new tool cannot ship unclassified or ungated by omission. Grants for desktop.context.screenshot_image must name their exact resource. The gated tools are held to their manifest schema at the kernel boundary and the resource a grant covers is derived only from the fields the tool acts on, so a model-supplied resource_ref can no longer become the grant pattern. A dispatch created through create_desktop_dispatch has its invocation payload key stripped and can never pose as a parked call. Authority is revalidated before a resolution is admitted and that check runs before the resolving transaction, so a revocation it triggers commits on its own; a rolled-back decision restores the broker's memory. The model still cannot resolve its own dispatch: the relay path keeps returning policy_denied without trustedUserControl. Parking sits behind a kernel switch, desktopToolApprovalsEnabled, that defaults to off and is not reachable from the daemon in this change, so runtime behavior is unchanged until a client that renders approval cards turns it on. The Swift card PR flips the default and deletes the switch.
…ol calls The stdio relay no longer answers a parked device tool call immediately. A new relay-tool-approvals module, taking the kernel and the frame and result writers, sends Swift an approval_requested frame carrying the dispatch id, the bound invocation, the policy preview, the options and the expiry, keeps the relay call pending, and settles it from the kernel's approval.resolved event: an allow decision hands the invocation to Swift through the ordinary authorized_tool_execution path exactly once, every other outcome returns approval_denied to the model. Every resolution is mirrored to Swift as approval_resolved so a card can close whatever ended it, including owner changes. Expiry fires from the runtime, a disconnected relay client cancels its own dispatch, and terminal run events fail parked calls like any other pending tool call. The Swift hand-off itself moves into the same module so the whole relay side is covered by hermetic tests with injected timers. Clearing a chat through journal_clear_turns now ends what that chat granted before the journal is cleared: the kernel closes the session's parked approvals and revokes the grants the person minted for it, because the main chat keeps one kernel session across clears and an "Allow for This Chat" grant would otherwise cover the new conversation. No tool reaches this; only the app's own clear message does. Parking is on only after a client declares desktop_tool_approval_cards through the new client_capabilities message; until the Swift card ships no client does, and the relay keeps today's immediate approval_required. The runtime advertises desktop_tool_approval_requests so that client can require it first. Realtime voice keeps today's behavior because its Swift tool wait is 180s and would race the approval TTL.
The pi-mono extension waits 30s for a normal Swift tool result and gives up with a timeout error. A human answering an approval card needs longer than that, and a late approval would then run a send the model had already reported as failed. The six device tools that park behind an approval, capture_screen among them, now declare the long timeout class, which the extension maps to its 10 minute wait, and a test holds every gated tool to it; the approval TTL of 180s plus the 120s Swift executor timeout stays well inside it. Swift does not consume timeoutClass outside the generated surfaces, so the regenerated files only carry the new manifest digests.
The approval UX design doc keeps its design status and points at the device tool surface doc for the runtime half that now exists; the Swift card, the Claude Code lane and the voice surface stay deferred and section 24 still applies. The device tool surface doc gains the ask-and-answer table, the schema and resource rules that stop the model naming what a grant covers, the session-grant options and their 24 hour cap, the restart and expiry behavior, and the interim client-capability gate. The changelog fragment is kind none because nothing is user-visible until the Swift card lands.
Render the kernel's approval_requested frame as a card in the main chat, workstream and agent pill threads, matched to the asking session, with the exact target, the content preview, the expiry and the answers the runtime offered; answer through signed direct control (allow once, allow for this chat with a 1 h session grant, deny) and close the card on approval_resolved whoever ended it. Declare desktop_tool_approval_cards after the handshake so the relay parks a sensitive call instead of failing it, require desktop_tool_approval_requests from the runtime, and add tool_approval_snapshot and answer_tool_approval bridge actions so omi-ctl can drive the card without Accessibility permission.
… stall In the live run the chat's 90 s tool no-progress guard revoked the turn and interrupted the bridge while a send_message card was waiting for the person, so the kernel cancelled the run and the card read Cancelled, not run. Teach StallDetector a waitingOnUser state the turn's tick loop sets from the approval store: while a card for the session is pending nothing promotes to slow or stalled, no tool is ever overdue, the tool row reads Waiting for your approval, and when the card closes every clock restarts from that moment; the stop button still cancels. Give the hermes and openclaw ACP adapters the same rule, treating a run the kernel holds in waiting_approval as progress for their 150 s no-progress cancel.
Six in-process captures from the packaged named bundle: a parked send_message card, the same card after Deny, parked list_message_chats and list_mail_messages cards, and the Allowed once state after an answer, with evidence.json recording agent capture through the automation bridge.
sauranshbhardwaj
force-pushed
the
feat/desktop-approval-card
branch
from
October 11, 2026 00:35
a795670 to
fe0cbf0
Compare
Collaborator
|
Reviewed the PR-2-only diff on What holds up well
Worth a maintainer's eye
Status Because this is PR 2 of a stacked sequence implementing security-sensitive permission-grant UX (approval cards that mint scoped grants and pause the stall watchdog), it remains behind This review was produced by an automated reviewer acting for the maintainers. AI-generated feedback — human maintainers make the final call. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed and why
Card half of #20938 (PR 2, stacked on
feat/desktop-approval-dispatch-relay, itself stacked onfeat/desktop-approval-dispatch). PR 1a taught the kernel to parksend_message,run_applescript,read_message_history,list_message_chats,list_mail_messagesandcapture_screenbehind an invocation-boundapprovaldispatch; PR 1b connected that to the relay and told Swift about it withapproval_requestedandapproval_resolved. This PR renders it.DesktopToolApprovalStoreprojects eachapproval_requestedframe into a card and closes it onapproval_resolved, whoever ended it: the user, expiry, cancellation, or an owner change.DesktopToolApprovalCardshows the tool's title and question, the exact target (resourceRef), the content preview the kernel bound the invocation to, the time the request stops waiting, and the answers: Allow Once, Allow for This Chat (1 h) only when the request offersallow_session(with itscoverstext under the buttons), and Deny.DesktopToolApprovalCardListmounts at the live edge of the thread whose kernel session asked, matched throughAgentRuntimeStatusStore.sessionIdBySurface, so a card never shows under another conversation: main chat (QueryAnswerThread), a workstream (TaskChatPanel) and the agent pill (AIResponseView, which falls back to the main chat session when no agent pill is open). Acapture_screencard is plain words only: "Take a screenshot", "Let Omi take a screenshot of your whole screen?", no target or preview rows (itsscreenresource is a grant key, not something to show), and only Allow Once or Deny: the runtime never offers a session grant for a screenshot, so a press of Allow for This Chat on such a card is refused locally like any answer the runtime did not offer. The copy and the options come from the runtime policy, so the card view needed no new code for it.Answers go through signed direct control.
DesktopCoordinatorService.resolveDispatchJSONcallsresolve_desktop_dispatchwith the full input: allow once sendsresolution.decision: "allow"and no grant; allow for this chat adds a grant withrunId: null, the dispatch's capability, operation andresourceRef,source: "user"and a 1 hexpiresAtMs; deny sendsdecision: "deny". A refused or undelivered answer puts the card back to pending with the reason and the buttons return; the kernel'sapproval_resolvedalways wins over a local answer in flight, and a second press while answering or after the decision sends nothing. The card never runs anything: the only path to the tool is still the kernel's resolution, exactly as PR 1a left it.The app declares
desktop_tool_approval_cardsthroughclient_capabilitiesright after a successful runtime handshake, but only to a runtime whoseinitadvertisesdesktop_tool_approval_requests; an older daemon without it still runs the chat and keeps its immediateapproval_required, so this build never takes the chat down over a capability. With a current daemon PR 1b's interim gate turns parking on whenever this app is connected. The gate and theclient_capabilitiesmessage stay as they are so this diff remains a reviewable UI change; removing them so parking is unconditional is a follow-up (PR 3) once the card has shipped in a beta. Resolved cards stay in the thread so an answer does not make the request vanish; the store keeps a bounded tail of fifty and never drops a pending card. Cards belong to one daemon process and one owner: the store is cleared on every runtime handshake, when the daemon exits and when the owner is revoked; a card whose deadline passes without a frame closes locally as expired and loses its buttons; and an answer the kernel refuses because the dispatch is no longer pending closes the card the way the kernel did rather than looping on "Couldn't send your answer". Model-authored text on the card is display-safe: bidirectional controls, zero-width and other invisible characters and control characters are shown as their code point (⟨U+202E⟩) rather than stripped, newlines are kept, and the rawresourceRefis what a session grant covers while the card shows the marked copy; the recipient is drawn in a monospaced run, a multi-line value is drawn as one ruled block so it cannot pose as another row, and a thread read by chat id still shows the handle it was given. Answers sent by the bridge carryresolvedBy: "desktop_automation"; the card's buttons answer asuser.Two automation bridge actions,
tool_approval_snapshot(read-only) andanswer_tool_approval(declaring all four effects, because an allow lets the parked tool run), letomi-ctlread and answer cards through the same store method the buttons call. They are non-production only like the rest of the bridge and are how the live run below was driven without Accessibility permission.A card waiting on the person is not a stall. The founder's live run found the chat lane's 90 s tool no-progress guard (
ChatProvider.perToolStallAbortMs) revoking the turn and interrupting the bridge while a card was up (app log:ChatProvider: tool no-progress guard fired at 90s (active_tools=1); interrupting bridge, 90 s aftertool.started; the kernel then wroterun.cancellation_requested,attempt.cancelled,approval.resolvedwithdecision: cancelled,run.cancelled), with the orange "taking longer than usual" banner up from 20 s.StallDetectornow has awaitingOnUserstate the turn's 500 ms tick loop sets from the approval store: while a card for the session is pending nothing promotes to slow or stalled, no tool is ever overdue, the tool row reads "Waiting for your approval" (newToolCallStatus.waitingApproval, persisted as in-flight), and when the card closes every clock restarts from that moment so the 90 s covers only the tool's real execution. The composer's stop button still cancels the run and the card still reads "Cancelled, not run". Every lane shares this one guard, because the main chat, the agent pill and a workstream thread all send throughChatProvider.sendMessagewith their own surface; the check is keyed by the session that turn resolved (pinnedSession.sessionId), so a card raised on the pill pauses the pill's turn and not the main chat's. The pill case is unit-tested, not live on the pill. Two residuals, documented rather than fixed: while a card is up every in-flight tool row of that turn reads "Waiting for your approval", not only the parked tool's, because the detector keys tools by the harnesstoolUseIdand the card carries the kernel invocation id (the in-app model runs one tool at a time, so in practice it is the same row); and a card stuck in.answeringbehind a hung direct-control call keeps the guard paused until the card's local expiry, at most 180 s, sincecallRuntimeControlToolhas no timeout of its own. On the daemon side (PR 1b's area, kept here because it is small) the hermes and openclaw ACP adapters' 150 s no-progress cancel now treats a run the kernel holds inwaiting_approvalas progress, through anisRunWaitingOnUseraccessor the adapters get from the kernel.One finding from the live run that PR 3 should take: left to itself, the in-app pi-mono model answers "send an iMessage" by calling
create_desktop_dispatchrather thansend_message, because the five tools' descriptions inomi-tool-manifest.tssay the call "always requires an approval dispatch". That model-authored dispatch is the pre-existing flow (no binding, no card, nothing in the app renders it, and it never expires), so the card appeared in the live run only when the model was told to call the tool directly; without that description change the in-app model often takes the oldcreate_desktop_dispatchpath and no card appears. Those descriptions should say the app asks the user itself; that change regenerates the Swift manifest digests, so it is prepared as a separate tiny branch,feat/device-tool-descriptions, stacked on this one to merge right after it.Not in this PR: a "Needs approval" line in the pill's activity row, parking for the realtime voice surface (unchanged from PR 1b), deleting the interim gate, and the tool-description change above (its own branch).
Oversized-file growth is limited to the call sites that cannot leave those files; everything else is in new files (
DesktopToolApprovalStore.swift,DesktopToolApprovalCard.swift,AgentRuntimeProcess+ToolApprovals.swift,Automation/DesktopAutomationToolApprovalActions.swift).e2e/flows/device-tool-approval-card.yamlcovers them for the flow-coverage gate, andscripts/desktop_flow_contract.pylists the new actions file.Line-Count-Exception: desktop/macos/Desktop/Sources/Chat/AgentRuntimeProcess.swift | 4373 -> 4384 | two kinds in the closed message switch, the post-handshake clear and conditional capability send, a two-line routing case, the owner-revoke case that clears cards, and one clear on daemon exit; the statics, wire message, routing body and clearing live in AgentRuntimeProcess+ToolApprovals.swift, and messageOwnerIsCurrentlyAuthorized drops private so that extension can call it
Line-Count-Exception: desktop/macos/Desktop/Sources/DesktopAutomationBridge.swift | 5172 -> 5173 | one registerToolApprovalActions() call in the builtin registration list; the actions live in Automation/DesktopAutomationToolApprovalActions.swift
Line-Count-Exception: desktop/macos/Desktop/Sources/FloatingControlBar/FloatingControlBarView.swift | 3018 -> 3020 | the approvalSurface argument at the one AIResponseView call site
Line-Count-Exception: desktop/macos/Desktop/Sources/MainWindow/Components/ChatBubble.swift | 2162 -> 2172 | the "Waiting for your approval" caption under the tool row and its accessibility value; the status itself and the icon live in ChatProvider's enum and ToolActivityTimelineLayout
Line-Count-Exception: desktop/macos/Desktop/Sources/Providers/ChatProvider.swift | 6824 -> 6834 | the waitingApproval case of ToolCallStatus with its doc line, its detector mapping, the turn's session id captured for the stall clocks, and the tick-loop lines that ask the approval store about that session; the lookup lives in ChatProvider+ToolApprovalWait.swift
Product invariants affected
INV-AGENT-* (
product/invariants/agent-control-plane.md), section Authority: the card is a display projection and holds no authority. Its allow path reusesresolve_desktop_dispatchover the same signed direct-control channeldirectControlToolalready uses, it never calls a tool, and the session grant it mints is bounded to the dispatch's exact resource and 1 h. The relay still dispatches only on the kernel'sapproval.resolved. Guard tests:ApprovalCardTests(allow once sends no grant; allow for this chat sends a session-scoped grant for the exact resource and is refused locally when the runtime did not offer it; a second press sends nothing; the kernel's resolution wins over a local answer in flight; a refused answer returns the card to pending),AgentRuntimeProcessTests(the handshake now requiresdesktop_tool_approval_requests; the shell declaresdesktop_tool_approval_cardsafter the handshake).INV-CHAT-1 (
product/invariants/chat-continuity.md) is named becausedesktop/macos/Desktop/Sources/Chat/**,desktop/macos/Desktop/Sources/FloatingControlBar/**anddesktop/macos/Desktop/Sources/MainWindow/Components/Chat*.swiftmatch its path globs. Unaffected: the store is a process-local projection of daemon frames, writes no transcript state, and is cleared on every handshake, daemon exit and owner change.INV-CHAT-2 (
product/invariants/chat-scroll-placement.md) is named becausedesktop/macos/Desktop/Sources/MainWindow/Components/ChatMessagesView.swiftmatches its glob. Unaffected: the card list is appended after the message rows inside the existing scroll content and touches no launch placement, anchoring, follow or virtualization logic; it never scrolls the reader, and a card appearing or closing changes content at the live edge only, exactly as a new message row does.INV-AUTH-1 (
product/invariants/auth-session.md) is named becausedesktop/macos/Desktop/Sources/Providers/ChatProvider.swiftmatches its glob. Unaffected: the change there is thewaitingApprovalstatus case, its detector mapping and two lines in the stall tick loop; it touches no session, token, owner-transition, 401 or Keychain path, and the lookup it adds reads a MainActor store keyed by the surface's kernel session.INV-UI-1 and INV-UI-2 (
product/invariants/brand-ui.md,product/invariants/desktop-ux-contract.md): the card draws withInk,OmiType,OmiSpacingandPageGlasstokens andOmiButtonStyleonly, no literal colors or fonts; text on the card and the "Waiting for your approval" caption uses onlyInk.primary/Ink.secondary, neverInk.tertiary, which measures under WCAG AA on the glass panel (ShellGlassChromeTestsfailed on the first version's caption; the card file is now in that test's list of chat surfaces too);check_desktop_ux_contract.pyreports no hand-rolled UX increase across the changed files.How it was verified
From
desktop/macosat the branch tip (fe0cbf0, stacked on PR 1b 7cc3022, PR 1a 10ef09e, main d3208a0). Thecapture_screengate was folded into the stack after the first round of checks; the build, the focused suites, the Node run and the preflight below were re-run on this tip, while the./test.shrun and the live packaged-app run were on the earlier tip (1158b11), beforecapture_screenwas gated:xcrun swift build -c debug --package-path Desktop-> clean (warnings are errors in this package).scripts/dev-feedback.py --once swift:ApprovalCardTests24/24 (new),AgentRuntimeProcessTests74/74,StallDetectorTests24/24 (3 new),ChatStallWatchdogTests10/10 (2 new),ToolCallStatusTests31/31,KernelTurnRecordedProjectionTests41/41,AgentRuntimeStatusStoreTests17/17,AuthorizedToolExecutionTests15/15,DesktopCoordinatorServiceTests32/32,DesktopAutomationActionEffectTests6/6,AgentContinuityGauntletTests5/5,ShellGlassChromeTests14/14; 0 failures. The whole Swift suite on the top of the stack (this branch plus PR 3) ran 7900 tests with one failure,RecentScreensMenuContentTests.testDayOldFrameShowsACalendarDateNotAClockTime, which also fails on upstream/main (clock and locale);SubscriptionEntitlementServiceTests.testAuthChangeInvalidatesCachecrashes the test process with signal 5 in a whole-suite run on upstream/main too, so that run skipped it../test.sh-> its shell phase ran all 54 launcher checks pluscheck-e2e-flow-coverage.py --strict; the only test-level failure wastests/test-omi-dev.sh, which fails on this machine's globaltag.gpgsign/tag.forceSignAnnotated(a baregit tagthen wants a message) and passes with those two settings off. The script then stopped at its Python phase becausebackend/.venvdid not exist; afterbackend/scripts/sync-python-deps.shthe same pytest command passed 269/269, and the Swift phase was run asscripts/swift-test-suites.sh: 108 suite invocations green, 2849 tests, and three failures outside this diff, each reproduced by the script's isolating rerun:RecentScreensMenuContentTests.testDayOldFrameShowsACalendarDateNotAClockTime,MeetingScreenEvidencePassTests.testRetryAfterFinalizeJoinsTheRunTheDeadlineLeftInFlight,DesktopAutomationWindowPresentationTests.testQuietPresentationAlsoParksNonactivatingOverlayPanels. None touches a file in this PR. Two fail alone for environmental reasons:RecentScreensMenuContentTestsrenders a 2027 frame as "13 Jan 2027 at 3:00 AM" where the test wants a bare calendar date (clock and locale), andDesktopAutomationWindowPresentationTestsmeasures real AppKit window frames on this display (title-bar and display geometry). The third,MeetingScreenEvidencePassTests, is a timing race (screenshots storeidlewherereadyis expected) that fails in the full run and in the one isolated run I made, and may pass alone. None was run on 1b's tip.npm ci,npm run buildclean,npx vitest runinagent-> 73 files, 959 tests passed (958 from PR 1b plus the ACP waiting-on-user test inruntime-adapter.test.ts).python3 .github/scripts/check_desktop_ux_contract.py --changed-files <list> --base feat/desktop-approval-dispatch-relay-> "OK: INV-UI-2 - no hand-rolled UX increase across 13 changed desktop file(s)";check_agent_doc_references.py-> OK;desktop-changelog.py validate-> OK.scripts/pr-preflight --base feat/desktop-approval-dispatch-relay --pr-body-file <this body>-> "PR preflight passed: 38 checks" (lane ci, base 7cc3022, 47 files; Python 3.11, as for PR 1a/1b). The run drove three changes that are in the diff: the approval wiring moved out ofAgentRuntimeProcess.swiftintoAgentRuntimeProcess+ToolApprovals.swiftfor the oversized-file ratchet,e2e/flows/device-tool-approval-card.yamlcovers the four new Swift files (two hermetic bridge steps, a log expectation for the capability line, and the live steps as manual prose), andscripts/desktop_flow_contract.pylists the new Automation actions file so flow lint knows its actions.scripts/failure-class prepare --pr-body-file <this body>-> OK,requires_declaration: false(nofix:commits).Real path, packaged app.
OMI_APP_NAME="omi-sauransh-test" ./run.sh --yolo --no-waitbuilt, signed and launched the named bundle (com.omi.omi-sauransh-test, dev backend, auth seeded from the production session) with this branch's app and daemon;omi-ctl wait-readyreported a signed-in main state andhealtha running runtime on protocol 2, and the daemon loggedclient_capabilities client=desktop-shell desktopToolApprovals=on. The card was driven through the new bridge actions (tool_approval_snapshot,answer_tool_approval), which call the same store method the buttons call; the accessibility driver on this machine was unavailable, so my harness never pressed a SwiftUI button; two cards were pressed by a person in the app during the run (below), which covered that path.send_messagedirectly parked the call; within 4 s the snapshot showed one pending card (send_message, resource+1 555 010 0001, options allow_once/allow_session/deny, "Waits until 12:59 PM") and the thread rendered it under the "Using send_message" row (image 001).answer_tool_approval ... answer=deny-> store statedenied, status "Denied, not run" (image 002); the kernel store shows the dispatchresolvedbyuserwith{"decision":"deny","selectedOptionId":"deny"}, the ledger row for that invocationfailedwitherror_code=approval_deniedand nodispatched_at_ms, and the model replied "The iMessage wasn't sent - the approval was denied." Nothing was sent.list_message_chatscall parked (image 003);answer=allow_once-> "Allowed once" (image 004); the ledger shows exactly one row for it with onedispatched_at_msand onecompleted_at_ms, and the app log one "Executing tool: list_message_chats with args: ["limit": 3]". The tool itself then failed because the fresh named bundle has no Full Disk Access, which the model reported; that is a TCC fact about the test bundle, not the approval path.send_messagecard (+1 555 010 0002, text "Expiry check, do not send", raised at 12:59:33 for the expiry check below) was answered Allow Once in the app's own card by the founder at the machine five seconds later, while my harness was waiting for it to expire. The kernel store shows that dispatchresolvedbyuserwith{"decision":"allow","selectedOptionId":"allow_once"}and its ledger rowsucceededwith onedispatched_at_msand onecompleted_at_ms, so the SwiftUI button path itself ran end to end once and Messages accepted the send to that reserved 555 number. No screenshot of that moment exists; the harness was asleep.ChatProvider: tool no-progress guard fired at 90s (active_tools=1); interrupting bridge90 s aftertool.started, and the kernel events for that run readrun.waiting_approval,approval.requested, thenrun.cancellation_requested,attempt.cancelled,approval.resolved(decision: cancelled,resolvedBy: system, reasonrun_tool_attempt_terminal) andrun.cancelled. That is the defect the stall fix above closes.list_mail_messagescard raised at 15:00:14 was left alone; at 30 s the tool row read "Waiting for your approval" with no banner (image 008); at its 180 s deadline the card read "Expired, not run" (image 007), the model replied "The request to read the latest mail expired before approval, so I couldn't retrieve it", the kernel store shows the dispatchexpiredbysystem(approval_wait_expired) and the ledger rowfailedwitherror_code=approval_expiredand nodispatched_at_ms, and the app log for that process has nono-progress guard firedline.approval.resolvedwithdecision: expired,reason: approval_wait_expired, 180.005 s afterapproval.requested, and zero cancellation events for that run.ApprovalCardTestsand the kernel side by PR 1a'scontrol-tools.test.ts.Not verified live: the workstream (
TaskChatPanel) and agent pill (AIResponseView) hosts, which share the sameDesktopToolApprovalCardListand differ only in the surface they pass; an answer refused by the kernel (ok: false) and a transport failure, both unit-tested; a Claude Code or pi lane through the stdio relay (the live run used the in-app pi-mono harness, whose device tools take the same relay). Two first-run setup gaps on this machine, both outside the diff:run.sh's bundle audit needs Homebrewpcre2for the SDK's vendored ripgrep and a staticffmpegat the git-ignoredDesktop/Sources/Resources/ffmpeg; I installedpcre2and fetched the arm64 build from the same sourcecodemagic.yamluses.Tests
ApprovalCardTests(new, 24): a requested frame becomes a pending card with target, preview and three answers; Allow for This Chat is offered only when the runtime offers it; a malformed frame is ignored; allow once sends one resolution and no grant; allow for this chat sends a session-scoped 1 h grant for the exact resource (runId: null,resourcePattern=resourceRef,expiresAtMs= now + 3600 s) and is refused locally when not offered; deny sends a deny resolution and the card reads denied; a second press while answering or after the decision sends nothing; a refused answer (ok: false) and a transport failure both put the card back to pending with the reason;approval_resolvedcloses the card for allow, deny, expired and cancelled; a resolution that arrives before its request still closes the card; a resolution for another card or with an unknown decision changes nothing; the kernel's resolution wins over a local answer in flight; trimming drops resolved cards and never pending ones;DesktopCoordinatorService.resolveDispatchJSONpasses the full resolution and grant through direct control; the bridge actions are registered with the declared effects and their helpers read and guard the shared store like the card; bridge answers carryresolvedBy: desktop_automationwhile the buttons answer asuser; a pending card expires locally at its deadline and offers no buttons, and a press after the deadline sends nothing; a refusal because the dispatch is no longer pending closes the card like the kernel did;reset()drops every card and early resolutions stay bounded at fifty; preview text is display-safe with each hidden character shown as⟨U+XXXX⟩while the rawresourceRefstill feeds the grant'sresourcePattern, and a thread read by chat id keeps its handle; acapture_screencard shows only its plain title and question with no target or preview rows and only Allow Once and Deny, a press of Allow for This Chat sends nothing, and Allow Once sends one resolution with no grant; Clear Chat sends the main chat's journal clear, which is what ends the chat's grants and parked cards in the daemon (PR 1b), so "Allow for This Chat" does not outlive the chat.e2e/flows/device-tool-approval-card.yamlgains a manual step for that.AgentRuntimeProcessTests: a runtime that advertisesdesktop_tool_approval_requestsgets theclient_capabilitiesmessage (protocol version, request id,desktop-shell, the capability) and an older runtime without it still passes the handshake and gets none;approval_requestedandapproval_resolvedparse to their kinds;clearToolApprovalCards()empties the shared store. The three places that call it (handshake success, daemon exit, owner revoke) are reviewed rather than unit-tested: the test-quality ratchet forbids another production-source scan, and the message router is private.StallDetectorTests(3 new): waiting on the person pauses every clock and restarts them when the card closes so the 90 s abort counts from the answer; a card appearing clears an already-stalled tool towaitingOnUserand nothing re-promotes while it is up; repeating the same wait state changes nothing.ChatStallWatchdogTests: the tick loop tells the detector about the wait, from the approval store keyed by the turn's own resolved session, before it asks which tools are overdue; a card on the pill's session pauses that turn and not the main chat's, and a closed card no longer pauses.ToolCallStatusTestsandKernelTurnRecordedProjectionTestscover the new in-flight status.runtime-adapter.test.ts(Node, 1 new): a hermes attempt whose run the kernel holds inwaiting_approvalis not cancelled for lack of progress, and the cancel fires one timeout after the card closes.UI evidence
Captured in the packaged named bundle built from this branch, through the bridge's in-process window capture (
capture_main_window_png), while driving the card withomi-ctl;captured_by: agent,source: visual-auditin.agent-artifacts/ui-evidence/desktop-approval-card/evidence.json..agent-artifacts/ui-evidence/desktop-approval-card/001-desktop-macos-approval-card-pending.png.agent-artifacts/ui-evidence/desktop-approval-card/002-desktop-macos-approval-card-denied.pnglist_message_chats) with the earlier denied card kept in the thread:.agent-artifacts/ui-evidence/desktop-approval-card/003-desktop-macos-approval-card-read-pending.png.agent-artifacts/ui-evidence/desktop-approval-card/004-desktop-macos-approval-card-allowed-once.pnglist_mail_messages):.agent-artifacts/ui-evidence/desktop-approval-card/005-desktop-macos-approval-card-mail-pending.png.agent-artifacts/ui-evidence/desktop-approval-card/006-desktop-macos-approval-card-mail-allowed-by-person.png.agent-artifacts/ui-evidence/desktop-approval-card/007-desktop-macos-approval-card-expired.png.agent-artifacts/ui-evidence/desktop-approval-card/008-desktop-macos-approval-card-waiting-no-banner.pngNo capture of a
capture_screencard: it is the same view with no target or preview rows, pinned byApprovalCardTests, and was not driven live.The captures above predate moving the card's field labels, covers note and pending status line, and the tool row's "Waiting for your approval" caption, from
Ink.tertiarytoInk.secondary; those runs now draw one step darker, and the images have not been recaptured.Known shape of this first version, visible in 002 and 004: a resolved card stays at the live edge of the thread, below the model's closing reply, rather than anchored to the turn that asked. Anchoring it to the turn needs the transcript to carry the dispatch id; left for a follow-up.
Failure class (fixes)
Failure-Class: none