feat(mcp): the agent face — odu mcp server (tools + resources)

[srid]

The agent face from the roadmap: odu mcp serves odu's surface as an MCP stdio server, so a coding agent (Claude Code, Codex, opencode, Gemini CLI) drives CI with structured calls instead of scraping the terminal. This is the one place justci told us the batch shape was the obstacle — its MCP mode auto-ran every recipe because process-compose has no serve-only mode (juspay/justci#22); a runner that owns the DAG as idle state has that separation by construction.

In-band like status/logs/monitor: it dials .ci/odu.sock in the cwd and predetermines no host — which boxes run the lanes stays the coordinator's job (pool lease / hosts.json).

Tools (stateless, dial-per-call)

Tool	What it does
run	Start a background odu run (its own coordinator) and return once the socket is live.
get_nodes	Snapshot the pipeline cell.
tail_log	One node's output — live stream, or the durable per-SHA log when no run is live.
rerun_node	Reset a node + its dependents on the live DAG (the only mutation).
wait_for_settle	Block until the run settles, or — fail-fast — the instant a node goes red.

The design fact that makes request/response enough: an agent wants a snapshot + a blocking "done", not a byte stream. The loop is run → wait_for_settle → read the red node's tail_log → fix → rerun_node.

Resources (the live-push ceiling)

odu://nodes + odu://log/{node} as subscribable resources — resources/subscribe + notifications/resources/updated maps the surface's snapshot-then-delta one-to-one. wait_for_settle is the blocking-pull floor for hosts that don't wake the model on a notification.

Design notes

• Built on the SDK's low-level Server for full subscribe/unsubscribe control + JSON-Schema tool inputs validated by odu's own zod (no SDK schema coupling).
• Tools are stateless (mirror src/cli/introspect.ts); the only state — one live socket attachment driving the resource pushes — is isolated in resources.ts. detach disposes the link rather than aborting (no oRPC cancel-send / socket-close race), and a generation token stops a torn-down stream from rescheduling. (@kolu/surface-mcp was considered and rejected — Lowy+Hickey: it hides no hard volatility and the wait_for_settle "settled" predicate is odu-domain, so this stays consumer-local until a second surface with a different vocabulary proves the concept.)
• apm.yml declares the MCP server (the nix-chrome-devtools-mcp pattern); the odu-mcp skill ships the serve launcher (nix run, ODU_FLAKE-overridable).

Verification

• typecheck + 57 unit tests green (17 new, over the real unix-socket transport: tool dispatch, fail-fast wait_for_settle, resource push).
• nix build .#odu .#odu-runner green; MCP initialize/tools/list/resources/list smoke-tested over stdio in the packaged closure.
• odu-on-odu CI runs on this PR.

🤖 Generated with Claude Code

[srid]

Codex ⇄ Claude debate — ✅ consensus

2 rounds · reviewer effort xhigh · codex (reviewer) ⇄ Claude (author). Per-round findings + dispositions below; each round is committed on the branch.

Round 1

codex — approved: false

The branch is directionally coherent, but I found several correctness and robustness issues in the MCP tool surface. The most serious one can report a non-settled run as passed if the coordinator/socket disappears before a terminal frame is observed. I could not run the typecheck because pnpm is not available on PATH in this environment; git diff --check was clean.

Findings:

• F1 · blocking · open — waitForSettle can return passed: true when the nodes stream ends before a terminal frame, as long as the last observed snapshot has no red nodes. That gives a false green for coordinator crashes, interrupts, or socket-close races while nodes are still pending/running. (src/mcp/tools.ts:260)
• F2 · major · open — startRun waits for the socket poll to finish before observing child exit. A dirty-tree failure, ENOENT for tsx, bad justfile, or stale non-socket path will not be reported until the full 60s poll completes; if the child remains alive but never serves a socket, line 417 then waits forever. (src/mcp/tools.ts:415)
• F3 · major · open — The durable-log fallback passes untrusted MCP input directly into logPathFor; logPathFor interpolates the parsed platform/namepath into a relative path. join(repoRoot, ...) will normalize .., so a crafted node id can escape .ci/<sha7> and read other *.log files accessible to the process. (src/mcp/tools.ts:122)
• F4 · major · open — The file fallback reads the entire durable log synchronously into memory and returns it as one MCP text payload. CI logs can be very large, so this can block the server, consume unbounded memory, and produce unusable MCP responses; the live path is already capped by the in-memory log tail. (src/mcp/tools.ts:124)
• F5 · minor · open — Long-running tool calls ignore the MCP request cancellation signal. If a client cancels wait_for_settle, the handler continues holding a socket and timer until settle or the 10 minute timeout. (src/mcp/server.ts:196)
• F6 · minor · open — resources/subscribe accepts any URI and hands it to ResourcePusher. Unknown resources can keep the pusher retrying/attached forever, and malformed log URIs can throw after being added to the subscribed set. (src/mcp/server.ts:277)
• F7 · nit · open — The test helper creates temporary directories but close only closes the socket listener; it does not remove the temp directory/socket path. Repeated test runs will leak odu-mcp-test-* directories under /tmp. (src/mcp/serveForTest.ts:54)

claude — Addressed all 7 of CODEX's findings on the odu MCP tool surface (src/mcp). All agreed-with — none disputed; one was already partly mitigated (timeout path) but the false-green stream-end path was a genuine bug. Typecheck (tsc --noEmit) clean and the full suite (61 tests, 7 files) green, including 4 new regression tests I added (F1 non-green-when-not-settled, F3 path-traversal rejection, F5 caller-cancel, F6 malformed-uri). Note for CODEX: pnpm isn't on PATH here either, but the repo's local binaries work — node_modules/.bin/tsc and node_modules/.bin/vitest ran fine, so I was able to verify what you couldn't. No JS formatter exists in this repo (just fmt only touches Nix), so no formatting pass was applicable to the TS files I changed.

• F1 fixed — Agreed — real false-green bug. In waitForSettle's stream-end branch (src/mcp/tools.ts ~307-323) passed now requires settled: const settled = last !== undefined && isDone(last); passed: settled && red.failed.length + red.errored.length === 0. A half-observed run (coordinator crash/socket-close while nodes pending/running, no red yet) can no longer report passed:true. Added regression test 'never reports passed when the run does not settle green' (asserts passed:false, settled:false whether the stream ends or the wait times out).
• F2 fixed — Agreed. Replaced the serial 'await waitForSocket then await onExit' with a new awaitStartup() that races socket availability against child exit via an exited promise threaded into the poll loop (src/mcp/tools.ts). defaultWaitForSocket now stops the instant the child dies (one final dial for a detached coordinator, then gives up) instead of polling the full 60s, so a dirty-tree refusal / ENOENT tsx / bad justfile is reported immediately. The 'socket up always wins' tie is preserved (a clean run can fork a detached coordinator and let the launcher exit). The infinite-hang case is closed: on timeout with the child still alive (r.code===null) we SIGTERM it and return startupError() with a clear 'did not serve a socket within the startup window' message rather than awaiting exit forever.
• F3 fixed — Agreed — path traversal via untrusted token. Added durableLogPath(repoRoot, sha7, token) (src/mcp/tools.ts) that rejects empty/unknown platform or namepath, resolves the final path, and requires it to stay under resolve(repoRoot,'.ci',sha7) via a relative() check (rejects '', '..'-prefixed, and absolute-sibling). tail_log's file fallback now uses it and returns source:'missing' for any escaping id. Added regression test 'refuses a node id that escapes the per-SHA log dir' that plants a real .log outside .ci/ and confirms a ..-laden token does not read it.
• F4 fixed — Agreed — unbounded readFileSync. Replaced it with tailFile(path, MAX_LOG_CHARS) (src/mcp/tools.ts) that opens the fd, stats size, and reads only the last MAX_LOG_CHARS (64KB) bytes — matching the cap the live in-memory tail already enforces, so the file fallback can't block the server or blow up the MCP payload.
• F5 fixed — Agreed. Threaded extra.signal from the CallTool handler (src/mcp/server.ts) into a new WaitOptions.signal in waitForSettle (src/mcp/tools.ts): the caller signal aborts the same AbortController as the timeout (closing the dialed socket promptly), with listener cleanup in finally. Added a cancelled field to SettleVerdict and the abort branch distinguishes caller-cancel (cancelled:true, timed_out:false) from the timeout (timed_out:true). Added regression test 'returns cancelled when the caller aborts the wait'.
• F6 fixed — Agreed. resources/subscribe (src/mcp/server.ts) now rejects any URI that isn't NODES_URI or a well-formed odu://log/ before handing it to the pusher, so an unknown URI can't keep the pusher retrying/attached. Also hardened parseLogUri (src/mcp/resources.ts) to return null for an empty node or malformed percent-encoding (decodeURIComponent wrapped in try/catch) so a bad log URI can't throw downstream, and exported isValidLogUri. Added unit tests for empty/malformed URI rejection.
• F7 fixed — Agreed. serveForTest.ts close() now closes the listener AND rmSync(dir, {recursive, force}) the mkdtemp directory, so repeated runs don't leak odu-mcp-test-* under tmpdir. Verified: after the full suite, ls -d /tmp/odu-mcp-test-* count is 0.

commit: 7d7d6d455c92b121affa27f5b434b955ef50eb43

Round 2

codex — approved: true

All seven previously raised findings have been addressed in the current tree. I verified the relevant MCP changes directly, confirmed node_modules/.bin/tsc --noEmit --pretty false passes, and git diff --check a9cdbc1e53579326483f2e38968255a50ce83b0b is clean. I did not rescan for unrelated pre-existing issues, per the convergence instruction.

Findings:

• F1 · blocking · resolved — Previously, waitForSettle could report passed: true after the stream ended without a terminal snapshot. The current code now computes settled separately and only allows passed when the last snapshot is terminal and has no red nodes. (src/mcp/tools.ts:325)
• F2 · major · resolved — Previously, startRun waited through the full socket poll before observing child exit and could then hang forever on a still-alive child that never served a socket. The current startup path threads an exit promise into socket polling, stops after child exit, and kills a still-alive child on startup timeout. (src/mcp/tools.ts:423)
• F3 · major · resolved — Previously, the durable-log fallback used untrusted node input directly in a path. The new durableLogPath validates the fan id and requires the resolved path to remain under .ci/<sha7>. Invalid or escaping ids now return source: "missing". (src/mcp/tools.ts:99)
• F4 · major · resolved — Previously, the durable-log fallback read entire log files synchronously. The current tailFile reads only the last MAX_LOG_CHARS bytes, matching the live tail cap and bounding the payload. (src/mcp/tools.ts:119)
• F5 · minor · resolved — Previously, wait_for_settle ignored MCP request cancellation. The server now passes extra.signal into waitForSettle, and waitForSettle aborts its controller, cleans up the listener, closes the socket, and returns a cancelled verdict. (src/mcp/server.ts:201)
• F6 · minor · resolved — Previously, resources/subscribe accepted arbitrary or malformed URIs. It now accepts only odu://nodes or a valid odu://log/<node>, while parseLogUri rejects empty or malformed percent-encoding without throwing. (src/mcp/server.ts:285)
• F7 · nit · resolved — Previously, the test socket helper leaked temporary directories. Its close function now closes the listener and removes the mkdtemp directory with rmSync(..., { recursive: true, force: true }). (src/mcp/serveForTest.ts:64)

codex on the rebuttal: CLAUDE did not dispute any finding. I verified each claimed fix in the current files and accept all seven resolutions.

claude — (no author turn this round)

(no author turn — the debate ended this round)

[srid]

⚖️ Lowy ⇄ Hickey lens debate — ✅ consensus

1 round · lowy (volatility-based decomposition) + hickey (structural simplicity), reviewed independently then debated to consensus. Reviewed a pinned snapshot read-only (--no-apply); fixes handed to the apply pass.

Agreed fixes

• lowy-1 / hickey-1 — wait_for_settle's redNodes re-derived redness with literal status === "failed"/"errored" instead of the declared receptacle STATUS_META[status].isRed. STATUS_META's own docstring says adding a NodeStatus is a single edit enforced across consumers; rowOf (same file) and run.ts already plug in — only redNodes reached past it, so a future red status would silently desync the agent verdict from the TUI verdict. (src/mcp/tools.ts)
• hickey-2 — isDone re-implemented summarize().done; the settle predicate should read the same receptacle the TUI uses, not a second pending/running scan. (src/mcp/tools.ts)
• lowy-2 — rowOf (get_nodes) duplicated statusCommand's odu status -o json projection and the two disagreed on the id key (status -o json keyed the id as name; get_nodes as id). One machine-readable node-snapshot axis, wired twice. (src/mcp/tools.ts, src/cli/introspect.ts)

Applied by /be-review

Re-validated against the post-codex tree, implemented, committed individually:

Finding	Disposition	Commit
lowy-1/hickey-1 — redNodes via STATUS_META[...].isRed (bucket by concrete status, gate on isRed — mirrors run.ts)	applied	b79f039
hickey-2 — settled predicate via summarize(state).done, isDone removed (codex's F1 false-green fix preserved)	applied	24eaf6f
lowy-2 — shared nodeRow() projection in render.ts; status -o json now emits both id and name (additive, back-compat — no consumer of its field names found)	applied	24fe898

[srid]

👮 Code-police

Three read-only passes over the diff — rule checklist · fact-check · elegance. The diff is high quality. Every load-bearing comment/claim fact-checked true against the code: ODU_SELF resolution + tsx dev fallback, the detach-without-abort race rationale, the tool descriptions vs behavior, the durable-log fallback, and "the first (snapshot) frame is the whole buffer." No logic errors, off-by-ones, or always-true conditions. Findings were all low-severity:

Finding	Disposition	Commit
defaultWaitForSocket's comma-operator probe (d?.close(), d !== null) was obscure (comment-the-non-obvious)	applied — factored an explicit probe-and-release helper	2f966d9
version()'s best-effort catch {} lacked the why-comment the rule asks for	applied	2997b5b
offline tail_log with a bare token degraded to missing opaquely	superseded — codex's F3 added durableLogPath validation (rejects escaping/invalid ids)	—
optional firstFrame(stream) / verdict({…}) factory extractions	skipped — moot after codex restructured waitForSettle	—

The security/robustness class (path traversal in the durable-log fallback, the wait_for_settle false-green, unbounded readFileSync, MCP request-cancellation, subscribe-URI validation) was independently caught and fixed by the codex track — see that comment.