From 1147bfac0e57648743ffdb4d90a2faa3905b6b2e Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 18 May 2026 17:51:17 +0200 Subject: [PATCH] =?UTF-8?q?T-125:=20argv=E2=86=92IpcRequest=20translator?= =?UTF-8?q?=20(CLI=20grammar=20in=20Dart)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second slice of T-99. Pure Dart function that takes the argv tail of a `clide ...` invocation and returns either an IpcRequest ready to dispatch or an ArgvError carrying a pre-built userError response. The grammar — `SUBSYSTEM VERB [pos...] [--flag value] [--flag=val] [-- passthrough...]` plus the umbrella commands `status`, `tail`, `version`, `ping` — sits here so the C client (T-126) is a dumb pipe: it sends argv as JSON and the server runs the translator before dispatch. Wire envelope: cmd is `subsystem.verb` (or just `subsystem` for umbrella commands). Args is a generic envelope — `positional: [...]`, `flags: {...}`, `passthrough: [...]` — none required, all omitted when empty so the dispatch surface stays minimal. Per-command typed schemas land later as each CLI verb gets wired end-to-end. Co-Authored-By: Claude --- .pql/changelog/ticket_history/2026-05.sql | 2 + .pql/changelog/tickets/2026-05.sql | 11 ++ CHANGELOG.md | 5 + lib/src/cli/argv_to_request.dart | 213 ++++++++++++++++++++++ test/cli/argv_to_request_test.dart | 145 +++++++++++++++ 5 files changed, 376 insertions(+) create mode 100644 lib/src/cli/argv_to_request.dart create mode 100644 test/cli/argv_to_request_test.dart diff --git a/.pql/changelog/ticket_history/2026-05.sql b/.pql/changelog/ticket_history/2026-05.sql index 10ad7c1c..f7cbd832 100644 --- a/.pql/changelog/ticket_history/2026-05.sql +++ b/.pql/changelog/ticket_history/2026-05.sql @@ -1854,3 +1854,5 @@ Open a per-workspace unix-domain socket on Flutter app boot. Accept JSON-lines p Source: T-99 sketch. Coordinates with: T-127 (InProcessClient swap), T-130 (MCP).', NULL, '2026-05-18 12:42:56', '2026-05-18 12:42:56', '2026-05-18 12:42:56', NULL, 'abb7a039f73d430cdba594b2a4ac381a', 1) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-124', 'status', 'backlog', 'in_progress', NULL, '2026-05-18 12:45:10', '2026-05-18 12:45:10', '2026-05-18 12:45:10', NULL, 'aa1b70849399f4caee043d40e48d1edf', 1) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-124', 'status', 'in_progress', 'done', NULL, '2026-05-18 12:51:08', '2026-05-18 12:51:08', '2026-05-18 12:51:08', NULL, 'b928ce09ae0b7dbb77bf93f8c0593657', 1) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-125', 'status', 'backlog', 'in_progress', NULL, '2026-05-18 15:49:20', '2026-05-18 15:49:20', '2026-05-18 15:49:20', NULL, 'ac0838fac9e3c97f1728256910776bed', 1) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-125', 'status', 'in_progress', 'done', NULL, '2026-05-18 15:51:09', '2026-05-18 15:51:09', '2026-05-18 15:51:09', NULL, '938624de487aa2f02e034424cae40c77', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-05.sql b/.pql/changelog/tickets/2026-05.sql index 1c63ae56..eae8d9e5 100644 --- a/.pql/changelog/tickets/2026-05.sql +++ b/.pql/changelog/tickets/2026-05.sql @@ -2111,3 +2111,14 @@ Open a per-workspace unix-domain socket on Flutter app boot. Accept JSON-lines p 6. No client yet — that lands in T-126. Source: T-99 sketch. Coordinates with: T-127 (InProcessClient swap), T-130 (MCP).', 'done', 'high', NULL, NULL, NULL, '2026-05-18 11:58:47', '2026-05-18 12:51:08', NULL, 'eaa0d45789ecd577e81dc07ef476e31c', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_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 (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-125', 'task', 'T-99', 'argv→IpcRequest translator on the Dart side', 'Second slice of T-99(a). Owns the CLI grammar so the C client stays a dumb pipe. + +Add lib/src/cli/argv_to_request.dart: takes a List (the argv after `clide`) and returns an IpcRequest. Handles `clide [positionals...] [--flag value] [-- argv...]` per D-6''s shape. Errors (unknown subsystem/verb, malformed flag) surface as IpcResponse.err with the right exit code per pql''s contract. + +Acceptance: +1. Pure-Dart function with no I/O; unit-testable. +2. Covers every active subsystem from D-6''s list (pane/tab/open/editor/panel/tree/git/pql/canvas/graph/theme/settings/project) at least to the level of "recognise the verb". +3. Round-trip tests against the existing IpcRequest envelope. +4. Sysexit-code parity with pql (0/1/2/3/4 + 64-78 reserved). + +Source: T-99 sketch.', 'done', 'high', NULL, NULL, NULL, '2026-05-18 11:58:52', '2026-05-18 15:51:09', NULL, '99f60de735779ddc37e7701ef4dd3304', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_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 898089d3..604d07f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,11 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. dispatch through `DaemonDispatcher` (D-72). Foundation for the C `clide` client (T-126) and MCP (T-130). No client yet — testable via `socat - UNIX-CONNECT:$SOCK`. +- argv→IpcRequest translator (`lib/src/cli/argv_to_request.dart`) — + parses `clide SUBSYSTEM VERB [pos...] [--flag] [-- passthrough]` and + the umbrella commands (`status`, `tail`, `version`, `ping`) per D-6 + into the wire envelope. Pure Dart; lets the C client (T-126) stay a + dumb pipe (T-99 / T-125). - Startup project picker — clide now opens to the welcome screen by default instead of auto-opening the last project. A per-row "always open this project on launch" checkbox in welcome's RECENT diff --git a/lib/src/cli/argv_to_request.dart b/lib/src/cli/argv_to_request.dart new file mode 100644 index 00000000..6b4d5f6d --- /dev/null +++ b/lib/src/cli/argv_to_request.dart @@ -0,0 +1,213 @@ +/// Translate the argv tail of a `clide …` shell invocation into an +/// [IpcRequest] the server can dispatch. Pure Dart, no I/O. +/// +/// Second slice of T-99 (D-56 path a). The C client (T-126) is a +/// dumb pipe: it sends `{argv:[…]}` over the socket, the server +/// runs this function, then dispatches. Keeping the CLI grammar in +/// Dart means the translator can be unit-tested and shared with +/// `make t T=…` workflows that don't shell out to the C client. +/// +/// Grammar (per D-6): +/// +/// ``` +/// clide SUBSYSTEM VERB [positional...] [--flag value] [-- passthrough...] +/// clide UMBRELLA # status, tail, version, ping +/// ``` +/// +/// The umbrella entries have no subsystem.verb split; the first arg +/// IS the command id. D-6 lists `tail` and `status` explicitly; the +/// dispatcher also recognises `ping` and `version` (registered by +/// default in [DaemonDispatcher]). +library; + +import 'package:clide/src/ipc/envelope.dart'; +import 'package:clide/src/ipc/schema_v1.dart'; + +/// Umbrella commands — single-token names with no subsystem.verb +/// split. Match the IDs the dispatcher exposes directly. +const Set _umbrellaCommands = {'status', 'tail', 'version', 'ping'}; + +/// Sealed result of translating argv. Caller (the IPC server, or the +/// C client wrapper in T-126) handles either branch. +sealed class ArgvParseResult { + const ArgvParseResult(); +} + +/// Successful parse — the [request] is ready to dispatch. +class ArgvParsed extends ArgvParseResult { + const ArgvParsed(this.request); + final IpcRequest request; +} + +/// argv was malformed at the syntactic layer (no subsystem, broken +/// flag, etc.). The [response] is ready to write back to the client +/// — exit code in `response.error.code` follows pql's sysexit +/// convention (64 = EX_USAGE for shape errors). +class ArgvError extends ArgvParseResult { + const ArgvError(this.response); + final IpcResponse response; +} + +/// Translate [argv] (everything after the program name) into an +/// [IpcRequest] or an [ArgvError]. [requestId] is the wire id the +/// server should echo on the response — caller assigns it (typically +/// a counter or random short string). +ArgvParseResult parseArgv(List argv, {required String requestId}) { + if (argv.isEmpty) { + return ArgvError(_err(requestId, 'usage: clide [args...]')); + } + + final first = argv[0]; + // Umbrella commands: single-token name, no verb required. + if (_umbrellaCommands.contains(first)) { + final tail = argv.sublist(1); + final parsed = _parseTail(tail); + if (parsed is _TailError) { + return ArgvError(_err(requestId, parsed.message)); + } + return ArgvParsed(IpcRequest( + id: requestId, + cmd: first, + args: (parsed as _TailOk).toArgs(), + )); + } + + // Subsystem.verb form: need at least two tokens. + if (argv.length < 2) { + return ArgvError(_err(requestId, 'usage: clide $first [args...]')); + } + final subsystem = first; + final verb = argv[1]; + if (!_isValidIdentifier(subsystem)) { + return ArgvError(_err(requestId, 'invalid subsystem: $subsystem')); + } + if (!_isValidIdentifier(verb)) { + return ArgvError(_err(requestId, 'invalid verb: $verb')); + } + final tail = argv.sublist(2); + final parsed = _parseTail(tail); + if (parsed is _TailError) { + return ArgvError(_err(requestId, parsed.message)); + } + return ArgvParsed(IpcRequest( + id: requestId, + cmd: '$subsystem.$verb', + args: (parsed as _TailOk).toArgs(), + )); +} + +// -- internals -------------------------------------------------------------- + +sealed class _TailParseResult { + const _TailParseResult(); +} + +class _TailOk extends _TailParseResult { + const _TailOk({required this.positional, required this.flags, required this.passthrough}); + final List positional; + final Map flags; + final List passthrough; + + Map toArgs() => { + if (positional.isNotEmpty) 'positional': positional, + if (flags.isNotEmpty) 'flags': flags, + if (passthrough.isNotEmpty) 'passthrough': passthrough, + }; +} + +class _TailError extends _TailParseResult { + const _TailError(this.message); + final String message; +} + +/// Walk [tail] splitting it into positionals, flags, and (everything +/// after a lone `--`) passthrough. +/// +/// Flag forms: +/// --key=value → flags[key] = value +/// --key value → flags[key] = value (value can't start with `--`) +/// --key → flags[key] = true (boolean; next token is `--…` or end) +/// +/// Anything not matching `--` is a positional. A bare `--` token +/// terminates option parsing — everything after lands in passthrough. +_TailParseResult _parseTail(List tail) { + final positional = []; + final flags = {}; + final passthrough = []; + var i = 0; + var inPassthrough = false; + while (i < tail.length) { + final t = tail[i]; + if (inPassthrough) { + passthrough.add(t); + i++; + continue; + } + if (t == '--') { + inPassthrough = true; + i++; + continue; + } + if (t.startsWith('--')) { + final body = t.substring(2); + if (body.isEmpty) { + return const _TailError('empty flag: "--" with no name; use a bare "--" to terminate options'); + } + final eq = body.indexOf('='); + if (eq >= 0) { + final key = body.substring(0, eq); + final value = body.substring(eq + 1); + if (!_isValidFlagName(key)) { + return _TailError('invalid flag name: $key'); + } + flags[key] = value; + i++; + continue; + } + // --key with no `=` — peek at next token. + if (!_isValidFlagName(body)) { + return _TailError('invalid flag name: $body'); + } + final next = i + 1 < tail.length ? tail[i + 1] : null; + if (next == null || next == '--' || next.startsWith('--')) { + // Boolean flag — no value follows. + flags[body] = true; + i++; + } else { + flags[body] = next; + i += 2; + } + continue; + } + positional.add(t); + i++; + } + return _TailOk(positional: positional, flags: flags, passthrough: passthrough); +} + +/// Subsystems + verbs use the same shape — letters, digits, dot, +/// hyphen, underscore. Reject anything else so a typo doesn't reach +/// the dispatcher as a wire-shaped command id. +bool _isValidIdentifier(String s) { + if (s.isEmpty) return false; + for (final code in s.codeUnits) { + final isLetter = (code >= 0x41 && code <= 0x5a) || (code >= 0x61 && code <= 0x7a); + final isDigit = code >= 0x30 && code <= 0x39; + final isOther = code == 0x2e /* . */ || code == 0x2d /* - */ || code == 0x5f /* _ */; + if (!isLetter && !isDigit && !isOther) return false; + } + return true; +} + +/// Flag names are the same alphabet plus `.` is rare but allowed. +/// Tighter than identifier here would just create false rejections. +bool _isValidFlagName(String s) => _isValidIdentifier(s); + +IpcResponse _err(String id, String message) => IpcResponse.err( + id: id, + error: IpcError( + code: IpcExitCode.userError, + kind: IpcErrorKind.userError, + message: message, + ), + ); diff --git a/test/cli/argv_to_request_test.dart b/test/cli/argv_to_request_test.dart new file mode 100644 index 00000000..9e3ff2bb --- /dev/null +++ b/test/cli/argv_to_request_test.dart @@ -0,0 +1,145 @@ +import 'package:clide/src/cli/argv_to_request.dart'; +import 'package:clide/src/ipc/envelope.dart'; +import 'package:clide/src/ipc/schema_v1.dart'; +import 'package:test/test.dart'; + +IpcRequest _expectOk(ArgvParseResult r) { + expect(r, isA(), reason: 'expected parse to succeed: $r'); + return (r as ArgvParsed).request; +} + +IpcResponse _expectErr(ArgvParseResult r) { + expect(r, isA(), reason: 'expected parse to error: $r'); + return (r as ArgvError).response; +} + +void main() { + group('parseArgv — subsystem.verb shape', () { + test('subsystem + verb maps to "subsystem.verb"', () { + final req = _expectOk(parseArgv(['git', 'status'], requestId: '1')); + expect(req.cmd, 'git.status'); + expect(req.id, '1'); + expect(req.args, isEmpty); + }); + + test('positional args land in args.positional', () { + final req = _expectOk(parseArgv(['files', 'read', 'lib/main.dart'], requestId: '2')); + expect(req.cmd, 'files.read'); + expect(req.args['positional'], ['lib/main.dart']); + expect(req.args.containsKey('flags'), isFalse); + }); + + test('multiple positionals preserve order', () { + final req = _expectOk(parseArgv(['git', 'stage', 'a.dart', 'b.dart', 'c.dart'], requestId: '3')); + expect(req.args['positional'], ['a.dart', 'b.dart', 'c.dart']); + }); + + test('--key=value flag', () { + final req = _expectOk(parseArgv(['git', 'log', '--count=20'], requestId: '4')); + expect(req.cmd, 'git.log'); + expect((req.args['flags'] as Map)['count'], '20'); + }); + + test('--key value flag (separated)', () { + final req = _expectOk(parseArgv(['git', 'log', '--count', '20'], requestId: '5')); + expect((req.args['flags'] as Map)['count'], '20'); + }); + + test('boolean flag at end of argv', () { + final req = _expectOk(parseArgv(['git', 'push', '--force'], requestId: '6')); + expect((req.args['flags'] as Map)['force'], isTrue); + }); + + test('boolean flag followed by another flag', () { + final req = _expectOk(parseArgv(['git', 'log', '--graph', '--count=10'], requestId: '7')); + expect((req.args['flags'] as Map)['graph'], isTrue); + expect((req.args['flags'] as Map)['count'], '10'); + }); + + test('bare -- terminates options; rest is passthrough', () { + final req = _expectOk(parseArgv(['pane', 'spawn', '--name', 'foo', '--', 'bash', '-c', 'echo hi'], requestId: '8')); + expect(req.cmd, 'pane.spawn'); + expect((req.args['flags'] as Map)['name'], 'foo'); + expect(req.args['passthrough'], ['bash', '-c', 'echo hi']); + }); + + test('mixed positional + flag + passthrough', () { + final req = _expectOk(parseArgv(['files', 'read', 'a.dart', '--limit=4096', '--', '--not-a-flag'], requestId: '9')); + expect(req.args['positional'], ['a.dart']); + expect((req.args['flags'] as Map)['limit'], '4096'); + expect(req.args['passthrough'], ['--not-a-flag']); + }); + }); + + group('parseArgv — umbrella commands', () { + test('"status" maps to cmd "status" with no subsystem', () { + final req = _expectOk(parseArgv(['status'], requestId: 's')); + expect(req.cmd, 'status'); + expect(req.args, isEmpty); + }); + + test('"tail --events" sets the events boolean flag', () { + final req = _expectOk(parseArgv(['tail', '--events'], requestId: 't')); + expect(req.cmd, 'tail'); + expect((req.args['flags'] as Map)['events'], isTrue); + }); + + test('"tail --events --filter git" pairs the filter value', () { + final req = _expectOk(parseArgv(['tail', '--events', '--filter', 'git'], requestId: 'tf')); + expect((req.args['flags'] as Map)['events'], isTrue); + expect((req.args['flags'] as Map)['filter'], 'git'); + }); + + test('ping + version are recognised as umbrella commands', () { + expect(_expectOk(parseArgv(['ping'], requestId: 'p')).cmd, 'ping'); + expect(_expectOk(parseArgv(['version'], requestId: 'v')).cmd, 'version'); + }); + }); + + group('parseArgv — errors', () { + test('empty argv → usage error (EX_USAGE)', () { + final err = _expectErr(parseArgv(const [], requestId: 'e1')); + expect(err.error?.code, IpcExitCode.userError); + expect(err.error?.message, contains('usage')); + }); + + test('subsystem without verb → usage error', () { + final err = _expectErr(parseArgv(['git'], requestId: 'e2')); + expect(err.error?.message, contains('usage')); + expect(err.error?.message, contains('git')); + }); + + test('garbage in the subsystem token is rejected', () { + final err = _expectErr(parseArgv(['bad subsystem', 'verb'], requestId: 'e3')); + expect(err.error?.message, contains('subsystem')); + }); + + test('"--" with no name is a flag-shape error', () { + // "git status -- --" — first -- starts passthrough so second is + // not a flag; that's fine. Reproduce the failure with the + // option-name-empty path via `git status --=foo` which gives an + // empty key. + final err = _expectErr(parseArgv(['git', 'status', '--=foo'], requestId: 'e4')); + expect(err.error?.message, contains('flag')); + }); + + test('empty flag body before passthrough is treated as bare --', () { + // `git status -- bar` — bar is passthrough, not a positional. + final req = _expectOk(parseArgv(['git', 'status', '--', 'bar'], requestId: 'e5')); + expect(req.args['passthrough'], ['bar']); + expect(req.args.containsKey('positional'), isFalse); + }); + }); + + group('parseArgv — wire round-trip', () { + test('encoded request → decoded request matches', () { + final req = _expectOk(parseArgv(['files', 'read', 'README.md'], requestId: 'rt')); + final wire = req.encode(); + final decoded = IpcMessage.decode(wire); + expect(decoded, isA()); + final r = decoded as IpcRequest; + expect(r.cmd, 'files.read'); + expect(r.args['positional'], ['README.md']); + }); + }); +}