return error envelopes from failed claude commands (T-391)
Sixteen claude.* handlers reported ok with an `error` field buried in the payload — `clide claude.agent.set-permission-mode bogus` exited 0, so scripts could not detect failure, drifting from the D-6 exit-code contract every other subsystem honors. Missing/invalid args are now userError, missing sessions notFound, a missing orchestrator toolError, and a failed task reassign no longer reports ok:false as a success. No UI consumer read the old payloads. Table-driven test walks every failure path asserting non-zero codes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Claude Fable 5
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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: <taskId> <sessionId>'});
|
||||
if (args.length < 2) return _userErr('usage: <taskId> <sessionId>');
|
||||
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] <text>'});
|
||||
if (args.isEmpty) return _userErr('usage: [@name] <text>');
|
||||
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 <sourceSessionId> [<cwd>]'});
|
||||
return _userErr('usage: claude.agent.fork <sourceSessionId> [<cwd>]');
|
||||
}
|
||||
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}';
|
||||
|
||||
@@ -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<CommandContribution>().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>, 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');
|
||||
});
|
||||
}
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user