diff --git a/CHANGELOG.md b/CHANGELOG.md index 1043dde0..875ac7cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,14 +18,19 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Added -- Native permission & AskUserQuestion prompts (T-166, D-78) — when Claude - needs tool approval or asks a question, an inline card appears in the - conversation with Allow/Deny or selectable options; the decision is - returned over the stream-json control channel. Closes the prompt gap - the tmux model couldn't surface. +- Native permission & AskUserQuestion prompts (T-166, T-175, T-176, D-78) — + when Claude needs tool approval or asks a question, the composer is + replaced by a prompt: Allow / Allow-and-don't-ask-again / Deny for + permissions (with an optional note), and a single-question or + stepped-with-review option picker for AskUserQuestion (with "Other" + free-text and per-choice notes). Closes the prompt gap the tmux model + couldn't surface. - Conversation message cards (T-173) — every turn in the Claude pane now renders through one card template with a copy button on hover and a collapse/expand caret for tool calls, results, and thinking. +- Collapsed-by-default tool cards (T-177) — multi-line tool calls and + results start collapsed behind a one-line summary; one-line output stays + inline so a caret never hides a single line. - Claude meta sidebar (T-141, T-157) — an always-pickable left-panel tab showing Claude activity (the latest day's messages/sessions/tool-calls plus lifetime totals, from `stats-cache.json`) and, when a tmux team is diff --git a/lib/builtin/claude/src/conversation_card.dart b/lib/builtin/claude/src/conversation_card.dart index 2bafe55e..4153b9a5 100644 --- a/lib/builtin/claude/src/conversation_card.dart +++ b/lib/builtin/claude/src/conversation_card.dart @@ -37,6 +37,7 @@ class ConversationCard extends StatefulWidget { this.actions = const [], this.collapsible = false, this.collapsedByDefault = false, + this.collapsedSummary, this.borderColor, }); @@ -54,6 +55,11 @@ class ConversationCard extends StatefulWidget { final bool collapsible; final bool collapsedByDefault; + /// One-line gist shown next to the label while collapsed (e.g. the tool's + /// key arg, or a result's first line), so a collapsed card still says what + /// it holds. Null → just the label. + final String? collapsedSummary; + /// Border colour for the bordered variant (e.g. error red); defaults to the /// panel border. final Color? borderColor; @@ -127,11 +133,20 @@ class _ConversationCardState extends State { } Widget _header(SurfaceTokens tokens) { + final summary = widget.collapsedSummary; return Row( children: [ if (widget.collapsible) _caret(tokens), ClideText(widget.label, fontSize: clideFontSmall, color: widget.accent, fontFamily: clideMonoFamily), - const Spacer(), + // While collapsed, show a one-line gist next to the label so the card + // still says what it holds. + if (_collapsed && summary != null) ...[ + const SizedBox(width: 10), + Expanded( + child: ClideText(summary, fontSize: clideFontMeta, color: tokens.globalTextMuted, fontFamily: clideMonoFamily, maxLines: 1), + ), + ] else + const Spacer(), // Hover-revealed actions. (Always-reachable keyboard a11y for these is // a follow-up detail; the collapse caret above is always visible.) if (_hover) ..._actions(tokens), diff --git a/lib/builtin/claude/src/conversation_view.dart b/lib/builtin/claude/src/conversation_view.dart index c85cd89c..e86bd8fe 100644 --- a/lib/builtin/claude/src/conversation_view.dart +++ b/lib/builtin/claude/src/conversation_view.dart @@ -149,26 +149,34 @@ class _ConversationTurn extends StatelessWidget { Widget _toolUse(AssistantToolUse t) { final pretty = const JsonEncoder.withIndent(' ').convert(t.input); + // Collapse only the bulky multi-line form; a trivial one-liner just shows. + final multiline = pretty.contains('\n'); return ConversationCard( variant: ConversationCardVariant.bordered, accent: tokens.globalFocus, label: t.name, copyText: pretty, - collapsible: true, + collapsible: multiline, + collapsedByDefault: multiline, + collapsedSummary: multiline ? _toolUseSummary(t) : null, body: ClideCodeBlock(source: pretty, language: 'json'), ); } Widget _toolResult(ToolResultMessage t) { final accent = t.isError ? tokens.statusError : tokens.globalTextMuted; + // A one-line result is all chrome to collapse — show it inline. Only fold + // away multi-line output, behind a summary of its first line. + final multiline = t.content.contains('\n'); return ConversationCard( variant: ConversationCardVariant.bordered, accent: accent, borderColor: t.isError ? tokens.statusError : tokens.panelBorder, label: t.isError ? 'error' : 'result', copyText: t.content, - collapsible: true, - collapsedByDefault: true, + collapsible: multiline, + collapsedByDefault: multiline, + collapsedSummary: multiline ? _firstLine(t.content) : null, body: ClideText( t.content, fontSize: clideFontMeta, @@ -177,4 +185,18 @@ class _ConversationTurn extends StatelessWidget { ), ); } + + /// A compact one-liner for a collapsed tool-use card: the most telling arg. + String _toolUseSummary(AssistantToolUse t) { + final input = t.input; + final key = + input['file_path'] ?? input['command'] ?? input['path'] ?? input['pattern'] ?? input['url'] ?? (input.values.isNotEmpty ? input.values.first : null); + final s = key?.toString().replaceAll('\n', ' ') ?? ''; + return s.length > 80 ? '${s.substring(0, 80)}…' : s; + } + + String _firstLine(String content) { + final line = content.split('\n').first.trim(); + return line.length > 80 ? '${line.substring(0, 80)}…' : line; + } } diff --git a/lib/builtin/claude/src/prompt_card.dart b/lib/builtin/claude/src/prompt_card.dart index ba87f6ad..76dd327d 100644 Binary files a/lib/builtin/claude/src/prompt_card.dart and b/lib/builtin/claude/src/prompt_card.dart differ diff --git a/lib/builtin/claude/src/stream_json_session.dart b/lib/builtin/claude/src/stream_json_session.dart index 12055825..55679217 100644 --- a/lib/builtin/claude/src/stream_json_session.dart +++ b/lib/builtin/claude/src/stream_json_session.dart @@ -91,6 +91,7 @@ class ToolPrompt { required this.input, this.description, this.toolUseId = '', + this.permissionSuggestions = const [], }); /// The control_request `request_id` — the key passed to [StreamJsonSession.resolvePrompt]. @@ -111,6 +112,11 @@ class ToolPrompt { /// The tool's proposed input — echoed back (possibly modified) on allow. final Map input; + /// Permission-rule suggestions from the request (e.g. a `setMode` / + /// `localSettings` entry). Non-empty → an "allow & don't ask again" path is + /// available; echo a chosen entry back as `updatedPermissions` (D-78). + final List permissionSuggestions; + /// AskUserQuestion is answered through the same channel (D-78). bool get isQuestion => toolName == 'AskUserQuestion'; } @@ -124,11 +130,22 @@ sealed class ToolDecision { /// Allow the tool. [updatedInput] is REQUIRED by the protocol — pass the /// request's input unchanged to allow as-is, or modified to alter the call. /// For AskUserQuestion, include the `answers` map (question text → label). +/// +/// [updatedPermissions] echoes a permission suggestion back to skip future +/// prompts ("don't ask again"). [followUpNote] is NOT part of the protocol — +/// the protocol has no allow-with-message — so the session sends it as a +/// separate user message right after allowing (D-78). final class AllowTool extends ToolDecision { - const AllowTool(this.updatedInput); + const AllowTool(this.updatedInput, {this.updatedPermissions, this.followUpNote}); final Map updatedInput; + final List? updatedPermissions; + final String? followUpNote; @override - Map toJson() => {'behavior': 'allow', 'updatedInput': updatedInput}; + Map toJson() => { + 'behavior': 'allow', + 'updatedInput': updatedInput, + if (updatedPermissions != null && updatedPermissions!.isNotEmpty) 'updatedPermissions': updatedPermissions, + }; } /// Deny the tool with a user-facing [message] (required by the protocol). @@ -218,6 +235,7 @@ class StreamJsonSession { description: request['description'] as String?, toolUseId: request['tool_use_id'] as String? ?? '', input: input, + permissionSuggestions: (request['permission_suggestions'] as List?) ?? const [], )); _pendingCtl.add(pendingPrompt); return; // awaits resolvePrompt @@ -232,13 +250,27 @@ class StreamJsonSession { /// [ToolPrompt.promptId]. No-op if unknown or already resolved. Advances the /// queue so the next pending prompt (if any) surfaces. void resolvePrompt(String promptId, ToolDecision decision) { - final before = _queue.length; - _queue.removeWhere((p) => p.promptId == promptId); - if (_queue.length == before) return; // unknown / already resolved + final idx = _queue.indexWhere((p) => p.promptId == promptId); + if (idx < 0) return; // unknown / already resolved + final prompt = _queue.removeAt(idx); _proc.writeLine(jsonEncode({ 'type': 'control_response', 'response': {'subtype': 'success', 'request_id': promptId, 'response': decision.toJson()}, })); + if (decision is AllowTool) { + // The prompt card is ephemeral (it vanishes once resolved), so leave a + // compact record of an answered question in the conversation log (D-78). + if (prompt.isQuestion) { + final answers = decision.updatedInput['answers']; + if (answers is Map && answers.isNotEmpty) { + final summary = answers.entries.map((e) => '${e.key} → ${e.value}').join('; '); + _items.add(UserMessage(uuid: 'local-${_localSeq++}', timestamp: DateTime.now(), isSidechain: false, text: '✓ answered: $summary')); + } + } + // The protocol has no allow-with-message, so an allow note rides as a + // follow-up user message right after the approval (D-78). + if (decision.followUpNote?.trim().isNotEmpty ?? false) send(decision.followUpNote!.trim()); + } _pendingCtl.add(pendingPrompt); } diff --git a/test/builtin/claude/conversation_view_test.dart b/test/builtin/claude/conversation_view_test.dart index 1c07fa0f..72e80821 100644 --- a/test/builtin/claude/conversation_view_test.dart +++ b/test/builtin/claude/conversation_view_test.dart @@ -170,6 +170,24 @@ void main() { expect(find.text('error'), findsOneWidget); }); + testWidgets('a one-line tool result renders inline (no collapse caret)', (tester) async { + await pumpWith(tester, [_result('hello-from-spike')]); + expect(find.text('hello-from-spike'), findsOneWidget); + expect(find.byType(ClideIcon), findsNothing); // not collapsible → no caret + }); + + testWidgets('a multi-line tool result starts collapsed with a first-line summary', (tester) async { + await pumpWith(tester, [_result('first line\nsecond line\nthird line')]); + // Collapsed: caret present, summary (first line) shown, full body hidden. + expect(find.byType(ClideIcon), findsOneWidget); + expect(find.text('first line'), findsOneWidget); + expect(find.text('first line\nsecond line\nthird line'), findsNothing); + + await tester.tap(find.byType(ClideIcon)); + await tester.pump(); + expect(find.text('first line\nsecond line\nthird line'), findsOneWidget); + }); + testWidgets('select-all + copy spans multiple cards', (tester) async { final clipboard = _MockClipboard(); TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMethodCallHandler(SystemChannels.platform, clipboard.handleMethodCall); diff --git a/test/builtin/claude/prompt_card_test.dart b/test/builtin/claude/prompt_card_test.dart index 0f3da171..9be7aee0 100644 --- a/test/builtin/claude/prompt_card_test.dart +++ b/test/builtin/claude/prompt_card_test.dart @@ -1,16 +1,18 @@ import 'package:clide/builtin/claude/src/prompt_card.dart'; import 'package:clide/builtin/claude/src/stream_json_session.dart'; +import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; import '../../helpers/kernel_fixture.dart'; import '../../helpers/widget_harness.dart'; -ToolPrompt permissionPrompt() => const ToolPrompt( +ToolPrompt permissionPrompt({List suggestions = const []}) => ToolPrompt( promptId: 'req-1', toolName: 'Write', displayName: 'Write', description: 'banana.txt', - input: {'file_path': '/tmp/banana.txt', 'content': 'banana'}, + input: const {'file_path': '/tmp/banana.txt', 'content': 'banana'}, + permissionSuggestions: suggestions, ); ToolPrompt questionPrompt({bool multi = false}) => ToolPrompt( @@ -32,6 +34,34 @@ ToolPrompt questionPrompt({bool multi = false}) => ToolPrompt( }, ); +ToolPrompt twoQuestionPrompt() => const ToolPrompt( + promptId: 'req-2q', + toolName: 'AskUserQuestion', + displayName: 'AskUserQuestion', + input: { + 'questions': [ + { + 'question': 'Which pet?', + 'header': 'Pet', + 'multiSelect': false, + 'options': [ + {'label': 'Cats', 'description': ''}, + {'label': 'Dogs', 'description': ''}, + ], + }, + { + 'question': 'How eaten?', + 'header': 'Eaten', + 'multiSelect': false, + 'options': [ + {'label': 'Fresh', 'description': ''}, + {'label': 'Smoothie', 'description': ''}, + ], + }, + ], + }, + ); + void main() { late KernelFixture f; setUp(() async => f = await KernelFixture.create()); @@ -79,6 +109,51 @@ void main() { expect((decision as DenyTool).message, isNotEmpty); }); + testWidgets('permission: no "don\'t ask again" button without a suggestion', (tester) async { + await tester.pumpWidget(harness(f, ToolPromptCard(prompt: permissionPrompt(), onResolve: (_, __) {}))); + await tester.pump(); + expect(find.text("Allow & don't ask again"), findsNothing); + }); + + testWidgets('permission: "don\'t ask again" shows with a suggestion and returns updatedPermissions', (tester) async { + ToolDecision? decision; + const sugg = [ + {'type': 'setMode', 'mode': 'acceptEdits', 'destination': 'session'} + ]; + await tester.pumpWidget(harness( + f, + ToolPromptCard(prompt: permissionPrompt(suggestions: sugg), onResolve: (_, d) => decision = d), + )); + await tester.pump(); + + expect(find.text("Allow & don't ask again"), findsOneWidget); + await tester.tap(find.text("Allow & don't ask again")); + await tester.pump(); + expect((decision as AllowTool).updatedPermissions, hasLength(1)); + }); + + testWidgets('permission: a typed note rides Deny as the message', (tester) async { + ToolDecision? decision; + await tester.pumpWidget(harness(f, ToolPromptCard(prompt: permissionPrompt(), onResolve: (_, d) => decision = d))); + await tester.pump(); + await tester.enterText(find.byType(EditableText), 'write it under docs/ instead'); + await tester.pump(); + await tester.tap(find.text('Deny')); + await tester.pump(); + expect((decision as DenyTool).message, 'write it under docs/ instead'); + }); + + testWidgets('permission: a typed note rides Allow as a follow-up note', (tester) async { + ToolDecision? decision; + await tester.pumpWidget(harness(f, ToolPromptCard(prompt: permissionPrompt(), onResolve: (_, d) => decision = d))); + await tester.pump(); + await tester.enterText(find.byType(EditableText), 'fyi: sandbox only'); + await tester.pump(); + await tester.tap(find.text('Allow')); + await tester.pump(); + expect((decision as AllowTool).followUpNote, 'fyi: sandbox only'); + }); + testWidgets('question card: Submit is gated until an option is picked, then returns answers', (tester) async { ToolDecision? decision; await tester.pumpWidget(harness( @@ -122,4 +197,78 @@ void main() { final answers = (decision as AllowTool).updatedInput['answers'] as Map; expect(answers['Do you prefer cats or dogs?'], 'Cats, Dogs'); }); + + testWidgets('question card: "Other…" free-text becomes the answer value', (tester) async { + ToolDecision? decision; + await tester.pumpWidget(harness(f, ToolPromptCard(prompt: questionPrompt(), onResolve: (_, d) => decision = d))); + await tester.pump(); + + await tester.tap(find.text('○ Other…')); + await tester.pump(); + // Two fields now: [0] = the Other free-text, [1] = the per-choice note. + await tester.enterText(find.byType(EditableText).first, 'Kiwi'); + await tester.pump(); + await tester.tap(find.text('Submit')); + await tester.pump(); + + final answers = (decision as AllowTool).updatedInput['answers'] as Map; + expect(answers['Do you prefer cats or dogs?'], 'Kiwi'); // not the word "Other" + }); + + testWidgets('question card: a per-choice note is appended to the label', (tester) async { + ToolDecision? decision; + await tester.pumpWidget(harness(f, ToolPromptCard(prompt: questionPrompt(), onResolve: (_, d) => decision = d))); + await tester.pump(); + + await tester.tap(find.textContaining('Dogs')); + await tester.pump(); + await tester.enterText(find.byType(EditableText), 'only big ones'); // the note field + await tester.pump(); + await tester.tap(find.text('Submit')); + await tester.pump(); + + final answers = (decision as AllowTool).updatedInput['answers'] as Map; + expect(answers['Do you prefer cats or dogs?'], 'Dogs — only big ones'); + }); + + testWidgets('multi-question: steps through to review, then submits both answers', (tester) async { + ToolDecision? decision; + await tester.pumpWidget(harness(f, ToolPromptCard(prompt: twoQuestionPrompt(), onResolve: (_, d) => decision = d))); + await tester.pump(); + + // Stepper nav shows numbered headers; only question 1 is visible. + expect(find.textContaining('1 · Pet'), findsOneWidget); + expect(find.text('Which pet?'), findsOneWidget); + expect(find.text('How eaten?'), findsNothing); + + await tester.tap(find.textContaining('Dogs')); + await tester.pump(); + await tester.tap(find.text('Next ›')); + await tester.pump(); + + expect(find.text('How eaten?'), findsOneWidget); + await tester.tap(find.textContaining('Fresh')); + await tester.pump(); + await tester.tap(find.text('Review ›')); + await tester.pump(); + + // Review screen lists both answers; submit delivers them. + expect(find.text('Review your answers'), findsOneWidget); + await tester.tap(find.text('Submit answers')); + await tester.pump(); + + final answers = (decision as AllowTool).updatedInput['answers'] as Map; + expect(answers['Which pet?'], 'Dogs'); + expect(answers['How eaten?'], 'Fresh'); + }); + + testWidgets('question card: "chat instead" denies the prompt', (tester) async { + ToolDecision? decision; + await tester.pumpWidget(harness(f, ToolPromptCard(prompt: questionPrompt(), onResolve: (_, d) => decision = d))); + await tester.pump(); + + await tester.tap(find.text('chat instead')); + await tester.pump(); + expect(decision, isA()); + }); } diff --git a/test/builtin/claude/stream_json_session_test.dart b/test/builtin/claude/stream_json_session_test.dart index b69541a2..ebd3366f 100644 --- a/test/builtin/claude/stream_json_session_test.dart +++ b/test/builtin/claude/stream_json_session_test.dart @@ -179,6 +179,50 @@ void main() { expect((decision['updatedInput'] as Map)['content'], 'banana'); }); + test('a permission request carries its permission_suggestions', () async { + proc.emit(jsonEncode({ + 'type': 'control_request', + 'request_id': 'rs', + 'request': { + 'subtype': 'can_use_tool', + 'tool_name': 'Write', + 'input': {'file_path': '/tmp/x'}, + 'permission_suggestions': [ + {'type': 'setMode', 'mode': 'acceptEdits', 'destination': 'session'} + ], + }, + })); + await Future.delayed(Duration.zero); + expect(session.pendingPrompt!.permissionSuggestions, hasLength(1)); + }); + + test('resolvePrompt(allow with updatedPermissions) echoes them in the response', () async { + proc.emit(canUseTool('rp')); + await Future.delayed(Duration.zero); + session.resolvePrompt( + 'rp', + AllowTool(const { + 'x': 1 + }, updatedPermissions: const [ + {'type': 'setMode'} + ])); + final decision = ((jsonDecode(proc.writes.single) as Map)['response'] as Map)['response'] as Map; + expect(decision['behavior'], 'allow'); + expect(decision['updatedPermissions'], hasLength(1)); + }); + + test('resolvePrompt(allow with a follow-up note) sends the note as a user message', () async { + proc.emit(canUseTool('rn')); + await Future.delayed(Duration.zero); + session.resolvePrompt('rn', AllowTool(const {'x': 1}, followUpNote: 'use docs/ instead')); + + // first write = control_response (allow), second = the follow-up message + expect(proc.writes, hasLength(2)); + final follow = jsonDecode(proc.writes[1]) as Map; + expect(follow['type'], 'user'); + expect((follow['message'] as Map)['content'], 'use docs/ instead'); + }); + test('resolvePrompt(deny) writes a deny decision with a message', () async { proc.emit(canUseTool('req-3')); await Future.delayed(Duration.zero); @@ -189,6 +233,29 @@ void main() { expect(decision['message'], 'nope'); }); + test('resolving an AskUserQuestion leaves an answered echo in the log', () async { + proc.emit(jsonEncode({ + 'type': 'control_request', + 'request_id': 'aq', + 'request': { + 'subtype': 'can_use_tool', + 'tool_name': 'AskUserQuestion', + 'input': {'questions': []}, + }, + })); + await Future.delayed(Duration.zero); + session.resolvePrompt( + 'aq', + AllowTool(const { + 'answers': {'Pet': 'Dogs'} + })); + await Future.delayed(Duration.zero); + + final echo = items.whereType().toList(); + expect(echo, hasLength(1)); + expect(echo.single.text, contains('Pet → Dogs')); + }); + test('prompts queue: resolving the head surfaces the next', () async { proc.emit(canUseTool('q1')); proc.emit(canUseTool('q2'));