fix(chat): withhold Trust from the composer when no slot can record it (#5486)

[chenmingwei23]

Problem / Motivation

ChatInput (the composer's approval bar) kept its own toApiDecision, and it mapped trust and trust_reads to a plain approve:

function toApiDecision(d: string): 'approve' | 'reject' | 'reject_once' {
  if (d === 'approved' || d === 'trust' || d === 'trust_reads') return 'approve'

That mapping feeds the one-shot api.resolveApproval fallback. handleApprovalAction routes a trust verb to the slot-scoped api.approveChatSlot only when activeSlot is set; with no slot it falls through to the one-shot endpoint, which honors exactly approve | reject | reject_once and records no standing grant (dashboard/handlers/sessions.py:1507). Meanwhile finish() dispatches resolveByApprovalId({ decision: 'trust' }), so the row renders as a standing grant.

Net effect in that state: the tool runs once, the composer says the user granted standing trust, and the backend recorded nothing. The user's next identical action prompts again, which reads as the grant having been forgotten rather than never made.

Compounding it, the same function already failed CLOSED for the other two verbs from the same dropdown: trust_command and trust_base fell to return 'reject'. Two of four trust verbs rejected, two silently approved, on one code path -- which is what marks the trust/trust_reads arm as an artifact rather than a policy.

Why it matters

This is a consent path, and the failure is quiet in the direction that overstates consent. A user who clicks Trust believes they have widened what runs without asking; the dashboard agrees with them; nothing was recorded. The gap is only observable later, as a prompt they thought they had silenced.

It is the third surface to ship this same defect independently -- #5400 (spawn-approval card, PR #5433) and #5434 (collapsed tool row, PR #5485) -- so the rule was evidently not written anywhere a third author had to read it.

What changed (motivation -> approach -> change)

The issue named two candidate readings and asked an owner to pick: the branch is dead, so delete the trust mapping; or the branch is reachable, so gate the Trust controls. The already-merged sibling resolves that by having done both: PR #5485 narrowed ChatPage's toApiDecision to action === 'approved' ? 'approve' : 'reject' AND made CollapsibleToolGroup's Trust button fail-closed behind an opt-in canTrust. This PR mirrors that shape, so nothing here is a fresh policy choice:

1. toApiDecision narrowed -- approved -> approve, rejected_once -> reject_once, everything else reject. It carries the constraint comment PR fix(chat): offer only resolvable decisions on collapsed tool rows (#5434) #5485 added at ChatPage's copy, which the issue said applies verbatim.
2. approvalTrustGrantable -- one predicate, !!activeSlot && !approvalIsUnattended, gating both Trust affordances (Trust reads; the Trust dropdown carrying trust / trust_command / trust_base). Previously they were gated on !approvalIsUnattended alone. So the mapping's trust arm is now unreachable from the DOM, and the narrowing is defence in depth behind a user-visible fail-closed gate.

What the user sees, before and after. With a slot present -- every state reachable from the shipped UI -- there is no change at all: Trust still routes to api.approveChatSlot and still records a standing grant. With no slot, the Trust affordances are absent and the bar offers Allow once / Reject / Reject once, which are exactly the decisions that path can honor.

What gets authorized, before and after. Strictly narrower, never wider. Before, a trust click in the no-slot state authorized one execution while claiming a standing grant. After, it authorizes nothing, because it cannot be clicked. No decision becomes auto-approved, and no set of commands newly runs without a prompt.

Reachability, stated honestly. Not reachable from today's grid: SessionGridView.renderLeaf mounts ChatPane only for leaf.kind === 'session' && leaf.slot, and deleteSlot.fulfilled clears state.messages in the same reducer that nulls activeSlot. It is one mount away: selectSlotPendingApproval(state, null) falls back to the global state.chat.messages, and SlotContext documents <SlotProvider slotId={null}> as a supported "intentionally empty pane" -- such a pane's composer would show the globally-active slot's pending approval with activeSlot null. The fix is fail-closed either way, which is why it does not depend on settling that.

Deliberate deviations

• The private toApiDecision is kept, not deleted in favour of the canonical one. It is not a pure duplicate: it also maps rejected_once -> reject_once, a third verb ChatPage's copy has no notion of and the only producer of which is RejectDropdown's "Reject once -- keep asking about the rest". Deleting it would silently downgrade that tier to a plain reject. ChatPage's version is also a component-private useCallback, so it cannot be imported. The two now agree on trust, and ChatInput.trustOneShot.test.tsx pins the agreement.
• The shared chokepoint is deliberately NOT built here. apps/mochi/panel/approvalActions.ts already holds a canonical routing predicate (TRUST_ACTIONS + approvalRoute), and promoting it out of apps/mochi for ChatPage / ChatInput / the tool-group row to consume is the right end state -- but it is a cross-app refactor on a consent path, which is a maintainer's call, not a rider on a bug fix. Filed as No chokepoint for the trust-verb/one-shot rule after #5400/#5434/#5486 are fixed #8193 with the audit behind it.

Tests

• website/src/test/ChatInput.trustOneShot.test.tsx (new) -- source contract on the narrowed mapping. It brace-matches the shipped toApiDecision body out of ChatInput.tsx and executes it, so the pin is on the MAPPING rather than its spelling: all four trust verbs answer reject, approved and rejected_once still answer their own verbs, no unknown verb answers approve, and there is still exactly one call site and it is the one-shot endpoint. A source contract for the same reason the sibling's ChatPage.collapsedGroupTrust.test.tsx is one -- after the render gate the arm is unreachable from the DOM, so a render test could not reach it.
• website/src/test/ChatInput.approval.test.tsx -- the pin that locked the old behaviour (trust action falls back to resolveApproval when no activeSlot, asserting resolveApproval('ap-123', 'approve')) is replaced by two that lock the new one, one per render site: the Trust dropdown is withheld with no slot, and Trust reads is withheld with no slot. The first also asserts Allow once and Reject are still present, so withholding the tier cannot silently take the bar with it.

Replacing that pin is the same contract change PR #5485 made at ChatPage and is on the same grounds; it was the second reason #5486 was held for a human, and the sibling merging is what settles it.

Mutation-verified, one per enforcement site, each read as a failure MESSAGE rather than an exit code:

mutation	reddens	failure
toApiDecision maps trust/trust_reads back to approve	trust-verb contract, only	AssertionError: expected 'approve' to be 'reject'
Trust reads gate back to !approvalIsUnattended	Trust reads pin, only	expect(element).not.toBeInTheDocument(), found the BookOpen "Trust reads" button
Trust dropdown gate back to !approvalIsUnattended	Trust dropdown pin, only	expect(element).not.toBeInTheDocument(), found the Handshake "Trust" button
approvalTrustGrantable drops !!activeSlot	both render pins	as above

A third candidate pin (nothing is resolved on render) was written and then dropped: it passed with the fix reverted, so it pinned nothing about this defect.

Manual verification

N/A -- and deliberately so rather than for convenience. The state where behaviour differs cannot be produced from the shipped UI (see Reachability above), so there is no click path to walk. The DOM evidence that the gate is the thing controlling those buttons comes from the mutation runs, which print the exact <button> that reappears when each gate is reverted.

Local, on 5c8ddc6: 26 files / 497 tests across ChatInput*, TrustDropdown, ApprovalCard; 8 files / 340 tests across the remaining Trust-asserting specs (ActivityViewerCoverage, ApprovalModePicker, ChannelPageCoverage, MochiChatPanel*, MochiSettingsPanel, mochiTrustGrantability); i18nLintExemptions 43 tests. eslint clean on the three files, tsc -b exit 0, check_focus_cue / check_feature_map / check_changelog_history / leaf_test_scope / ratchet_scope / check_harness_parity all pass with BASE_REF set to the merge-base.

Screenshots / video

None, and not omitted for convenience: with a slot present the rendered bar is unchanged, and the state that differs is not reachable from the shipped UI, so a capture would either show no difference or show a state a user cannot reach. The reappearing buttons are evidenced as serialized DOM in the mutation table above instead.

Related Issues

Closes #5486
Refs #5400, #5434, #8193

Pattern harvest

Rule candidate: review-prompt
Pattern: a decision string crossing into an endpoint that cannot honor every value it can carry, with the UI reporting the value it SENT rather than the one that was recorded. Three surfaces shipped this same shape independently (#5400, #5434, #5486), and the tell each time was an else-approve fallback (x === 'reject' ? 'reject' : 'approve', or an OR-chain onto approve) reachable from a richer verb set than the endpoint's. The generalizable question for a reviewer: for each verb this control can emit, does the endpoint it lands on record that verb -- and does the UI report what was recorded or what was clicked? The static form of the guard, plus whether one predicate should own the routing, is #8193.

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of 6e1460f4c89c91230868f1246e12faff98463322 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The change is a small, honest fix: the composer's Trust buttons ("Trust reads", the Trust dropdown) are now withheld when no slot exists to record a standing grant — previously a Trust click silently ran the tool once via the one-shot endpoint while the UI claimed a persistent grant. "Allow once" and Reject remain, so the approval can still be resolved. No new user-facing strings, no screenshots, and the behavior matches the sibling fixes on the spawn card and collapsed tool row. That is my review:

UX-Verdict: PASS

Removes a Trust control that lied — clicking it never recorded a grant — while keeping the honest Allow once / Reject paths intact.

[UX-REVIEWED] 6e1460f

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 6e1460f4c89c91230868f1246e12faff98463322 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Fail-closed gate plus narrowed mapping is the right shape, mirrors the settled sibling (#5485), strictly narrows authorization, and defers the chokepoint refactor legitimately (#8193).

Suggestions

• Exporting toApiDecision (test-only export) would pin the shipped function directly, without the brace-matching + new Function source scrape — simpler and it survives refactors like a TS annotation in the body; weigh against staying symmetrical with the sibling's source-contract pattern.

[DESIGN-REVIEWED] 6e1460f

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 6e1460f4c89c91230868f1246e12faff98463322 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 6e1460f

Verdict parsed from the review's SHA-scoped output markers for commit 6e1460f4c89c91230868f1246e12faff98463322.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 6e1460f4c89c91230868f1246e12faff98463322: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 6e1460f4c89c91230868f1246e12faff98463322 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 6e1460f

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 6e1460f4c89c91230868f1246e12faff98463322: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 6e1460f4c89c91230868f1246e12faff98463322 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims verified: the routing at ChatInput.tsx:1008-1023, the merged sibling narrowing at ChatPage.tsx:4971, the existing approvalRoute chokepoint in apps/mochi, and — grepping all 12 non-test resolveApproval( call sites — every other caller is typed or literal approve | reject, so no trust-verb sibling remains unfixed. Final review:

First-Principles-Verdict: PASS

Third and last surface feeding trust verbs into an endpoint with no trust verb; harm named, siblings counted to zero, alternative considered and filed.

What this change ships

Intent: stop the composer claiming a standing Trust grant the backend never recorded — a FIX.

1. Trust dropdown disappears when no slot can record the grant — justified (the fix).
2. "Trust reads" button disappears in the same state — justified (the fix).
3. A trust verb reaching the one-shot fallback now rejects instead of running once — justified, declared defence in depth mirroring merged ChatPage.tsx:4971.
4. New approvalTrustGrantable predicate — 2 consumers (both render sites), declared.
5. New source-contract test executing the mapping from source text — declared; uniquely reddens the mapping mutation the render pins cannot reach.
6. Old fallback test pin replaced by two withhold pins — declared, same contract change the merged sibling made.

Watch

The trust-verb set is now spelled in three places — apps/mochi/panel/approvalActions.ts:25 (TRUST_ACTIONS), ChatInput.tsx:1008 (inline array), and the new test's verb list — and they can diverge until the declared consolidation (#8193) lands. The author counted this and filed it; noting it here only so a human tracks that issue rather than letting the third spelling become permanent.

[FIRST-PRINCIPLES-REVIEWED] 6e1460f

[bolichen97]

Approving on the strength of a full readiness audit of every open PR against main, not a
line-by-line reading of this diff — recording that plainly so the next reader knows what this
stamp does and does not cover.

Verified against this exact head SHA:

• readiness: passed present, and PR Readiness — the one required status context on main
  (ruleset protected-branches) — is success on this head.
• No check run on this head is failure, cancelled, timed_out or still in flight. Skipped
  jobs are path-filtered conditionals, none of them required.
• mergeable: true, and the head is not far enough behind main for its green CI to describe a
  base that no longer exists.
• No surviving reviewer CHANGES_REQUESTED: any such review is on an older commit and therefore
  already dismissed by dismiss_stale_reviews_on_push.
• Every issue comment, inline review comment and review thread was read and classified. Nothing
  left is an unresolved human change request — the remainder is bot review-lane output, resolved
  or outdated threads, explicitly non-blocking suggestions, and author status notes.

Auto-merge (squash) is armed, so this lands once every other ruleset requirement is met.