From 3b23da052ee78b261760bde01aa00775ea553a32 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 10 Jun 2026 16:29:30 +0200 Subject: [PATCH] fold the Deny & simplify denial instead of a red error (T-340) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user-initiated denial (Deny & simplify) came back as an isError tool_result and rendered as a prominent expanded-red "Bash · error" block — pure noise, since the user chose it. It now folds to a muted, collapsed "denied" card. Built as a reusable filter rather than string-matching the note: DenyTool carries a `quiet` flag, the session collects quiet denials' tool_use_ids, and ConversationView renders any error whose id is in that set folded + muted. Genuine tool failures (ids not in the set) keep the expanded-red treatment. Adding future "expected error" cases is just adding ids. Co-Authored-By: Claude Opus 4.8 (1M context) --- .pql/changelog/ticket_history/2026-06.sql | 1 + .pql/changelog/tickets/2026-06.sql | 16 +++++++ CHANGELOG.md | 5 +++ lib/builtin/claude/src/claude_pane.dart | 1 + lib/builtin/claude/src/conversation_view.dart | 43 ++++++++++++++++--- lib/builtin/claude/src/prompt_card.dart | 4 +- .../claude/src/stream_json_session.dart | 20 ++++++++- .../claude/conversation_view_test.dart | 39 ++++++++++++++++- 8 files changed, 117 insertions(+), 12 deletions(-) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index e261dd9b..782e488b 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -3188,3 +3188,4 @@ INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, chang INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB0TNQM7F2TBJJV1F2KP7XR8', 'status', 'in_progress', 'done', NULL, '2026-06-10 13:55:30', '2026-06-10 13:55:30', '2026-06-10 13:55:30', NULL, 'f618ee8a3a33dabfa822bd0cebae4e03', 2) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB3JAXDKZMS0805MMEYB6820', 'status', 'ready', 'done', NULL, '2026-06-10 14:03:10', '2026-06-10 14:03:10', '2026-06-10 14:03:10', NULL, '96365bc3ef7dbf505448be2dad16e4b6', 2) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB3JM0AXK1CTWD720RV1AVZ0', 'status', 'ready', 'done', NULL, '2026-06-10 14:14:33', '2026-06-10 14:14:33', '2026-06-10 14:14:33', NULL, 'd028dbbdbad663a682c5ed4d92dc7cc2', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB3KS499THAD899M0NWN7E3R', 'status', 'ready', 'done', NULL, '2026-06-10 14:29:07', '2026-06-10 14:29:07', '2026-06-10 14:29:07', NULL, 'b7a07e0516cc232a959ff1930f338f15', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index ed0f2219..ce9a4f11 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -2606,3 +2606,19 @@ Acceptance: - Picking up a ticket with a live Claude pane injects the prompt AND moves the ticket to in_progress, with the sidebar reflecting the new state. - Picking up with no live session leaves the ticket untouched. - Test coverage on _onTicketPickUp for both the accepted and no-session paths.', 'done', 'medium', NULL, NULL, NULL, '2026-06-10 13:48:18', '2026-06-10 14:14:33', NULL, 'd615d07653f4916b50514993935b90da', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB3KS499THAD899M0NWN7E3R', 'bug', '06FB0TNQM5TWC00GW0P3X02HZW', 'Fold the Deny & simplify denial note instead of showing it as an expanded error', 'The ''Deny & simplify'' permission option (T-311/T-328) denies the action carrying a preformatted note (_kDenySimplifyNote in lib/builtin/claude/src/prompt_card.dart:27). Because it comes back as a tool_result with isError, the Claude pane renders it as a prominent red ''Bash · error'' block, expanded by default — see _toolResult() in lib/builtin/claude/src/conversation_view.dart:698, specifically collapsedByDefault: false (line 715). + +But this denial is user-initiated (the user clicked ''Deny & simplify''), so the full red confirmation is noise — the user already knows what they did and just wants Claude to retry simpler. Either suppress this specific error block, or fold it into the collapsible (collapsedByDefault: true, with the first line as the collapsed summary) so it doesn''t shout. + +Scope: ONLY the Deny & simplify-spawned error. Genuine Bash/tool errors must keep their current expanded-by-default behaviour (T-168). So this needs a way to distinguish a user-initiated deny-simplify denial from a real tool error — e.g. tag the DenyTool result so the renderer can recognise it, rather than string-matching the note text. + +Implementation sketch: +- Mark the deny-simplify result as a user-initiated denial when constructing DenyTool(note) in _permDenySimplify() (prompt_card.dart:234), carrying a flag through the tool_result envelope. +- In _toolResult(), when that flag is set, render collapsed-by-default (or suppress) instead of the expanded error path. +- Avoid brittle string matching against _kDenySimplifyNote. + +Acceptance: +- Clicking ''Deny & simplify'' produces a collapsed/quiet card, not an expanded red error. +- A real Bash error still renders expanded by default. + +Reference screenshot: paste-1781099488608.png (the expanded red ''Denied — this action is too complex...'' block over a follow-up retry).', 'done', 'medium', NULL, NULL, NULL, '2026-06-10 13:53:23', '2026-06-10 14:29:07', NULL, '6fad665013ede83fb4e538bb73308aa6', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/CHANGELOG.md b/CHANGELOG.md index 35d73bdb..95ac4ccf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,11 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Fixed +- **"Deny & simplify" no longer shows a loud red error.** A denial the user + deliberately chose (Deny & simplify) folds into a muted, collapsed "denied" + card instead of the prominent expanded-red block reserved for genuine tool + failures — which still render expanded. Driven by a reusable per-result + "quiet error" flag, not by matching the note text. (T-340) - **Sub-agent prompts no longer render as a blue "you" card.** In live (stream-json) sessions a sub-agent's spawning prompt is tagged with `parent_tool_use_id` rather than the transcript's `isSidechain`/`parentUuid`, diff --git a/lib/builtin/claude/src/claude_pane.dart b/lib/builtin/claude/src/claude_pane.dart index b3360d48..40a67041 100644 --- a/lib/builtin/claude/src/claude_pane.dart +++ b/lib/builtin/claude/src/claude_pane.dart @@ -507,6 +507,7 @@ class _ClaudePaneState extends State { foldLevel: foldLevelFromName(_kernel()?.settings.get(kActivityFoldLevelKey)), hiddenToolUseIds: _session?.promptedToolUseIds ?? const {}, toolUseOutcomes: _session?.toolUseOutcomes ?? const {}, + quietErrorToolUseIds: _session?.quietErrorToolUseIds ?? const {}, emptyState: ClaudeBanner( role: widget.isPrimary ? 'primary' : 'session ${widget.secondaryIndex}', workspace: _repoRoot, diff --git a/lib/builtin/claude/src/conversation_view.dart b/lib/builtin/claude/src/conversation_view.dart index 4d8faa36..14e1161d 100644 --- a/lib/builtin/claude/src/conversation_view.dart +++ b/lib/builtin/claude/src/conversation_view.dart @@ -34,6 +34,7 @@ class ConversationView extends StatefulWidget { this.emptyState, this.hiddenToolUseIds = const {}, this.toolUseOutcomes = const {}, + this.quietErrorToolUseIds = const {}, this.foldLevel = FoldLevel.tools, }); @@ -53,6 +54,13 @@ class ConversationView extends StatefulWidget { /// border instead of being hidden (D-78). final Map toolUseOutcomes; + /// tool_use_ids whose error result should render folded + muted instead of as + /// a loud red failure (T-340) — expected, user-initiated denials the user + /// already understands (Deny & simplify). Genuine tool errors (ids not in + /// here) keep the prominent expanded-red treatment (T-168). A reusable filter: + /// add ids to quiet more error kinds without string-matching their text. + final Set quietErrorToolUseIds; + /// Whether to wrap the list in its own [ClideSelectionArea]. The team /// grid sets this false and wraps all tiles in one shared area so /// selection spans tiles — nesting SelectionAreas is illegal (T-140). @@ -305,6 +313,7 @@ class _ConversationViewState extends State { tokens: tokens, collapseTools: true, toolUseOutcomes: widget.toolUseOutcomes, + quietErrorToolUseIds: widget.quietErrorToolUseIds, toolUseById: widget.controller.toolUseById, resultByToolUseId: resultByToolUseId, promptsByToolUseId: fold.promptsByToolUseId, @@ -315,6 +324,7 @@ class _ConversationViewState extends State { items: items, tokens: tokens, toolUseOutcomes: widget.toolUseOutcomes, + quietErrorToolUseIds: widget.quietErrorToolUseIds, toolUseById: widget.controller.toolUseById, resultByToolUseId: resultByToolUseId, promptsByToolUseId: fold.promptsByToolUseId, @@ -325,6 +335,7 @@ class _ConversationViewState extends State { edits: edits, tokens: tokens, toolUseOutcomes: widget.toolUseOutcomes, + quietErrorToolUseIds: widget.quietErrorToolUseIds, toolUseById: widget.controller.toolUseById, resultByToolUseId: resultByToolUseId, promptsByToolUseId: fold.promptsByToolUseId, @@ -407,6 +418,7 @@ class _ConversationTurn extends StatelessWidget { required this.tokens, this.collapseTools = false, this.toolUseOutcomes = const {}, + this.quietErrorToolUseIds = const {}, this.toolUseById = const {}, this.resultByToolUseId = const {}, this.promptsByToolUseId = const >{}, @@ -427,6 +439,10 @@ class _ConversationTurn extends StatelessWidget { EdgeInsetsGeometry get _childMargin => collapseTools ? const EdgeInsets.only(bottom: 14) : const EdgeInsets.only(bottom: kClideCardHeaderPadH); final Map toolUseOutcomes; + /// tool_use_ids whose error result folds quietly instead of expanded-red + /// (T-340) — see [ConversationView.quietErrorToolUseIds]. + final Set quietErrorToolUseIds; + /// Index from toolUseId → AssistantToolUse, for result-card pairing (T-168). final Map toolUseById; @@ -627,6 +643,7 @@ class _ConversationTurn extends StatelessWidget { item: r, tokens: tokens, toolUseOutcomes: toolUseOutcomes, + quietErrorToolUseIds: quietErrorToolUseIds, toolUseById: toolUseById, resultByToolUseId: resultByToolUseId, promptsByToolUseId: promptsByToolUseId, @@ -717,22 +734,28 @@ class _ConversationTurn extends StatelessWidget { // Error result: render the error message prominently (T-168). If we have // the paired tool_use, show the tool name as a sub-label so the user can // see what failed without expanding. + // + // Exception (T-340): an expected, user-initiated denial (Deny & simplify) + // is noise as a loud red error — the user already knows what they did. Fold + // it to a muted, collapsed card. Genuine failures keep the expanded-red look. if (t.isError) { + final quiet = quietErrorToolUseIds.contains(t.toolUseId); final multiline = t.content.contains('\n'); + final errLabel = quiet ? 'denied' : label; return ConversationCard( variant: ConversationCardVariant.bordered, - accent: accent, - borderColor: tokens.statusError, - label: paired != null ? '${paired.name} · $label' : label, + accent: quiet ? tokens.globalTextMuted : accent, + borderColor: quiet ? tokens.panelBorder : tokens.statusError, + label: paired != null ? '${paired.name} · $errLabel' : errLabel, copyText: t.content, - collapsible: multiline, - collapsedByDefault: false, // errors default expanded so they're visible - collapsedSummary: multiline ? _firstLine(t.content) : null, + collapsible: quiet || multiline, + collapsedByDefault: quiet, // genuine errors stay expanded; a denial folds + collapsedSummary: (quiet || multiline) ? _firstLine(t.content) : null, body: ClideText( t.content, fontSize: clideFontMeta, fontFamily: clideMonoFamily, - color: tokens.statusError, + color: quiet ? tokens.globalTextMuted : tokens.statusError, ), ); } @@ -790,6 +813,7 @@ class _ActivityCard extends StatelessWidget { required this.items, required this.tokens, required this.toolUseOutcomes, + required this.quietErrorToolUseIds, required this.toolUseById, required this.resultByToolUseId, required this.promptsByToolUseId, @@ -799,6 +823,7 @@ class _ActivityCard extends StatelessWidget { final List items; final SurfaceTokens tokens; final Map toolUseOutcomes; + final Set quietErrorToolUseIds; final Map toolUseById; final Map resultByToolUseId; final Map> promptsByToolUseId; @@ -819,6 +844,7 @@ class _ActivityCard extends StatelessWidget { item: item, tokens: tokens, toolUseOutcomes: toolUseOutcomes, + quietErrorToolUseIds: quietErrorToolUseIds, toolUseById: toolUseById, resultByToolUseId: resultByToolUseId, promptsByToolUseId: promptsByToolUseId, @@ -851,6 +877,7 @@ class _EditRunCard extends StatelessWidget { required this.edits, required this.tokens, required this.toolUseOutcomes, + required this.quietErrorToolUseIds, required this.toolUseById, required this.resultByToolUseId, required this.promptsByToolUseId, @@ -860,6 +887,7 @@ class _EditRunCard extends StatelessWidget { final List edits; final SurfaceTokens tokens; final Map toolUseOutcomes; + final Set quietErrorToolUseIds; final Map toolUseById; final Map resultByToolUseId; final Map> promptsByToolUseId; @@ -880,6 +908,7 @@ class _EditRunCard extends StatelessWidget { item: item, tokens: tokens, toolUseOutcomes: toolUseOutcomes, + quietErrorToolUseIds: quietErrorToolUseIds, toolUseById: toolUseById, resultByToolUseId: resultByToolUseId, promptsByToolUseId: promptsByToolUseId, diff --git a/lib/builtin/claude/src/prompt_card.dart b/lib/builtin/claude/src/prompt_card.dart index b416aed1..9b8deeb2 100644 --- a/lib/builtin/claude/src/prompt_card.dart +++ b/lib/builtin/claude/src/prompt_card.dart @@ -234,7 +234,9 @@ class _ToolPromptCardState extends State { void _permDenySimplify() { final user = _permNote(); final note = user == null ? _kDenySimplifyNote : '$_kDenySimplifyNote\n\nUser note: $user'; - widget.onResolve(widget.prompt.promptId, DenyTool(note)); + // Quiet: the user deliberately chose this, so its denial folds rather than + // shouting as a red error (T-340). + widget.onResolve(widget.prompt.promptId, DenyTool(note, quiet: true)); } (Color, String, List) _permission(SurfaceTokens tokens) { diff --git a/lib/builtin/claude/src/stream_json_session.dart b/lib/builtin/claude/src/stream_json_session.dart index cabc2e10..d34091ac 100644 --- a/lib/builtin/claude/src/stream_json_session.dart +++ b/lib/builtin/claude/src/stream_json_session.dart @@ -183,9 +183,15 @@ final class AllowTool extends ToolDecision { } /// Deny the tool with a user-facing [message] (required by the protocol). +/// +/// [quiet] marks a deliberate, user-initiated denial that the user already +/// understands (e.g. "Deny & simplify", T-340) — the resulting error tool-result +/// should fold to a muted card rather than shout as a red failure. Off by +/// default, so a genuine/unexpected denial still renders prominently. final class DenyTool extends ToolDecision { - const DenyTool(this.message); + const DenyTool(this.message, {this.quiet = false}); final String message; + final bool quiet; @override Map toJson() => {'behavior': 'deny', 'message': message}; } @@ -246,9 +252,16 @@ class StreamJsonSession { /// green/red border (D-78). final _toolUseOutcome = {}; + /// tool_use_ids whose error result should render folded + muted rather than as + /// a loud red failure (T-340): expected, user-initiated denials (Deny & + /// simplify, today) that the user already understands. The reusable extension + /// point — add an id here at the moment you know its error is non-alarming. + final _quietErrorToolUses = {}; + /// Read-only views for the conversation view. Set get promptedToolUseIds => _promptedToolUses; Map get toolUseOutcomes => _toolUseOutcome; + Set get quietErrorToolUseIds => _quietErrorToolUses; /// Whether a turn is in flight (between a send and claude's `result`). Drives /// the composer's Stop affordance. @@ -542,7 +555,10 @@ class StreamJsonSession { final idx = _queue.indexWhere((p) => p.promptId == promptId); if (idx < 0) return; // unknown / already resolved final prompt = _queue.removeAt(idx); - if (prompt.toolUseId.isNotEmpty) _toolUseOutcome[prompt.toolUseId] = decision is AllowTool; + if (prompt.toolUseId.isNotEmpty) { + _toolUseOutcome[prompt.toolUseId] = decision is AllowTool; + if (decision is DenyTool && decision.quiet) _quietErrorToolUses.add(prompt.toolUseId); + } _proc.writeLine(jsonEncode({ 'type': 'control_response', 'response': {'subtype': 'success', 'request_id': promptId, 'response': decision.toJson()}, diff --git a/test/builtin/claude/conversation_view_test.dart b/test/builtin/claude/conversation_view_test.dart index e28cb29f..92782877 100644 --- a/test/builtin/claude/conversation_view_test.dart +++ b/test/builtin/claude/conversation_view_test.dart @@ -149,7 +149,10 @@ void main() { tearDown(() => f.dispose()); Future pumpWith(WidgetTester tester, List items, - {Set hiddenToolUseIds = const {}, Map toolUseOutcomes = const {}, FoldLevel foldLevel = FoldLevel.none}) async { + {Set hiddenToolUseIds = const {}, + Map toolUseOutcomes = const {}, + Set quietErrorToolUseIds = const {}, + FoldLevel foldLevel = FoldLevel.none}) async { tester.view.physicalSize = const Size(900, 700); tester.view.devicePixelRatio = 1.0; addTearDown(() { @@ -166,7 +169,12 @@ void main() { Builder( builder: (ctx) => MediaQuery( data: MediaQuery.of(ctx).copyWith(disableAnimations: true), - child: ConversationView(controller: c, hiddenToolUseIds: hiddenToolUseIds, toolUseOutcomes: toolUseOutcomes, foldLevel: foldLevel), + child: ConversationView( + controller: c, + hiddenToolUseIds: hiddenToolUseIds, + toolUseOutcomes: toolUseOutcomes, + quietErrorToolUseIds: quietErrorToolUseIds, + foldLevel: foldLevel), ), ), )); @@ -697,6 +705,33 @@ void main() { expect(find.text('Bash · error'), findsOneWidget); }); + testWidgets('a quiet (user-initiated) denial folds to a muted "denied" card (T-340)', (tester) async { + await pumpWith( + tester, + [ + _tool('Bash', {'command': 'rm -rf x'}), + _result('Denied — too complex\nretry simpler', isError: true), + ], + quietErrorToolUseIds: {'x1'}, // the toolUseId shared by _tool/_result + ); + // Labelled "denied", not the loud "error", and folded by default so the + // body (its second line) is hidden behind a collapsed summary. + expect(find.text('Bash · denied'), findsOneWidget); + expect(find.text('Bash · error'), findsNothing); + expect(find.textContaining('retry simpler'), findsNothing); + }); + + testWidgets('a genuine error (not in the quiet set) stays expanded red (T-340)', (tester) async { + await pumpWith(tester, [ + _tool('Bash', {'command': 'rm -rf x'}), + _result('Denied — too complex\nretry simpler', isError: true), + ]); + // Same content, but not flagged quiet → the normal expanded error path. + expect(find.text('Bash · error'), findsOneWidget); + expect(find.text('Bash · denied'), findsNothing); + expect(find.textContaining('retry simpler'), findsOneWidget); + }); + testWidgets('result without a paired tool_use uses plain "result" label (T-168)', (tester) async { // Orphan result (no matching tool_use in the controller). await pumpWith(tester, [