Skip to content

feat(desktop): gated ui_screenshot — the visual representation (D3) - #487

Merged
physercoe merged 2 commits into
mainfrom
j8-d3-ui-screenshot
Jul 31, 2026
Merged

feat(desktop): gated ui_screenshot — the visual representation (D3)#487
physercoe merged 2 commits into
mainfrom
j8-d3-ui-screenshot

Conversation

@physercoe

Copy link
Copy Markdown
Owner

D3 of docs/plans/desktop-ui-context-and-pointing.md — the third representation of the desktop UI entity (ADR-062 D-4): pixels for the residue structure cannot answer.

Code only. The Electron halves (capturePage, the hub approval round trip) are unexercised on the authoring machine — no display, no Electron binary — and need a desktop/Playwright pass.

The three rules, all in one electron-free module

desktop/electron/src/uicapture.ts decides all of them, so node --test proves them without Electron.

1. Refusal by table. ui_policy.ts's capture column decides — read from the same projected focus snapshot the agent can already see, not a second source of truth. A split refuses if either pane refuses (the vault cannot ride along in the half the user is not focused on); an undeclared surface and an unpublished screen refuse too. Guests go through the same table via surfaceForPartition, so a new partition opts in by naming its row.

2. Per-call approval, always. A desktop_action attention card, never a session grant. §3.3 says "a hub card" and the hub raises it for relayed calls (D5) — but a local call never touches the hub, so the desktop raises the same kind itself and parks on it. via: 'hub' skips the local card, or a remote agent would be double-prompted. Signed out ⇒ no way to ask ⇒ refused, never captured unasked.

3. Fail closed. No answer in the window, a dismissed row, an unreadable resolution: all deny. The window leg re-checks the refusal after the park — the approval was for what was on screen when the card went up.

Read scope, action class

ui_screenshot sits in READ_TOOLS so the local kimi-code loop (read token only) can reach it — the consent that governs a capture is the card, not the spawn-time browser_bridge: true flag. It is an action everywhere else: audited on every leg (local browser reads are not) and hub-mirrored on every leg (hub-leg reads are ring-only). DESKTOP_ACTION_TOOL_NAMES holds that distinction in one place.

The park is 2 minutes, not the hub's 10: this leg holds an MCP call open over stdio, and a client that gives up mid-park would leave the user approving a call nobody is waiting for. Timeout dismisses the row and denies.

Files

uicapture.ts (new) the policy: refusal, partition→surface, card shaping, decision reading
uicapture_host.ts (new) capturePage + the hub approval round trip; registers itself on the bridge
browserbridge.ts the tool def, UI_TOOL_NAMES catalog gate, the captureUi dep, audit classification
AttentionDock.tsx the desktop_action card branch — Allow once / Reject, and a line saying why there is no session option

Verification

  • npm test (electron): 283 pass, 0 fail — D3 adds 21 (uicapture.test.ts 9, uiscreenshot.test.ts 12)
  • tsc --noEmit: clean, both frontend and electron
  • node --test src/state/*.test.ts src/ssh/*.test.ts: 376 pass (CI does not run these)
  • lint-desktop-tokens.sh: clean at baseline 65, phantom 0 · lint-docs.sh / lint-glossary.sh: clean
  • Not run: anything needing a display or an Electron binary

First of a stack: D3 → D4 → D5 → D6.

🤖 Generated with Claude Code

The desktop UI's third representation (ADR-062 D-4): pixels for the
residue structure cannot answer — a rendering bug, a layout question,
"why does this look wrong". It is also the most sensitive artifact the
app can emit, so three rules bind every call and all three are decided
in one electron-free module (uicapture.ts):

- REFUSAL BY TABLE. ui_policy.ts's `capture` column decides, reading the
  same projected focus snapshot the agent can already see. A split
  refuses if EITHER pane refuses, so the vault cannot ride along in the
  half the user is not focused on; an undeclared surface and an
  unpublished screen refuse too — we decline what we cannot classify.
  Guests go through the same table via surfaceForPartition, so a new
  partition opts in by naming its row instead of inheriting one.
- PER-CALL APPROVAL, ALWAYS. A `desktop_action` attention card, never a
  session grant. The hub raises it for relayed calls (D5); a LOCAL call
  never touches the hub, so the desktop raises the same kind itself and
  parks on it — 2 minutes, not the hub's 10, because this leg holds an
  MCP call open over stdio. `via: 'hub'` skips the local card, or a
  remote agent would be double-prompted. Signed out means no way to ask,
  which means refused — never captured unasked.
- FAIL CLOSED. No answer in the window, a dismissed row, an unreadable
  resolution: all deny. The window leg re-checks the refusal after the
  park, because the approval was for what was on screen when the card
  went up.

Read SCOPE, action CLASS: the tool sits in READ_TOOLS so the local
kimi-code loop (read token only) can reach it — the consent that governs
a capture is the card, not the spawn flag — but it audits on every leg
(local browser reads do not) and hub-mirrors on every leg (hub-leg reads
are ring-only). DESKTOP_ACTION_TOOL_NAMES is that distinction, in one
place.

The AttentionDock grows a desktop_action branch showing what was asked
for — surfaces, or the fragment-stripped guest URL — with Allow once /
Reject and a line saying why there is no session option.

21 new node --test cases (283 pass). The Electron halves (capturePage,
the hub approval round trip) are unexercised here — no display, no
Electron binary — and need the desktop/Playwright pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@physercoe
physercoe force-pushed the j8-d3-ui-screenshot branch from 28dc9d6 to 665f20d Compare July 31, 2026 03:14
@physercoe

Copy link
Copy Markdown
Owner Author

Rebased the whole stack onto 4f5d86c3 (main moved 7 commits during authoring — #479, #482#486 plus the docs audit). Conflicts were confined to the plan doc's status block; the code merged cleanly, including D4's pointer fold into state/annotation.ts, which #483 (unified assistant dock) had rewritten — re-verified rather than trusted.

Post-rebase, on each branch in turn: tsc --noEmit clean both sides, electron node --test 283 → 293 → 299 → 308 pass, frontend state/ssh 415 pass, lint-docs / lint-desktop-tokens clean.

@agentfleets

Copy link
Copy Markdown
Collaborator

Review of D3 (verified against head 665f20d8; local run of the electron suite at this commit: 283/283 pass, incl. the 21 new tests). Overall the policy core is clean and fail-closed, the consent card correctly has no session grant, and spec conformance against §3.3 is systematically checked. Two issues worth fixing before merge:

Major — timeout-dismiss is broken end-to-end (hub FK violation)

desktop/electron/src/uicapture_host.ts:141:

body: JSON.stringify({ resolved_by: 'desktop' }),

The hub column is resolved_by TEXT REFERENCES agents(id) (hub/migrations/0001_initial.up.sql:171) with FK enforcement on for runtime connections (_pragma=foreign_keys(1)). No agent with id desktop exists ⇒ the UPDATE FK-violates ⇒ 500 ⇒ swallowed by the catch. So every timed-out request leaves its card open in the director's inbox forever, contradicting the plan's pinned "timeout dismisses the row and denies" and this function's own "does not loiter in the inbox" comment.

Fix: omit resolved_by from the dismiss payload — NULL is the human/system-dismiss convention per handleResolveAttention's comment.

Minor

  1. Card misattributes the requesterraiseCard sends actor_handle: agentHandle, but handleCreateAttention only honors the body field when the auth-context handle is empty. The desktop's hub token belongs to the signed-in user ⇒ the row's actor becomes the desktop user, not the agent. The agent is still named in summary/payload.agent_id, so nothing is lost, but the meta line (and mobile rendering keyed on actor) misattributes. The hub-side browser_action raise avoids this by inserting actor_kind='agent' in SQL directly; the REST path has no equivalent.
  2. uicapture_host.ts has zero test coverage — raise/poll/dismiss sequencing, the FK-sensitive dismiss payload (a fetch-mocked node --test would have caught the major above), the post-park re-check, the via:'hub' skip. The seams are already injected (awaitCard(leg, id, deadline, now, sleep)), so this is cheap to add.
  3. Latent: refusal inherits the projection's focus-gated suppression — projectFocus drops secondary when the active pane's row has an empty allowlist, so "a split refuses if EITHER pane refuses" currently rests on settings being splitEligible: false, not on the capture path. Worth a comment coupling the invariant in uicapture.ts.
  4. Nit: a raiseCard failure on a reachable hub (500/transient) returns the "not signed in to a hub" message — wrong diagnosis (uicapture_host.ts:207).
  5. Nit: docs/reference/attention-kinds.md has no desktop_action entry (raised desktop-side, binary answer space, never session) — D5's PR adds the hub-raised leg, but the kind itself is introduced here.

…estable hub leg

Review fixes for #487 (D3):

- The timeout-dismiss sent resolved_by:'desktop'; the hub column
  REFERENCES agents(id) with FK enforcement on, so the UPDATE 500'd, the
  catch swallowed it, and every timed-out card loitered in the director's
  inbox forever. The dismiss body is now {} — NULL is the
  human/system-dismiss convention in handleResolveAttention.
- The raise/poll/dismiss round trip moves to uicapture_hub.ts,
  electron-free with an injectable fetch, and gains 9 node tests — one
  pins the FK-sensitive empty dismiss body, the rest cover
  approve/deny/timeout/raise-failure sequencing.
- A reachable hub that refuses the card now reports its own denial
  message (raise_failed) instead of the signed-out one.
- Comments: the visibleSurfaces split-refusal invariant (rests on every
  capture:'refuse' row being splitEligible:false), and why the card row's
  actor stays the desktop user (a user token attributing rows to
  arbitrary agents would be a spoofing lever).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@physercoe

Copy link
Copy Markdown
Owner Author

Second review pass (agent): verified the fleet review's findings against code and fixed them on the branch — commit f7da605f.

Major (confirmed + fixed): the timeout-dismiss body sent resolved_by:'desktop'; handleResolveAttention runs it through NULLIF(?,''), so the non-empty value hits the REFERENCES agents(id) FK (enforcement on) → 500 → swallowed → the card loiters forever. The dismiss now sends {} (NULL = the documented human/system-dismiss convention), and a fetch-mocked node test pins the empty body.

Also in the commit:

  • The raise/poll/dismiss round trip moved to an electron-free uicapture_hub.ts with injectable fetch — 9 new tests cover approve/deny/timeout/raise-failure sequencing (the minor-2 gap; capture() itself stays electron-bound as the reviewer noted).
  • Nit 4: a reachable hub that refuses the card now gets its own raise_failed denial message instead of the signed-out one.
  • Minor 3: the visibleSurfaces split-refusal invariant is documented — it rests on every capture:'refuse' row being splitEligible:false; if that ever changes the helper must read the raw focus state.
  • Minor 1 (actor misattribution): deliberately not changed hub-side — honoring body actor_handle for user tokens would let any team user attribute cards to arbitrary agents. Documented at the call site; the agent stays authoritative in summary + payload.agent_id. Flag if you want a hub-side actor_kind override instead.
  • Nit 5 (attention-kinds entry): the doc section lands in D5's PR; its header credit is fixed there.

Verified at the new head: electron 292/292 (283+9), tsc clean both sides.

🤖 Generated with Claude Code

@physercoe
physercoe merged commit 855791a into main Jul 31, 2026
8 checks passed
physercoe pushed a commit that referenced this pull request Jul 31, 2026
…490)

The stacked PRs squash-merged into their base branches (the GitHub
stack cascade), so main received only D3 (#487); this merge brings the
cascaded j8-d5-remote-desktop-ui head — element-resolved pointing (D4),
hub-relayed desktop UI context (D5), and agent pointing (D6), each
including its review-fix commit — onto main. Conflicts (main's D3-squash
regions vs the branch's D5/D6-extended versions of the same code)
resolved by taking the branch side, which is a strict superset; the
plan's status block updated to all-shipped.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants