fix(channels): label channel trust as channel-wide and persistent (#5203) - #5230
Conversation
UX Review (Fable 5) — ✅ PASSUX-level review of UX-Verdict: PASS The label now tells the truth about the grant — channel-wide scope and persistence are named at the point of consent, verified in en, zh-CN, and 320px renders. [UX-REVIEWED] 99ca30e |
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS Verified against [DESIGN-REVIEWED] 99ca30e |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsThe backend confirms the label's accuracy: The change is a defensive label pass-through: No findings. [OPUS-REVIEWED] 99ca30e Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
First Principles Review (Fable 5) — 🟡 CONCERNSPremise-level review of All checks done. The backend claim verifies ( First-Principles-Verdict: CONCERNS The label fix is real and derived, but What this change shipsIntent: make the channel approval card's trust option name the grant it actually makes (channel-wide, persisted) — a FIX.
Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] 99ca30e |
|
Disposition for the First Principles CONCERNS verdict on
|
|
🤖 Kiro Crew Auto-Pipeline [operator: bolichen97#66809557]: Review-ready at |
) The channels surface's trust decision grants auto-approval to every agent in the channel and persists it to disk, but the approval card reused the chat surface's session-scoped 'Trust all tools' label, so the consent shown was narrower than the grant made. TrustDropdown gains an optional trustAllLabelKey override; the channels approval card passes a key naming the real grant ('Trust all tools in this channel — persists across restarts'). No behavior change; the chat surface keeps its default label. New string added to all 14 catalogs.
df93374 to
99ca30e
Compare
|
🤖 Kiro Crew Auto-Pipeline [operator: bolichen97#66809557]: rebased onto main Why it conflicted: #5202 landed in the meantime and touched the same three lines, giving Other changes in this push: #5202's channel-surface assertion ( Local gates green on the new head: |
|
Disposition for the First Principles CONCERNS verdict on the rebased head
|
|
🤖 Kiro Crew Auto-Pipeline [operator: bolichen97#66809557]: Review-ready at |
Problem / Motivation
On the channels surface, the approval card's trust dropdown shows the chat surface's session-scoped label, "Trust all tools" — but the channel
trustdecision executesch.trusted = True; ch._save()(src/kiro_crew/dashboard/handlers_channel.py,api_channel_approve_agent): it grants auto-approval to every agent in the channel and persists the grant to disk, surviving restarts. The label describes something much narrower than the grant it makes.Why it matters
A user who believes they trusted one agent for one session has actually granted channel-wide, restart-surviving auto-approval. That is a consent-integrity defect on a security control — the same shape as #4421 (label narrower than the actual grant). This PR is only about the label telling the truth; whether channel trust should persist is a separate product question and deliberately out of scope.
What changed (motivation → approach → change)
Symptom: the shared
TrustDropdownhardcodes thecomponents.trustDropdown.trust_all_toolskey, so every surface shows the session-scoped wording regardless of what itstrustaction actually does. Root cause: the component has no way for a surface to label a wider grant. Change:TrustDropdowngains an optionaltrustAllLabelKeyprop that overrides the trust-all label key;ApprovalCardpasses it through;ChannelPagesupplies a new key,components.trustDropdown.trust_all_tools_channel— "Trust all tools in this channel — persists across restarts". The chat surface passes nothing and keeps its default label. No behavior change: the emitted action is stilltrust.Rebased onto main after #5202 landed (which gave both components a
hasCommandprop and turned the channels surface into a single-tier menu). The two changes compose: the channels card now passeshasCommand={false}andtrustAllLabelKey, so the one remaining tier is the one whose scope this PR states. #5202's channel-surface test assertion was updated from the old session-scoped label to the new one, and a test pins the shipped combination.The new string is added to
en.manual.json(hand-authored, no source literal), all 11 translated catalogs (zh-CN, hi, es, fr, bn, pt, ru, de, ja, ko, it — following each language's style guide, em dash preserved per precedent), the regenerateden-XApseudolocale, and theen.context.jsontranslator sidecar.Tests
website/src/test/TrustDropdown.test.tsx:trustaction;Existing 26 TrustDropdown tests pass unchanged, pinning that the default surface is untouched.
Manual verification
Rendered both surfaces in a browser (Vite + Playwright) on the rebased head, asserting the rendered menu items: channels surface = 1 tier reading "Trust all tools in this channel — persists across restarts"; chat surface = 3 tiers with the default "Trust all tools" intact; zh-CN renders its translation; at a 320px viewport the longer label wraps inside the menu's viewport-aware width cap rather than overflowing.
Screenshots / video
Channels surface — corrected label:
Chat surface — default unchanged:
More: zh-CN, narrow viewport (320px)
Related Issues
Closes #5203
Related: #4421 (command-scoped tiers offered on the channel surface and 400-rejected) was fixed independently by #5202 and is now closed — this PR rebases onto it rather than duplicating it. #5204 (the card optimistically showing 'Trusted' when the decision POST fails) remains open and out of scope here.
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)en.context.json