refactor: share snapshot capture persistence

[ThomasK33]

Fixes #60.

Summary

• Extract snapshot result construction and artifact persistence into src/snapshot/capture.ts.
• Route both live host snapshot RPCs and offline replay snapshots through the shared capture path.
• Add shared snapshot capture coverage for result validation, artifact equality, metadata, no-write failure cases, and defensive consistency guards.
• Update snapshot command tests to keep command-surface coverage while leaving artifact details to the shared module tests.
• Document snapshot domain terms in CONTEXT.md.

User-facing / automation-facing behavior

No intended public JSON, RPC, filename, artifact, or manifest shape changes. Snapshot artifacts still contain exactly the emitted snapshot result JSON with two-space formatting and a trailing newline.

Validation

• mise run ci
• npx vitest run test/unit/snapshot/capture.test.ts test/unit/commands/snapshot.test.ts
• npx vitest run --maxWorkers=1 test/integration/host-renderer-rpc.test.ts
• npm run typecheck
• npm run lint
• npm run format:check
• npm run build
• npm run test
• npm run smoke:install -- --skip-build
• git diff --check

Dogfooding

Used an isolated AGENT_TTY_HOME to run both live-host and offline-replay snapshot flows:

• live: create -> wait for rendered output -> structured snapshot with scrollback/cells -> text snapshot with scrollback -> screenshot -> destroy -> WebM export
• offline: create completed command -> wait for exit -> structured snapshot with scrollback/cells -> text snapshot with scrollback -> screenshot -> WebM export

Verified each emitted snapshot .result exactly matched its persisted snapshot artifact JSON.

Dogfood proof root from the implementation workspace:

/tmp/agent-tty-snapshot-dogfood-vw6fkl

Screenshot proof artifacts:

/tmp/agent-tty-snapshot-dogfood-vw6fkl/home/sessions/01KQCMC6HQHN3WMN6S411MCGDF/artifacts/screenshot-1-reference-dark.png
/tmp/agent-tty-snapshot-dogfood-vw6fkl/home/sessions/01KQCMCGR4HR7AYP7S7XJW7WNA/artifacts/screenshot-2-reference-dark.png

WebM proof artifacts:

/tmp/agent-tty-snapshot-dogfood-vw6fkl/home/sessions/01KQCMC6HQHN3WMN6S411MCGDF/artifacts/recording-2-webm.webm
/tmp/agent-tty-snapshot-dogfood-vw6fkl/home/sessions/01KQCMCGR4HR7AYP7S7XJW7WNA/artifacts/recording-2-webm.webm

Design-doc deviations

None. This keeps the existing CLI/RPC/artifact contracts while consolidating duplicated snapshot behavior.

---

📋 Implementation Plan

Plan: Share snapshot capture across live and offline paths

Goal

Implement GitHub issue #60: live host snapshot RPCs and CLI offline replay snapshots should share one snapshot-specific implementation for deriving public SnapshotResult payloads and persisting matching snapshot artifacts, while preserving the existing CLI/RPC JSON contract and artifact behavior.

Evidence gathered

• Issue Share snapshot capture and artifact persistence across live and offline replay #60 requests a shared snapshot capture/result/persistence implementation for live host snapshot RPCs and offline replay snapshots.
• Current offline snapshot behavior lives in src/cli/commands/snapshot.ts via local createSnapshotResult() and persistSnapshotArtifact() helpers.
• Current live RPC snapshot behavior duplicates equivalent result construction and artifact persistence inline in src/host/hostMain.ts.
• SnapshotResultSchema in src/protocol/schemas.ts remains the public validation contract for structured/text snapshot results.
• RendererBackend exposes a non-empty rendererBackend identifier used for artifact metadata.
• src/storage/artifactPaths.ts owns snapshotFilename(seq, format) and artifact path safety.
• Existing integration coverage in test/integration/host-renderer-rpc.test.ts verifies returned RPC snapshot results, persisted artifact JSON equality, manifest metadata, scrollback metadata, and defaults.
• Existing command coverage in test/unit/commands/snapshot.test.ts currently duplicates low-level offline replay artifact expectations that should move to the shared module where practical.

Resolved design decisions

1. Shared module boundary

   • The shared module starts after renderer capture.
   • Callers remain responsible for renderer setup and backend.snapshot({ includeScrollback, includeCells }).
   • The shared module owns only deriving a SnapshotResult, validating it, writing the JSON artifact, and appending the artifact manifest entry.

2. Module location

   • Add src/snapshot/capture.ts.
   • Do not place shared behavior under src/cli/ or src/host/ because both are callers.

3. Layered API

   • Export a small layered API:

     createSnapshotResult(snapshot, format): SnapshotResult;
     persistSnapshotArtifact({ sessionDir, format, snapshot, result, rendererBackend }): Promise<void>;
     captureSnapshotResult({ sessionDir, format, snapshot, rendererBackend, expectedSessionId? }): Promise<SnapshotResult>;

   • Production callers should use captureSnapshotResult(...).
   • Tests may target the lower-level pure and persistence helpers to avoid duplicated expectations in CLI/host tests.

4. Renderer backend metadata invariant

   • rendererBackend is required by the shared persistence/capture API.
   • Assert it is a non-empty string.
   • This aligns with the renderer interface and preserves current real artifact metadata behavior.

5. Validation order

   • createSnapshotResult(...) must validate through SnapshotResultSchema before any artifact persistence happens.
   • If result construction is invalid, no snapshot artifact or manifest entry should be written.

6. Session identity guard

   • captureSnapshotResult(...) accepts optional expectedSessionId.
   • If provided, assert snapshot.sessionId === expectedSessionId before result/persistence work.
   • Live host must pass its known sessionId.
   • Offline replay must pass manifest.sessionId; replay setup already asserts derived session id and manifest id alignment, and passing it keeps the guard symmetric across live/offline paths.

7. Domain language captured during grilling

   • CONTEXT.md now distinguishes Semantic Snapshot, Snapshot Result, Snapshot Artifact, and Snapshot Capture.
   • Snapshot Capture means deriving/recording a public result from a semantic snapshot; it does not mean asking the renderer for state.

Implementation steps

1. Add shared snapshot capture module

Create src/snapshot/capture.ts.

Implementation details:

• Import types from src/protocol/messages.ts and src/renderer/types.ts.
• Import SnapshotResultSchema from src/protocol/schemas.ts.
• Import ERROR_CODES and makeCliError from src/protocol/errors.ts so shared validation failures preserve current protocol-error semantics.
• Import artifact helpers:
  • appendArtifact, createArtifactEntry from src/storage/artifactManifest.ts
  • artifactPath, ensureArtifactsDir, snapshotFilename from src/storage/artifactPaths.ts
  • writeTextFileAtomic from src/storage/manifests.ts
• Use invariant from src/util/assert.ts for defensive checks.

createSnapshotResult(snapshot, format) should preserve current behavior exactly:

• For format === 'structured', return { format: 'structured', ...snapshot } after schema validation.
• For format === 'text', join (snapshot.scrollbackLines ?? []) followed by snapshot.visibleLines, mapping each line to .text and joining with \n.
• Validate with SnapshotResultSchema.safeParse(...) before returning.
• Preserve current CLI failure semantics by throwing makeCliError(ERROR_CODES.PROTOCOL_ERROR, ...) with schema issues when validation fails, instead of replacing protocol errors with a generic invariant failure.

persistSnapshotArtifact(...) should preserve current behavior exactly:

• Assert rendererBackend.length > 0.
• Assert exported-helper consistency before writing: result.format === format and core result fields match snapshot (sessionId, capturedAtSeq, cols, rows, cursorRow, cursorCol). This prevents future callers/tests from writing artifact JSON for one result while deriving metadata from another snapshot.
• Ensure artifacts directory.
• Use snapshotFilename(snapshot.capturedAtSeq, format).
• Write ${JSON.stringify(result, null, 2)}\n to the artifact path.
• Append createArtifactEntry({ kind: 'snapshot', filename, sessionId: snapshot.sessionId, capturedAtSeq: snapshot.capturedAtSeq, metadata: ... }).
• Metadata must preserve existing fields:
  • format
  • rendererBackend
  • cols
  • rows
  • cursorRow
  • cursorCol
  • optional scrollbackLineCount only when snapshot.scrollbackLines !== undefined

captureSnapshotResult(...) should:

• Assert expectedSessionId when provided.
• Call createSnapshotResult(...).
• Call persistSnapshotArtifact(...) with the validated result.
• Return the validated result.

2. Refactor CLI offline replay caller

Update src/cli/commands/snapshot.ts:

• Remove local createSnapshotResult, persistSnapshotArtifact, and now-unused storage/artifact imports.
• Keep command input normalization and live/offline routing unchanged.
• In runOfflineSnapshot(...), destructure manifest from withOfflineReplayRenderer(...); after backend.snapshot({ includeScrollback, includeCells }), call:

  return await captureSnapshotResult({
    sessionDir: sessionDirectory,
    format,
    snapshot,
    rendererBackend: backend.rendererBackend,
    expectedSessionId: manifest.sessionId,
  });

• Preserve runRpcSnapshot(...) response parsing because RPC responses are still external protocol data.
• Preserve human output formatting in formatSnapshotLines(...).

3. Refactor live host snapshot RPC caller

Update src/host/hostMain.ts:

• Remove inline snapshot text construction, result object construction, artifact write, and manifest append from the snapshot RPC handler.
• Keep parameter normalization, renderer resolution, replay input loading, backend acquisition, and backend.snapshot({ includeScrollback, includeCells }) unchanged.
• Replace duplicated result/persistence block with:

  return await captureSnapshotResult({
    sessionDir: sessDir,
    format,
    snapshot,
    rendererBackend: backend.rendererBackend,
    expectedSessionId: sessionId,
  });

• Remove only imports made obsolete by the snapshot refactor; src/host/hostMain.ts still needs artifact/storage helpers for screenshot/export paths.

4. Add shared module unit tests

Add test/unit/snapshot/capture.test.ts.

Test responsibilities:

• createSnapshotResult(...) returns structured results without changing existing shape.
• createSnapshotResult(...) returns text results with scrollback lines prepended before visible lines.
• includeCells behavior is preserved by retaining snapshot.cells in structured results and artifacts when present.
• Result schema validation happens before persistence by testing malformed snapshot/result inputs where practical.
• No artifact or manifest writes happen on schema validation failure.
• No artifact or manifest writes happen on expectedSessionId mismatch.
• persistSnapshotArtifact(...) writes snapshot-<seq>-<format>.json content exactly equal to the returned/public result JSON plus trailing newline.
• Manifest entry metadata matches existing behavior, including rendererBackend, dimensions, cursor coordinates, and optional scrollbackLineCount.
• Text snapshots with scrollback still record scrollbackLineCount because metadata comes from the source SemanticSnapshot, not only from structured result fields.
• Structured cells are preserved when present and omitted when absent.
• rendererBackend is required and non-empty.
• Exported-helper consistency checks reject mismatched format/core snapshot-result fields before writing.
• A write failure does not append a manifest entry.

Mock storage/artifact helper dependencies in this test where needed so exact calls can be asserted once at the shared interface boundary.

5. Adjust caller tests without weakening integration coverage

Update test/unit/commands/snapshot.test.ts:

• Keep tests for:
  • default structured format
  • text format
  • includeScrollback and includeCells options passed to live RPC and offline renderer
  • live RPC fallback to offline replay on host unreachable
  • malformed RPC response rejection
  • unsupported format rejection
  • human/JSON output formatting
• Remove or simplify duplicated assertions for low-level artifact filename/content/manifest construction now covered in test/unit/snapshot/capture.test.ts.
• If practical and not brittle under ESM mocking, mock captureSnapshotResult(...) for offline replay routing tests so command tests assert the shared interface is invoked with sessionDir, format, snapshot, rendererBackend, and expectedSessionId. If that adds fragility, keep one command test using the real shared capture with mocked storage helpers and let shared module tests own the exhaustive metadata matrix.

Update test/integration/host-renderer-rpc.test.ts:

• Keep end-to-end assertions that:
  • RPC snapshot returns the expected public result shape.
  • The persisted artifact JSON equals the returned RPC result.
  • A snapshot manifest entry is created with expected high-value metadata.
  • scrollback count omission/presence remains correct.
• Avoid expanding duplicate low-level metadata matrix beyond what shared unit tests own.

6. Preserve public contracts and out-of-scope boundaries

Do not change:

• SnapshotParamsSchema or SnapshotResultSchema public shapes.
• CLI JSON envelope behavior.
• RPC method schemas.
• Snapshot filenames.
• Snapshot artifact JSON formatting.
• Artifact manifest schema.
• Renderer snapshot fidelity, scrollback semantics, or cell field semantics.
• Screenshot, wait, or recording export behavior except for removing unused imports if necessary.

Acceptance criteria

• Live host snapshot RPCs and offline replay snapshots both call the shared snapshot capture implementation for result construction and artifact persistence.
• Snapshot filenames remain snapshot-<seq>-<format>.json.
• Snapshot artifact contents remain exactly the JSON result emitted to callers, formatted with two-space indentation and a trailing newline.
• Manifest entries preserve kind, filename, sessionId, capturedAtSeq, renderer backend metadata, dimensions, cursor coordinates, and optional scrollbackLineCount behavior.
• includeCells: true still requests renderer cell data in both caller paths and preserves cells in structured snapshot results/artifacts.
• includeCells remains omitted from structured results by default when renderer snapshots omit cells.
• Invalid shared result construction fails before artifact persistence.
• A live-host or offline replay snapshot whose SemanticSnapshot.sessionId differs from the expected session id fails fast before persistence.
• Existing public CLI/RPC behavior remains unchanged.

Validation plan

Run the narrowest useful checks first, then broaden as needed:

npm run test:unit -- test/unit/snapshot/capture.test.ts test/unit/commands/snapshot.test.ts
npm run test:integration -- test/integration/host-renderer-rpc.test.ts
npm run typecheck
npm run lint
npm run format:check

If touched code or failures indicate broader risk, run:

npm run test
npm run build

Before committing or opening a PR, run the project-level gate. Prefer the project-standard mise task when available:

mise run ci

If mise is unavailable, run the npm fallback:

npm run verify

Dogfooding plan

Use an isolated absolute home so no real sessions are touched:

export AGENT_TTY_HOME="$(mktemp -d)"

Run one live-host snapshot flow and one offline-replay snapshot flow from the source tree:

# Live host path: keep the PTY running with `exec cat`, then snapshot while the host is reachable.
LIVE_SESSION_JSON=$(npx tsx src/cli/main.ts create --json -- /bin/sh -lc 'for i in $(seq 1 50); do echo live-line-$i; done; exec cat')
LIVE_SESSION_ID=$(printf '%s' "$LIVE_SESSION_JSON" | jq -r '.result.session.sessionId')
LIVE_STRUCTURED_JSON=$(npx tsx src/cli/main.ts snapshot "$LIVE_SESSION_ID" --format structured --include-scrollback --include-cells --json)
LIVE_TEXT_JSON=$(npx tsx src/cli/main.ts snapshot "$LIVE_SESSION_ID" --format text --include-scrollback --json)
LIVE_SCREENSHOT_JSON=$(npx tsx src/cli/main.ts screenshot "$LIVE_SESSION_ID" --json)
npx tsx src/cli/main.ts inspect "$LIVE_SESSION_ID" --json
npx tsx src/cli/main.ts destroy "$LIVE_SESSION_ID" --json
LIVE_WEBM_JSON=$(npx tsx src/cli/main.ts record export "$LIVE_SESSION_ID" --format webm --json)

# Offline replay path: let the process exit, wait for persisted terminal state, then snapshot after exit.
OFFLINE_SESSION_JSON=$(npx tsx src/cli/main.ts create --json -- /bin/sh -lc 'for i in $(seq 1 50); do echo offline-line-$i; done')
OFFLINE_SESSION_ID=$(printf '%s' "$OFFLINE_SESSION_JSON" | jq -r '.result.session.sessionId')
npx tsx src/cli/main.ts wait "$OFFLINE_SESSION_ID" --exit --timeout 5000 --json
OFFLINE_STRUCTURED_JSON=$(npx tsx src/cli/main.ts snapshot "$OFFLINE_SESSION_ID" --format structured --include-scrollback --include-cells --json)
OFFLINE_TEXT_JSON=$(npx tsx src/cli/main.ts snapshot "$OFFLINE_SESSION_ID" --format text --include-scrollback --json)
OFFLINE_SCREENSHOT_JSON=$(npx tsx src/cli/main.ts screenshot "$OFFLINE_SESSION_ID" --json)
OFFLINE_WEBM_JSON=$(npx tsx src/cli/main.ts record export "$OFFLINE_SESSION_ID" --format webm --json)
npx tsx src/cli/main.ts inspect "$OFFLINE_SESSION_ID" --json

Review artifacts:

• Locate both session directories under $AGENT_TTY_HOME/sessions/$LIVE_SESSION_ID and $AGENT_TTY_HOME/sessions/$OFFLINE_SESSION_ID.

• Confirm live and offline snapshot artifact files use snapshot-<seq>-structured.json and snapshot-<seq>-text.json names.

• Confirm each artifact JSON equals the corresponding emitted .result payload from LIVE_STRUCTURED_JSON, LIVE_TEXT_JSON, OFFLINE_STRUCTURED_JSON, and OFFLINE_TEXT_JSON.

• Confirm each artifacts.json contains snapshot entries with rendererBackend, dimensions, cursor coordinates, and scrollbackLineCount only for captures that included scrollback.

• Capture screenshot PNG artifacts and WebM replay artifacts as reviewer-facing proof, even though snapshot JSON equality remains the primary behavior check.

• Attach at least one generated screenshot PNG with attach_file in the implementation report if visual proof is requested/available; include WebM artifact paths because the attachment tool may not support video.

Because this issue is a refactor of JSON result/artifact construction and not a rendered-output change, JSON artifact equality and automated tests are the primary behavior proof; screenshots and WebM recordings are included as reviewable dogfood evidence rather than as indicators of changed visual-rendering behavior.

Risks and mitigations

• Risk: introducing a circular import between protocol, renderer, storage, and snapshot modules.

  • Mitigation: keep src/snapshot/capture.ts dependency direction one-way into protocol/renderer/storage/util only; do not import CLI or host code.

• Risk: caller tests become too mocked and miss integration regressions.

  • Mitigation: keep test/integration/host-renderer-rpc.test.ts artifact equality and manifest smoke assertions.

• Risk: stricter required rendererBackend breaks mocks.

  • Mitigation: update mocks to reflect the production RendererBackend contract; failures here are desirable.

• Risk: validation-before-persistence changes failure behavior.

  • Mitigation: preserve current protocol-error semantics for malformed snapshot results while intentionally avoiding artifact/manifest writes before validation succeeds.

Advisor review incorporated

The plan was reviewed with the advisor and updated to include:

• dogfooding for both live-host and offline-replay snapshot paths,
• symmetric offline expectedSessionId: manifest.sessionId,
• preserved ERROR_CODES.PROTOCOL_ERROR semantics for shared result validation failures,
• consistency checks for exported persistence helpers to prevent mismatched snapshot/result metadata,
• sharper no-write tests for validation failure, session mismatch, and write failure,
• caution against brittle ESM mocking in caller tests,
• and host import cleanup guidance that avoids breaking screenshot/export paths.

ADR decision

No ADR is currently warranted. The decisions are local, easy to reverse, and unsurprising: extract duplicated snapshot result/artifact behavior into a shared snapshot-specific module while preserving the public contract.

---

Generated with mux • Model: openai:gpt-5.5 • Thinking: xhigh

[ThomasK33]

/coder-agents-review

[coder-agents-review]

Clean refactoring, well-scoped to its stated goal. The shared capture module eliminates real duplication between the host and CLI paths, the test suite is genuine (real filesystem, no mock theatre), and the validation-before-persistence ordering is correct and tested. The PR also adds schema validation to the host snapshot path where it was previously absent, which is strictly an improvement.

Panel: 10 reviewers (Bisky, Hisoka, Mafu-san, Mafuuu, Pariston, Ging-ts, Chopper, Robin, Meruem, Melody). Ging-ts and Melody returned with no findings; Hisoka and Pariston confirmed the design holds under adversarial pressure.

1 P2, 4 P3, 1 Nit.

"The (snapshot.scrollbackLines ?? []) fallback in createSnapshotResult is exercised only with scrollback present; the empty-array spread is unproven through the capture path." (Bisky)

🤖 This review was automatically generated with Coder Agents.

[ThomasK33]

/coder-agents-review

[coder-agents-review]

All 6 R1 findings addressed in commit 6503400. Fixes verified by all 6 R2 panel reviewers (Bisky, Mafuuu, Mafu-san, Chopper, Robin, Meruem). Each fix addresses the root cause: parameterized error message instead of hardcoded string, structured CliError instead of bare invariant, exported shared symbols instead of duplication, complete test coverage for the text-without-scrollback path, and centralized test helpers.

One new Nit (AGTTY69-7): the createSemanticSnapshot test factory is duplicated in 4 files, and given this PR already extracted createTemporarySessionDir into test/helpers.ts, extracting the snapshot factory would be the natural companion.

CI passes (11 checks). No P0-P2 findings.

"The fix commit is 6 files, 123+/101-. Proportional to 6 findings. No scope drift. No unrelated changes smuggled in." (Mafu-san)

🤖 This review was automatically generated with Coder Agents.

[ThomasK33]

/coder-agents-review

[coder-agents-review]

AGTTY69-7 verified fixed: createTestSemanticSnapshot consolidated in test/helpers.ts, used by all three test files. Robin confirmed, no duplication remains.

One new Nit (AGTTY69-8). All prior findings remain resolved. CI pending.

🤖 This review was automatically generated with Coder Agents.

[ThomasK33]

/coder-agents-review

[coder-agents-review]

AGTTY69-8 verified fixed: validation failure test now asserts message: 'Snapshot result validation failed.', closing the regression gap for the R1 P2 fix.

All 8 findings across 4 rounds are resolved. No new findings. CI pending.

🤖 This review was automatically generated with Coder Agents.

[ThomasK33]

/coder-agents-review

[coder-agents-review]

Head SHA changed via rebase; the PR-scoped delta since R4 is a CONTEXT.md addition ("Event Log" domain term). No code changes to the snapshot capture module or tests.

All 8 findings from R1-R3 remain resolved. Panel (Bisky, Mafuuu, Robin) reviewed the full diff. No new findings across 5 rounds.

🤖 This review was automatically generated with Coder Agents.