diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index 08f153a4..baa335b2 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -4035,3 +4035,5 @@ Fix: either wire the tiles (clone flow; open a Claude pane) and register the sho Acceptance: every tile on the welcome screen performs its action (widget test taps each); every shortcut shown is registered in the keymap. Resolved by removal, not stub flows: the two inert tiles are gone (each returns with its real flow — clone is honorable-mention territory in Q-49), the Open-folder shortcut glyph now matches the actual ctrl+o binding, and the tips card was corrected to six bindings that actually exist (quick open, palette, sidebar/context collapse, find-in-files, focus mode — the old card advertised four bindings that were never registered).', NULL, '2026-06-11 23:24:39', '2026-06-11 23:24:39', '2026-06-11 23:24:39', NULL, '53b1878fe9fd9c383d11bade928e811d', 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 ('06FBHDAK04ZA0PBT69ZWNBXPSR', 'status', 'backlog', 'in_progress', NULL, '2026-06-11 23:25:05', '2026-06-11 23:25:05', '2026-06-11 23:25:05', NULL, '9da97fcbea83b39c381d63cb06ea7b50', 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 ('06FBHDAK04ZA0PBT69ZWNBXPSR', 'status', 'in_progress', 'done', NULL, '2026-06-11 23:28:30', '2026-06-11 23:28:30', '2026-06-11 23:28:30', NULL, 'bd49ff4ff2fa7892ab5b667e46236300', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index b9356318..cfea8b6a 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -4183,3 +4183,13 @@ Fix: either wire the tiles (clone flow; open a Claude pane) and register the sho Acceptance: every tile on the welcome screen performs its action (widget test taps each); every shortcut shown is registered in the keymap. Resolved by removal, not stub flows: the two inert tiles are gone (each returns with its real flow — clone is honorable-mention territory in Q-49), the Open-folder shortcut glyph now matches the actual ctrl+o binding, and the tips card was corrected to six bindings that actually exist (quick open, palette, sidebar/context collapse, find-in-files, focus mode — the old card advertised four bindings that were never registered).', 'done', 'medium', NULL, NULL, NULL, '2026-06-11 22:00:14', '2026-06-11 23:24:39', NULL, '49859fb995fa3e1dbd916a153c651c40', 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 ('06FBHDAK04ZA0PBT69ZWNBXPSR', 'bug', '06FBHBGHNEQTAEPGNJKN42C1E8', 'claude builtin returns ok with an error payload in 16 handlers — honor the D-6 exit-code contract', '16 handlers in the claude builtin return result: ok with an error field in the payload instead of an error envelope, drifting from the D-6 exit-code contract every other subsystem honors. Scripted example: clide claude.agent.set-permission-mode bogus exits 0 today, so scripts cannot detect failure. + +Fix: sweep the claude builtin handlers; on failure return the error envelope (non-zero CLI exit) like the rest of the dispatcher. Audit callers/UI that may currently rely on ok-with-error. + +Acceptance: clide claude.agent.set-permission-mode bogus exits non-zero; a table-driven test walks the claude verbs'' failure paths asserting error envelopes; D-6 conformance restored.', 'in_progress', 'medium', NULL, NULL, NULL, '2026-06-11 22:02:31', '2026-06-11 23:25:05', NULL, 'd51d7435910cc30ef2b8893ae05ba6a1', 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 ('06FBHDAK04ZA0PBT69ZWNBXPSR', 'bug', '06FBHBGHNEQTAEPGNJKN42C1E8', 'claude builtin returns ok with an error payload in 16 handlers — honor the D-6 exit-code contract', '16 handlers in the claude builtin return result: ok with an error field in the payload instead of an error envelope, drifting from the D-6 exit-code contract every other subsystem honors. Scripted example: clide claude.agent.set-permission-mode bogus exits 0 today, so scripts cannot detect failure. + +Fix: sweep the claude builtin handlers; on failure return the error envelope (non-zero CLI exit) like the rest of the dispatcher. Audit callers/UI that may currently rely on ok-with-error. + +Acceptance: clide claude.agent.set-permission-mode bogus exits non-zero; a table-driven test walks the claude verbs'' failure paths asserting error envelopes; D-6 conformance restored.', 'done', 'medium', NULL, NULL, NULL, '2026-06-11 22:02:31', '2026-06-11 23:28:30', NULL, 'd3e727b4dd8848404845c9cf5af32891', 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 96dde1c1..125dda4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -132,6 +132,11 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. warnings) accumulated in a list no surface rendered; they now raise toasts with matching severity. (T-382) +- **Failed `clide claude.*` commands now exit non-zero.** Sixteen handlers + reported success with an error message buried in the payload, so scripts + could not detect failures like an unknown permission mode; they now + return proper error envelopes per the D-6 contract. (T-391) + - **Accepting ExitPlanMode now leaves plan mode in the conversation panel.** Approving Claude's plan (the ExitPlanMode tool) transitioned the underlying session out of plan mode, but clide's tracked permission mode didn't follow, diff --git a/lib/builtin/claude/src/extension.dart b/lib/builtin/claude/src/extension.dart index 37129b35..d6efc788 100644 --- a/lib/builtin/claude/src/extension.dart +++ b/lib/builtin/claude/src/extension.dart @@ -21,6 +21,13 @@ import 'package:clide/kernel/kernel.dart'; import 'package:clide/widgets/widgets.dart'; import 'package:flutter/widgets.dart'; +/// D-6 contract (T-391): a failed command returns an ERROR envelope (non-zero +/// CLI exit), never `ok` with an `error` field a script can't detect. +IpcResponse _userErr(String msg, {String? hint}) => + IpcResponse.err(id: '', error: IpcError(code: IpcExitCode.userError, kind: IpcErrorKind.userError, message: msg, hint: hint)); + +IpcResponse _notFound(String msg) => IpcResponse.err(id: '', error: IpcError(code: IpcExitCode.notFound, kind: IpcErrorKind.notFound, message: msg)); + class ClaudeExtension extends ClideExtension { @override String get id => 'builtin.claude'; @@ -95,7 +102,7 @@ class ClaudeExtension extends ClideExtension { title: 'Claude: show an agent session pane', run: (args) async { final id = args.firstOrNull; - if (id == null) return IpcResponse.ok(id: '', data: const {'error': 'missing session id'}); + if (id == null) return _userErr('missing session id'); _orchestrator?.show(id); return IpcResponse.ok(id: '', data: {'id': id, 'status': 'shown'}); }, @@ -106,7 +113,7 @@ class ClaudeExtension extends ClideExtension { title: 'Claude: hide an agent session pane', run: (args) async { final id = args.firstOrNull; - if (id == null) return IpcResponse.ok(id: '', data: const {'error': 'missing session id'}); + if (id == null) return _userErr('missing session id'); _orchestrator?.hide(id); return IpcResponse.ok(id: '', data: {'id': id, 'status': 'hidden'}); }, @@ -117,7 +124,7 @@ class ClaudeExtension extends ClideExtension { title: 'Claude: close (kill) an agent session', run: (args) async { final id = args.firstOrNull; - if (id == null) return IpcResponse.ok(id: '', data: const {'error': 'missing session id'}); + if (id == null) return _userErr('missing session id'); await _orchestrator?.close(id); return IpcResponse.ok(id: '', data: {'id': id, 'status': 'closed'}); }, @@ -128,7 +135,7 @@ class ClaudeExtension extends ClideExtension { title: 'Claude: mute broker delivery to an agent session', run: (args) async { final id = args.firstOrNull; - if (id == null) return IpcResponse.ok(id: '', data: const {'error': 'missing session id'}); + if (id == null) return _userErr('missing session id'); _orchestrator?.mute(id); return IpcResponse.ok(id: '', data: {'id': id, 'status': 'muted'}); }, @@ -139,7 +146,7 @@ class ClaudeExtension extends ClideExtension { title: 'Claude: unmute broker delivery to an agent session', run: (args) async { final id = args.firstOrNull; - if (id == null) return IpcResponse.ok(id: '', data: const {'error': 'missing session id'}); + if (id == null) return _userErr('missing session id'); _orchestrator?.unmute(id); return IpcResponse.ok(id: '', data: {'id': id, 'status': 'unmuted'}); }, @@ -151,9 +158,9 @@ class ClaudeExtension extends ClideExtension { title: 'Claude: inject a text turn into an agent session', run: (args) async { final id = args.firstOrNull; - if (id == null) return IpcResponse.ok(id: '', data: const {'error': 'missing session id'}); + if (id == null) return _userErr('missing session id'); final text = args.skip(1).join(' '); - if (text.isEmpty) return IpcResponse.ok(id: '', data: const {'error': 'missing message text'}); + if (text.isEmpty) return _userErr('missing message text'); _orchestrator?.injectMessage(id, text); return IpcResponse.ok(id: '', data: {'id': id, 'status': 'injected'}); }, @@ -169,12 +176,12 @@ class ClaudeExtension extends ClideExtension { title: 'Claude: set permission mode for an agent session', run: (args) async { final id = args.firstOrNull; - if (id == null) return IpcResponse.ok(id: '', data: const {'error': 'missing session id'}); + if (id == null) return _userErr('missing session id'); final mode = args.length >= 2 ? args[1] : null; - if (mode == null) return IpcResponse.ok(id: '', data: const {'error': 'missing mode (default|acceptEdits|plan|bypassPermissions)'}); + if (mode == null) return _userErr('missing mode (default|acceptEdits|plan|bypassPermissions)'); const valid = {'default', 'acceptEdits', 'plan', 'bypassPermissions'}; if (!valid.contains(mode)) { - return IpcResponse.ok(id: '', data: {'error': 'unknown mode "$mode"; use one of: ${valid.join(', ')}'}); + return _userErr('unknown mode "$mode"; use one of: ${valid.join(', ')}'); } _orchestrator?.byId(id)?.session.setPermissionMode(mode); return IpcResponse.ok(id: '', data: {'id': id, 'mode': mode, 'status': 'sent'}); @@ -188,7 +195,7 @@ class ClaudeExtension extends ClideExtension { title: 'Claude: Cycle permission mode', run: (_) async { final managed = _orchestrator?.byId('primary'); - if (managed == null) return IpcResponse.ok(id: '', data: const {'error': 'no primary session'}); + if (managed == null) return _notFound('no primary session'); final next = nextSafePermissionMode(managed.session.status.permissionMode ?? 'default'); managed.session.setPermissionMode(next); return IpcResponse.ok(id: '', data: {'mode': next, 'status': 'sent'}); @@ -200,11 +207,12 @@ class ClaudeExtension extends ClideExtension { command: 'claude.task.reassign', title: 'Claude: reassign a shared task to an agent', run: (args) async { - if (args.length < 2) return IpcResponse.ok(id: '', data: const {'error': 'usage: '}); + if (args.length < 2) return _userErr('usage: '); final taskId = args[0]; final toId = args[1]; final ok = _orchestrator?.broker.reassignTask(taskId, toId) ?? false; - return IpcResponse.ok(id: '', data: {'taskId': taskId, 'toId': toId, 'ok': ok}); + if (!ok) return _notFound('could not reassign task "$taskId" to "$toId"'); + return IpcResponse.ok(id: '', data: {'taskId': taskId, 'toId': toId, 'ok': true}); }, ), // T-180: full team chat pane opened as a workspace tab. @@ -241,7 +249,7 @@ class ClaudeExtension extends ClideExtension { command: 'claude.team-chat.post', title: 'Claude: post a message into the team channel as the user', run: (args) async { - if (args.isEmpty) return IpcResponse.ok(id: '', data: const {'error': 'usage: [@name] '}); + if (args.isEmpty) return _userErr('usage: [@name] '); final raw = args.join(' '); String? recipient; String body = raw; @@ -269,15 +277,18 @@ class ClaudeExtension extends ClideExtension { run: (args) async { final sourceId = args.firstOrNull; if (sourceId == null) { - return IpcResponse.ok(id: '', data: const {'error': 'usage: claude.agent.fork []'}); + return _userErr('usage: claude.agent.fork []'); } final orch = _orchestrator; if (orch == null) { - return IpcResponse.ok(id: '', data: const {'error': 'orchestrator unavailable'}); + return IpcResponse.err( + id: '', + error: IpcError(code: IpcExitCode.toolError, kind: IpcErrorKind.toolError, message: 'orchestrator unavailable'), + ); } final source = orch.byId(sourceId); if (source == null) { - return IpcResponse.ok(id: '', data: {'error': 'unknown session "$sourceId"'}); + return _notFound('unknown session "$sourceId"'); } final cwd = args.length >= 2 ? args[1] : source.cwd; final forkId = 'fork:$sourceId-${DateTime.now().millisecondsSinceEpoch}'; diff --git a/test/builtin/claude/extension_commands_test.dart b/test/builtin/claude/extension_commands_test.dart new file mode 100644 index 00000000..4b2aa761 --- /dev/null +++ b/test/builtin/claude/extension_commands_test.dart @@ -0,0 +1,47 @@ +/// T-391: the claude builtin's command handlers must honor the D-6 +/// exit-code contract — a failure is an ERROR envelope (non-zero CLI +/// exit), never `ok` with an `error` field a script can't detect. +/// `clide claude.agent.set-permission-mode bogus` exited 0 before this. +library; + +import 'package:clide/builtin/claude/src/extension.dart'; +import 'package:clide/clide.dart'; +import 'package:clide/extension/extension.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + final ext = ClaudeExtension(); + CommandContribution cmd(String id) => ext.contributions.whereType().firstWhere((c) => c.id == id); + + group('failure paths return error envelopes (T-391, D-6)', () { + // (command id, args, expected error kind) + final cases = <(String, List, String)>[ + ('claude.agent.show', [], IpcErrorKind.userError), + ('claude.agent.hide', [], IpcErrorKind.userError), + ('claude.agent.close', [], IpcErrorKind.userError), + ('claude.agent.mute', [], IpcErrorKind.userError), + ('claude.agent.unmute', [], IpcErrorKind.userError), + ('claude.agent.inject-message', [], IpcErrorKind.userError), + ('claude.agent.inject-message', ['some-id'], IpcErrorKind.userError), + ('claude.agent.set-permission-mode', [], IpcErrorKind.userError), + ('claude.agent.set-permission-mode', ['some-id'], IpcErrorKind.userError), + ('claude.agent.set-permission-mode', ['some-id', 'bogus'], IpcErrorKind.userError), + ('claude.mode.cycle', [], IpcErrorKind.notFound), + ('claude.task.reassign', [], IpcErrorKind.userError), + ('claude.team-chat.post', [], IpcErrorKind.userError), + ('claude.agent.fork', [], IpcErrorKind.userError), + // No orchestrator is wired in this test (extension not activated), + // so a fork with a source id fails as unavailable tooling. + ('claude.agent.fork', ['some-id'], IpcErrorKind.toolError), + ]; + + for (final (id, args, kind) in cases) { + test('$id ${args.isEmpty ? '(no args)' : args.join(' ')} → $kind', () async { + final r = await cmd(id).run(args); + expect(r.ok, isFalse, reason: 'a failure must not report ok'); + expect(r.error!.kind, kind); + expect(r.error!.code, isNot(0), reason: 'the CLI must exit non-zero'); + }); + } + }); +}