fix(channels): offer only session trust on channel approval cards (#4421)

[bolichen97]

Problem / Motivation

On a channel approval (the Channels page, not the chat surface), the approval card offered command-scoped trust tiers that were both mislabeled and destructive:

1. ChannelPage.tsx mounts ApprovalCard with title={msg.fromRole} — an agent role string, not a command. TrustDropdown derived its fullCommand/basePattern from that title, so the "Trust this command" tier literally read Trust "Researcher".
2. TrustDropdown rendered the trust_command action unconditionally; only trust_base was gated on the title looking like a shell command.
3. Clicking it emitted trust_command — a decision outside the channel backend's accepted set (approved/rejected/trust): the approve endpoint rejects it with HTTP 400, ChannelPage's .catch(() => {}) swallows the error, and the card optimistically flips to "Trusted — auto-approving future calls" while the approval future stays pending until channel.py's 1-hour timeout resolves it rejected.

Why it matters

A user who clicks "Trust this command" on a channel approval silently rejects the tool call they meant to allow — the UI confirms a grant that never happened, and the tier they clicked was describing an agent role rather than any command. That is a trust-UI integrity bug on an approval surface.

What changed (motivation → approach → change)

The channel approval message schema has no structured command field, so this takes the narrow direction from #4421: stop offering command-scoped tiers on a surface that has no command to scope them to.

• TrustDropdown gains an explicit hasCommand prop (default true). When false, the command-scoped tiers (trust_command, trust_base) are not rendered; the session-scoped plain trust action remains — the one decision in this family the channel backend accepts.
• ApprovalCard forwards the prop.
• ChannelPage passes hasCommand={false} at its single ApprovalCard mount (all three MessageBubble call sites, including the thread panel, route through it).
• An explicit prop rather than title-sniffing, so the chat surfaces (ChatInput.tsx, ActivityViewer.tsx), which omit the prop, keep all three tiers exactly as today.
• ChannelPage's onApprove signature still drops the pattern argument — moot for the removed tiers; no synthetic pattern is threaded through.

Explicitly out of scope (this PR does NOT close the schema gap):

• Direction (a) of ChannelPage approval trust tiers: role passed as command, trust_command decision coerced to rejected #4421 — threading a real tool title + pattern through the channel approval message schema — is not attempted; follow-up tracked in the issues linked below.
• The chat surface's trust tiers are untouched.
• channel.py's decision coercion (unknown → rejected) is deliberately untouched: fail-closed is the right default.
• Cause refactor: remove dead legacy compatibility shims #3's mechanism — the optimistic setDecided + .catch(() => {}) that lets a failed decision POST still flip the card to "Trusted"/"Approved" — is what made the removed tiers destructive, and this PR eliminates the only guaranteed-failure path through it. The mechanism itself (still reachable via a stale card after the 1-hour timeout, a decision raced from another tab, or a 404 after gateway restart) spans all ApprovalCard consumers and is tracked as ApprovalCard optimistically shows 'Trusted/Approved' even when the decision POST fails #5204 with a concrete fix shape (make onApprove return the request Promise; roll back decided and render failure on rejection).
• hasCommand stays optional-with-default-true by design (the narrow direction leaves command-bearing call sites untouched); a future command-less surface must pass false explicitly.

Tests

• TrustDropdown.test.tsx: with hasCommand={false} — only the plain session-trust item renders (even when the title looks like a shell command); it emits trust with no pattern; default keeps all 3 tiers (chat-surface regression guard).
• ApprovalCard.test.tsx: hasCommand={false} offers only the plain trust action and emits trust (a backend-accepted decision); shell-looking titles stay suppressed; default keeps 3 tiers.
• ChannelPageCoverage.test.tsx: integration — a channel approval message offers exactly one menu item ("Trust all tools"), no tier describes the role string, and the posted decision is always within approved/rejected/trust.
• ChatInput.approval.test.tsx: the chat surface offers all three tiers for a shell command (regression guard so a future change cannot silently strip them).

Local gates: backend pytest 61259 passed; tsc -b clean; vitest 22919 passed (the single failure is the pre-existing main-red #5188 — capture/thinking-block-align.tsx boots the English-only i18n entry, reproduced on clean main, fix in flight in #5166); isort/flake8/mypy/black/brand/i18n/jscpd all green.

Manual verification

Captured against a live dev server with mocked channel data (screenshots below): before — the dropdown offers Trust "Researcher" (the agent role rendered as a trustable command); after — only "Trust all tools".

Screenshots / video

Before — command tier mislabels the agent role as a command:

After — only the session-scoped trust action, which the backend accepts:

Full-card context (before / after)

Related Issues

Closes #4421

Checklist

• Single commit with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) — N/A: no documented API/schema change
• No secrets, credentials, or internal references in the diff

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 81c5d94f4b56dc4b4d4478d961ba56c7af6c9fc9 — this comment is updated in place on each push.

Review details

The change is a well-contained, subtractive UI gate: hasCommand defaults to true (chat surfaces and all command-bearing callers keep every tier), and false is passed only by the channel surface, where the plain trust action emits onAction('trust') with no pattern — matching the backend's accepted set. The suppression is guarded by hasCommand && on both command-scoped items, with the session-scoped trust item always rendered. No new conditional touches an existing path in a harmful way, no guard is removed, no crash or unsafe decision is introduced. The candidate pass found nothing, and I re-derived the same: nothing here produces an observable wrong outcome on a path the diff changes.

No findings.

[OPUS-REVIEWED] 81c5d94

Verdict parsed from the review's SHA-scoped output markers for commit 81c5d94f4b56dc4b4d4478d961ba56c7af6c9fc9.

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

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Correctly scopes trust tiers to what the channel backend accepts, with an explicit opt-out prop and the root cause (no structured command in the message schema) named for the eventual fix.

[DESIGN-REVIEWED] 81c5d94

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 81c5d94f4b56dc4b4d4478d961ba56c7af6c9fc9 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 81c5d94

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

[github-actions]

UX Review (Fable 5) — ✅ PASS

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

UX-Verdict: PASS

Removing the lying "Trust "Researcher"" tier fixes a label that silently rejected what it promised to grant; what remains is truthful and backend-honored.

Suggestions

• With hasCommand={false} the "Trust ▾" trigger opens a one-item menu ("Trust all tools") — every channel trust now costs two clicks to reveal a single choice, and a lone-item dropdown reads as broken; when hasCommand is false, render "Trust all tools" as a direct button in TrustDropdown/ApprovalCard instead of a menu.

[UX-REVIEWED] 81c5d94

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 81c5d94f4b56dc4b4d4478d961ba56c7af6c9fc9 — 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 evidence gathered; I verified the backend's accepted decision set in channel.py, counted TrustDropdown/ApprovalCard consumers, and checked the temp-screenshots/ convention against the PR template. Emitting the review.

First-Principles-Verdict: PASS

A reported silent-reject defect (#4421) is removed by deleting the two menu tiers that caused it; the one new prop has a counted consumer.

What this change ships

Intent: stop the channel approval card from offering trust tiers that mislabel an agent role as a command and silently reject the tool call. This is a FIX.

1. Channel approval's trust menu loses "Trust this command" / "Trust all … commands"; only "Trust all tools" remains — justified (the reported defect).
2. Trust clicks on a channel approval now post a decision the backend accepts (approved/rejected/trust, channel.py:740) — justified.
3. New hasCommand prop on TrustDropdown + forwarded by ApprovalCard — one real consumer (ChannelPage.tsx:163); minimal boolean, justified.
4. Chat surfaces keep all three tiers via default true — justified regression guard.
5. Four before/after PNGs under temp-screenshots/ — sanctioned convention (.github/PULL_REQUEST_TEMPLATE.md:47, pruned by cleanup-temp-screenshots.yml).

Counts run: non-test ApprovalCard mounts — 1 (ChannelPage.tsx:163), so no unfixed sibling surface; non-test TrustDropdown mounts — 3 (ApprovalCard, ChatInput.tsx:2391, ActivityViewer.tsx:287), the other two command-bearing and untouched; hasCommand={false} callers — 1. No existing knob does this job: showTrust={false} would also remove the plain trust the backend does accept, and isShell never gated trust_command. The deeper causes (schema gap, the .catch(() => {}) optimistic flip) are named in the description with tracked follow-ups (#5204), which is the accepted-and-deferred shape, not a finding.

[FIRST-PRINCIPLES-REVIEWED] 81c5d94

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97#bb3ad1ca]

CI failure attribution for this PR's first run (all four failures are external to this frontend-only diff):

• Backend Tests (3.10, 2) + Backend Tests (Windows) (2): main-red main-red: #3543 × #5113 mid-air collision — set_project on a slot-less channel turn crashes instead of refusing #5208 — mid-air collision between feat: allow set_project on non-dashboard surfaces (Telegram, Slack) #3543 and fix(messaging): consume session-directive markers in TurnDriver (#4540) #5113 (set_project gate vs the ChannelApplierBoundary tests). Repair PR: fix(dashboard): refuse slot-less set_project as a decision, not a crash (#5208) #5212. These two tests pass on this branch locally (its base predates feat: allow set_project on non-dashboard surfaces (Telegram, Slack) #3543); the merge-ref is what fails.
• Frontend Tests (4): tracked flake Flaky: CliPanel theme-sync repaint assertions fail under load in Frontend Tests shard 4 #5178 (CliPanel theme-sync repaint under shard-4 load; prior instance Flaky: CliPanel custom-theme repaint test depends on an rAF left pending by an earlier test #4222). Passes locally on this branch 41/41.
• Frontend Coverage Merge: downstream of the failed frontend shard (missing coverage artifact), not an independent failure.

Will rerun failed jobs once the run completes; the backend pair stays red until #5212 (or an equivalent) merges.

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97#bb3ad1ca]

Re: First Principles CONCERNS (cause #3 — the silent-confirm mechanism): agreed it deserved a named line, and it now has one — the PR body's out-of-scope list explicitly covers the optimistic setDecided + .catch(() => {}) swallow, tracked as #5204 (filed during this PR's pre-push review, before this CONCERNS round, with the same fix shape the review implies: onApprove returns the request Promise, the card rolls back decided and renders failure on rejection).

Why not close it here: the swallow spans every ApprovalCard consumer and changes the shared card contract (onApprove's return type), while this PR is the narrow #4421 fix — it removes the only guaranteed-failure path through that mechanism (the tiers whose decisions the backend always refused). Folding a cross-surface contract change into it would couple an urgent trust-labeling fix to a riskier refactor. The residual triggers (stale card past the 1-hour timeout, cross-tab race, gateway restart) are precisely scoped in #5204.

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97#bb3ad1ca]

Head refreshed 46d9b02 → 81c5d94 (zero-diff amend, same tree). Reason: gh run rerun --failed re-tests the original run's merge-ref snapshot, so the backend shards kept failing on the pre-#5212 main even after the main-red fix merged. A new head forces a fresh merge-ref against current main (which now carries #5212). Screenshot URLs in the body re-pinned to the new SHA.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #5248 is PARTIALLY_COVERED relative to this PR. Coverage is explicitly incomplete; this finding is not a completion or closure claim. Recommended action for PR #5248: REBASE. Merged PR #5202 landed only the hasCommand scaffolding and the suppression that this PR flips. It covers none of the channel trust capability, so closure is not available; the PR must be reconciled with the landed suppression and its pinning test. Files: website/src/components/TrustDropdown.tsx, website/src/pages/ChannelPage.tsx, website/src/test/ChannelPageCoverage.test.tsx.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.