record D-74: co-registered IPC command schema
Captures the T-120 design decision: the typed argument schema travels with each command (built-in modules and extension contributions both register their own) rather than living in a central static map, which would be blind to extension-contributed commands and fight the plugin model (D-46). The dispatcher accumulates a cmd->schema registry and validates req.args before invoking the handler. The T-104 git validators stay as defense-in-depth because the git client is also reachable directly from the UI. MCP tools/list generation deferred to the T-130 track. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Claude
@@ -1892,3 +1892,37 @@ Source: consultants.md "Security — Findings — [Major]" item 1.', NULL, '2026
|
||||
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-99', 'status', 'backlog', 'done', NULL, '2026-05-19 13:14:41', '2026-05-19 13:14:41', '2026-05-19 13:14:41', NULL, '5ff47ab0fc7fb8cb8e9e039be48ada2a', 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-131', 'status', 'in_progress', 'done', NULL, '2026-05-19 13:14:41', '2026-05-19 13:14:41', '2026-05-19 13:14:41', NULL, 'a57b3fbe4e6815fcbb28691a08130801', 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-119', 'status', 'ready', 'in_progress', NULL, '2026-05-19 14:44:43', '2026-05-19 14:44:43', '2026-05-19 14:44:43', NULL, '1d5048cf9a031c504aa3df570a7b0cae', 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-119', 'status', 'in_progress', 'done', NULL, '2026-05-20 07:00:06', '2026-05-20 07:00:06', '2026-05-20 07:00:06', NULL, '1f2c2bcc722417d8f4c5bfa27faf0c93', 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-120', 'description', 'T-104 covered the spot-fixes (argv-injection rejection, size/count caps on specific commands). What remains is the framework piece: a typed schema per IPC command — branch/remote/path/etc. with regex/charset constraints — applied at DaemonDispatcher dispatch time rather than scattered through individual handlers.
|
||||
|
||||
Re-scope (2026-05-19, post T-99): the wire contract is now real — IpcRequest envelopes flow over the unix socket (per D-70/71/72) and through the C clide client which serialises raw argv under the `_argv` sentinel (per D-72/D-6). The argv parser (lib/src/cli/argv_to_request.dart) is the de-facto schema today; T-120 should formalise it by:
|
||||
|
||||
- Lifting the per-cmd grammar from argv_to_request.dart into a typed registry keyed by cmd, sharing it with both the argv parser and the dispatcher.
|
||||
- Using the same registry to validate inbound IpcRequests at dispatch time (regex/charset constraints on branch/remote/path args).
|
||||
- Exposing the registry to the MCP server (T-130) so the `tools/list` payload is generated rather than hand-rolled.
|
||||
|
||||
Design still open:
|
||||
- Where the schema lives (per-handler? central registry?).
|
||||
- Whether it generates handler boilerplate or wraps existing handlers.
|
||||
|
||||
Source: consultants.md "Security — Findings — [Major]" item 1.', 'T-104 covered the spot-fixes (argv-injection rejection, size/count caps on specific commands). What remains is the framework piece: a typed schema per IPC command — branch/remote/path/etc. with regex/charset constraints — applied at DaemonDispatcher dispatch time rather than scattered through individual handlers.
|
||||
|
||||
---
|
||||
Design resolved 2026-05-20 (see D-74). Co-registration model, NOT a central map:
|
||||
|
||||
- The argv parser (lib/src/cli/argv_to_request.dart) only knows syntactic shape (identifier/flag charset, -- terminator); it has NO per-command argument knowledge. So this is build-fresh, not a lift — each handler currently hand-reads its own keys.
|
||||
- Extend the DaemonDispatcher registration API to carry an optional typed schema per command. The dispatcher accumulates a cmd->schema registry as commands register (built-in register*Commands modules supply theirs; extension CommandContributions carry their own — central static map rejected because it cannot see extension-contributed commands, D-46).
|
||||
- DaemonDispatcher.dispatch validates req.args against schema[cmd] BEFORE invoking the handler (hook at dispatcher.dart line 39, the `return h(req)` call). Violation -> userError (sysexit 64). Commands with no schema dispatch unvalidated (opt-in per command) until migrated.
|
||||
- Constraint vocabulary is hand-rolled (prefer-zero-deps): per-arg required/optional, kind, charset/regex, numeric range. Collapses the _ResizeArgs-style hand-lifts (panel_commands.dart, T-119) into schema.
|
||||
- KEEP existing T-104 checks as defense-in-depth: validateGitRef (lib/src/git/operations.dart:46) and the count/path caps (lib/src/daemon/git_commands.dart:18,23). The git client is callable directly from the Flutter UI, not only via the dispatcher — do not remove them.
|
||||
- OUT OF SCOPE: MCP tools/list generation from the registry — deferred to the T-130 track.
|
||||
|
||||
Acceptance:
|
||||
1. Dispatcher validates req.args against a registered per-command schema before the handler runs; violations return userError.
|
||||
2. At least the security-relevant commands (git ref/remote/path args, panel.resize) carry schemas; argv-injection + caps still enforced (no regression vs T-104).
|
||||
3. validateGitRef + git caps retained as defense-in-depth.
|
||||
4. No third-party validation dep; no lint suppressions.
|
||||
5. Tests cover: schema accept/reject per constraint kind, dispatcher-level rejection, and that an unschema-d command still dispatches.
|
||||
|
||||
Source: consultants.md "Security — Findings — [Major]" item 1. Decision: D-74.', NULL, '2026-05-20 15:54:35', '2026-05-20 15:54:35', '2026-05-20 15:54:35', NULL, '7f972739c82134897869432a3978d6ec', 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-120', 'status', 'backlog', 'in_progress', NULL, '2026-05-20 15:54:38', '2026-05-20 15:54:38', '2026-05-20 15:54:38', NULL, 'fa11190ac1bd9cd8f67e3041a8ca7236', 1) ON CONFLICT(hash) DO NOTHING;
|
||||
|
||||
@@ -2219,3 +2219,24 @@ INSERT INTO tickets (id, type, parent_id, title, description, status, priority,
|
||||
|
||||
Source: consultants.md "Architecture — Findings — [Critical] No IPC socket server exists".', 'done', 'high', NULL, NULL, NULL, '2026-05-17 18:47:18', '2026-05-19 13:14:41', NULL, '2ad1fbf0b1b6f5b9e46d4a31d1ad830c', 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-119', 'task', 'T-99', 'clide panel resize CLI verb (split from T-111)', 'Once T-99 lands an IPC dispatch path, register a ''panel.resize'' command that takes <slot> and <delta-or-absolute> args and calls LayoutArrangement.setSize / setEditorRatio. Keyboard parity already landed in T-111; this completes user/Claude parity per D-6.', 'in_progress', 'low', NULL, NULL, NULL, '2026-05-18 07:43:50', '2026-05-19 14:44:43', NULL, 'ac4c7a1d76836e546dac66e77ed4cb67', 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-119', 'task', 'T-99', 'clide panel resize CLI verb (split from T-111)', 'Once T-99 lands an IPC dispatch path, register a ''panel.resize'' command that takes <slot> and <delta-or-absolute> args and calls LayoutArrangement.setSize / setEditorRatio. Keyboard parity already landed in T-111; this completes user/Claude parity per D-6.', 'done', 'low', NULL, NULL, NULL, '2026-05-18 07:43:50', '2026-05-20 07:00:06', NULL, 'ba93872d695800697af1654a8bfd6e93', 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-120', 'task', 'T-97', 'typed IPC command schema framework (split from T-104)', 'T-104 covered the spot-fixes (argv-injection rejection, size/count caps on specific commands). What remains is the framework piece: a typed schema per IPC command — branch/remote/path/etc. with regex/charset constraints — applied at DaemonDispatcher dispatch time rather than scattered through individual handlers.
|
||||
|
||||
---
|
||||
Design resolved 2026-05-20 (see D-74). Co-registration model, NOT a central map:
|
||||
|
||||
- The argv parser (lib/src/cli/argv_to_request.dart) only knows syntactic shape (identifier/flag charset, -- terminator); it has NO per-command argument knowledge. So this is build-fresh, not a lift — each handler currently hand-reads its own keys.
|
||||
- Extend the DaemonDispatcher registration API to carry an optional typed schema per command. The dispatcher accumulates a cmd->schema registry as commands register (built-in register*Commands modules supply theirs; extension CommandContributions carry their own — central static map rejected because it cannot see extension-contributed commands, D-46).
|
||||
- DaemonDispatcher.dispatch validates req.args against schema[cmd] BEFORE invoking the handler (hook at dispatcher.dart line 39, the `return h(req)` call). Violation -> userError (sysexit 64). Commands with no schema dispatch unvalidated (opt-in per command) until migrated.
|
||||
- Constraint vocabulary is hand-rolled (prefer-zero-deps): per-arg required/optional, kind, charset/regex, numeric range. Collapses the _ResizeArgs-style hand-lifts (panel_commands.dart, T-119) into schema.
|
||||
- KEEP existing T-104 checks as defense-in-depth: validateGitRef (lib/src/git/operations.dart:46) and the count/path caps (lib/src/daemon/git_commands.dart:18,23). The git client is callable directly from the Flutter UI, not only via the dispatcher — do not remove them.
|
||||
- OUT OF SCOPE: MCP tools/list generation from the registry — deferred to the T-130 track.
|
||||
|
||||
Acceptance:
|
||||
1. Dispatcher validates req.args against a registered per-command schema before the handler runs; violations return userError.
|
||||
2. At least the security-relevant commands (git ref/remote/path args, panel.resize) carry schemas; argv-injection + caps still enforced (no regression vs T-104).
|
||||
3. validateGitRef + git caps retained as defense-in-depth.
|
||||
4. No third-party validation dep; no lint suppressions.
|
||||
5. Tests cover: schema accept/reject per constraint kind, dispatcher-level rejection, and that an unschema-d command still dispatches.
|
||||
|
||||
Source: consultants.md "Security — Findings — [Major]" item 1. Decision: D-74.', 'in_progress', 'medium', NULL, NULL, NULL, '2026-05-18 07:53:08', '2026-05-20 15:54:38', NULL, '3c33fccd4b7442444ca598d430fab7f8', 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);
|
||||
|
||||
@@ -115,6 +115,7 @@ You might also want, project-permitting:
|
||||
- [D-71: IPC socket access gated by chmod 0600 on socket + parent](decisions/architecture.md#d-71-ipc-socket-access-gated-by-chmod-0600-on-socket--parent) — _architecture_
|
||||
- [D-72: IPC server is multi-connection with serial dispatch on the main isolate](decisions/architecture.md#d-72-ipc-server-is-multi-connection-with-serial-dispatch-on-the-main-isolate) — _architecture_
|
||||
- [D-73: MCP transport for /ide is SSE over HTTP](decisions/architecture.md#d-73-mcp-transport-for-ide-is-sse-over-http) — _architecture_
|
||||
- [D-74: IPC command schema is co-registered with the handler, validated at dispatch](decisions/architecture.md#d-74-ipc-command-schema-is-co-registered-with-the-handler-validated-at-dispatch) — _architecture_
|
||||
|
||||
## Open questions
|
||||
|
||||
|
||||
@@ -289,4 +289,12 @@ Core, rendering, IPC, kernel, panel manager.
|
||||
- **Cross-reference:** [D-68](#d-68-dual-integration-surface--bash-cli-primary-mcp-secondary), [D-72](#d-72-ipc-server-is-multi-connection-with-serial-dispatch-on-the-main-isolate), `lib/src/ipc/mcp_server.dart` (this transport's implementation lands in T-130).
|
||||
- **Raised by:** 2026-05-19 — T-130 design pass; user picked SSE over HTTP after weighing against WebSocket and stdio.
|
||||
|
||||
### D-74: IPC command schema is co-registered with the handler, validated at dispatch
|
||||
- **Date:** 2026-05-20
|
||||
- **Decision:** Each IPC command may declare a **typed argument schema** (per-arg charset/regex/range constraints, required/optional, kind). The schema is **registered alongside the handler**, not authored in a central file: the `DaemonDispatcher` registration API carries an optional schema per command, and the dispatcher accumulates a `cmd → schema` registry as commands register. `DaemonDispatcher.dispatch` validates `req.args` against the command's schema **before** invoking the handler (`lib/src/daemon/dispatcher.dart`), returning a `userError` (sysexit 64) on violation so no handler sees malformed input. Built-in command modules (`registerPaneCommands`, `registerGitCommands`, …) supply their schemas at registration; extension-contributed commands ([`CommandContribution`](../../lib/extension/src/contribution.dart), Tier 6 / [D-46](tooling.md)) carry their own. The pre-existing T-104 spot-checks — `validateGitRef` in `lib/src/git/operations.dart` and the count/path caps in `lib/src/daemon/git_commands.dart` — stay in place as **defense-in-depth** below the dispatcher, because the git client is also callable directly from the Flutter UI (not only through the dispatcher). MCP `tools/list` generation from this registry is **out of scope** here — deferred to the T-130 track.
|
||||
- **Rationale:** clide's command surface is open, not fixed: every `register*Commands()` module already contributes a *set* of commands, and the extension framework lets plugins contribute more at runtime. A central static schema map (`command_schema.dart` keyed by every known cmd) cannot see extension-contributed commands and would fight the plugin model — it was considered and rejected for exactly that reason. Co-registration keeps the schema next to the code that owns the command's meaning, supports any number of independent registrants (the normal case, not an edge case), and still yields a single validation lookup at dispatch time. Validating at the dispatcher (rather than per-handler) is what makes the constraint a contract instead of a convention: a new command can't forget to validate, and the `_ResizeArgs`-style hand-lifts scattered through handlers (see `panel_commands.dart`, T-119) collapse into the schema. No third-party validation library — the constraint vocabulary is hand-rolled per the prefer-zero-deps guardrail.
|
||||
- **Cost:** A schema must be authored for each command (build-fresh — the argv parser at `lib/src/cli/argv_to_request.dart` only knows syntactic shape, never per-command argument names, so there is nothing to "lift"). Keeping `validateGitRef` + caps as well as the schema is deliberate redundancy on the git path; the alternative (single gate at the dispatcher) would leave the UI→client call path unvalidated. Until every command has a schema, validation is opt-in per command — commands with no registered schema dispatch unvalidated, same as today.
|
||||
- **Cross-reference:** [D-6](#d-6-cli-and-event-surface-contract) (CLI/Claude parity the schema enforces), [D-68](#d-68-dual-integration-surface--bash-cli-primary-mcp-secondary) / [D-72](#d-72-ipc-server-is-multi-connection-with-serial-dispatch-on-the-main-isolate) (the dispatch path being gated), [D-46](tooling.md) (extension model the central-map alternative would have broken), `lib/src/daemon/dispatcher.dart` (the validation hook), `lib/src/git/operations.dart` + `lib/src/daemon/git_commands.dart` (the T-104 checks kept as defense-in-depth).
|
||||
- **Raised by:** 2026-05-20 — T-120 design pass; user rejected the central registry as misaligned with the extensions/plugins model and confirmed co-registration with multiple command-set registrants.
|
||||
|
||||
---
|
||||
|
||||
Reference in New Issue
Block a user