From 8c518c6c17ac9888ea439e7c849bfd801b01dcad Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 20:42:14 +0700 Subject: [PATCH 01/22] =?UTF-8?q?docs:=20journal=20history=20=E2=80=94=20t?= =?UTF-8?q?he=20agent's=20own=20log=20instead=20of=20a=20guess=20at=20its?= =?UTF-8?q?=20screen?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit paddock reconstructs scrollback by diffing viewport snapshots, which its own header calls a viewer rather than a recorder: an agent nobody had open has no history at all, and a scroll bigger than half a screen is a gap. That cannot be improved by asking herdr harder — a coding agent sits on the alternate screen, which has no scrollback ring, so the bytes were never retained. The roadmap already measured the dead end: 500/1000/2000 lines each ~15.8s, returning LESS than `visible` returns in 2ms. The history is on disk instead. Claude Code writes every turn to ~/.claude/projects//.jsonl, and herdr hands us the uuid on AgentInfo.agent_session — verified live against herdr 0.8.2, protocol 20, on the agent.list call paddock already makes, so the pane.list rule is untouched. Measured on one real session: 1.5 MB, 729 records, 40 minutes. Scope is narrow on purpose: "Show earlier" goes deeper and stops having gaps. No conversation view. Six decisions carry the weight — the client only ever sees lines, journal and reconstruction never coexist for one agent, menus are stripped from journal lines so a stale prompt cannot read as the live one, tool_result is never served because that is where file contents and secrets live, the session id stays server-side, and a missing journal is quiet in the UI but loud on the host. Design only. No implementation. Co-Authored-By: Claude Opus 5 --- .../2026-08-20-journal-history-design.md | 264 ++++++++++++++++++ 1 file changed, 264 insertions(+) create mode 100644 docs/design/2026-08-20-journal-history-design.md diff --git a/docs/design/2026-08-20-journal-history-design.md b/docs/design/2026-08-20-journal-history-design.md new file mode 100644 index 0000000..faa55b2 --- /dev/null +++ b/docs/design/2026-08-20-journal-history-design.md @@ -0,0 +1,264 @@ +# Journal history — design + +An agent's history is not on its screen, and paddock has been trying to +reconstruct it from there. This replaces that guess with the harness's own +session log, for the harnesses that keep one. + +The scope is deliberately narrow: **"Show earlier" goes deeper and stops having +gaps.** No new view, no conversation UI. `AgentTerminal` keeps its shape and +changes where its earlier lines come from. + +--- + +## Why the current approach cannot be improved + +`src/web/history.ts` accumulates a transcript by diffing consecutive viewport +snapshots and committing only the lines an offset match proves scrolled off the +top. Its own header calls it a **viewer, not a recorder**, and the two limits +that follow are structural rather than defects: + +- An agent nobody had open has **no history at all**. Nothing was watching, so + nothing was captured. +- A scroll larger than half the visible screen between polls is recorded as a + gap rather than guessed at. + +Asking herdr for more does not help, and this is measured rather than assumed. +From `docs/roadmap.md`, against herdr 0.8.0: `recent_unwrapped` costs ~35 ms per +line past the viewport — 120 lines took 3.1 s, 300 lines 10.7 s (past +`HERDR_TIMEOUT_MS`, so `POST /output` with `lines: 300` fails outright), and +500/1000/2000 lines each took ~15.8 s and returned *less* than `visible` returns +in 2 ms. + +The reason is upstream of paddock. A pane running a coding agent sits on the +terminal's **alternate screen**, which has no scrollback ring — herdr's terminal +core keeps nothing behind the viewport. The bytes were never retained. No clamp, +no timeout and no better parser recovers data that was never kept. + +## Where the history actually is + +Claude Code writes every turn to its own session log as it goes: + +``` +~/.claude/projects//.jsonl +``` + +**And herdr hands us the key.** Verified live against herdr 0.8.2, protocol 20, +on the `agent.list` result paddock already calls — `AgentInfo.agent_session`: + +```json +{"agent": "claude", "kind": "id", "source": "herdr:claude", + "value": ""} +``` + +`pane.list` is not involved, so the rule in `CLAUDE.md` stands untouched. + +This source is strictly better than scrollback would have been: real message +boundaries, timestamps, tool calls adjacent to their results, and it survives the +pane being closed. Measured on one real session while writing this document: +**1.5 MB, 729 records**, spanning 40 minutes — 201 assistant turns, 112 user +records, 93 `Bash` calls. + +Codex, pi and OpenCode each keep an equivalent log. Only Claude ships in v1; the +seam for the others is a registry entry, not a rewrite. + +--- + +## Decisions + +### 1. The journal is flattened server-side; the client only ever sees lines + +`journal/` returns text. The client renders it the way it renders any other +history lines, and gains no per-harness knowledge. + +This is the same reason `parsePrompt` lives in `src/server/` rather than in +`web/`: the dependency rule keeps harness- and protocol-shaped assumptions on the +server side of the socket. A structured-turns payload was considered — it would +let a future conversation view reuse the route unchanged — and rejected, because +it puts a second renderer, and per-harness rendering rules, in `web/`. + +Pushing history over the WebSocket was also rejected: it needs file watching and +a per-agent buffer for every open pane, which is a large amount of machinery for +an affordance the operator taps. + +### 2. Journal history and reconstruction never coexist for one agent + +Where a journal is readable it is the **only** source above the live screen, and +`history.ts` is switched off for that agent. Where one is not, nothing changes +from today. + +Two sources for one range means reconciling overlapping text that was produced by +two different mechanisms, which is guesswork of exactly the kind this feature +exists to remove. Nothing regresses either way: a plain shell pane keeps the +reconstruction it has now. + +### 3. One continuous scroll, with menus stripped from journal lines + +Journal text joins the buffer above the live screen without a labelled divider. + +The cost is stated plainly: those lines are a **reconstruction rendered as +prose**, and cannot reproduce the box drawing and colour the agent actually +painted, so they will not look like the screen below them. + +The sharp edge is not cosmetic, and is designed out rather than accepted. A +journal turn can contain an old prompt menu — `❯ 1. Yes / 2. No` — which, blended +directly above the live screen, reads as the question being asked *now*. +`prompt-parse.ts` already records this failure in its own scoping comment: a +marker left on an already-answered question reappearing as the live menu's +selection. Therefore **cursor markers and option rows are stripped from +journal-derived lines**. Only the live screen may ever render a selectable menu. + +### 4. Prose is served; tool output is not + +The journal holds far more than the screen ever showed: every file the agent +read, every command's output, any secret that passed through either. paddock has +no authentication of its own (decision 3), so what this route serves is bounded +at the source rather than at the gate. + +- **Kept:** assistant text, and user text the operator actually typed. +- **Summarised:** a `tool_use` becomes one line — `▸ Bash ×3 · Read timer.ts` — + carrying the tool name and a short input hint. +- **Dropped:** every `tool_result`. That is where file contents and command + output live. +- **Dropped:** subagent (sidechain) traffic and thinking blocks. + +A `user` record whose content is a **list** is tool-result traffic, not something +a person typed. Folding those into the call that produced them is what stops a +session rendering hundreds of fabricated "you" turns. + +### 5. The session id never reaches the browser + +`adapter.ts` maps `agent_session` into a **server-side** map of `agentId → +session ref`. The wire type `Agent` gains exactly one field, `hasJournal: +boolean`, which is all the UI needs in order to choose a history source. + +A session id is a filesystem key. The browser has no use for one, and paddock +does not hand out filesystem keys to clients that cannot need them. + +### 6. A missing journal is quiet in the UI and loud on the host + +The operator sees the old behaviour, not an error: falling back to reconstruction +is a working dashboard, and a red banner for a pane that never had a journal +would be noise. + +The server does not get to be quiet — `CLAUDE.md` forbids swallowing errors. Each +cause logs once per agent and travels in the response's `detail`: no adapter for +this harness, no session ref from herdr, file missing (compacted, rotated or +deleted), permission denied. An unparseable line skips **that line**, never the +file. + +--- + +## The route + +``` +POST /api/agents/:id/history +→ { before?: string, limit?: number } +← { ok: true, lines: string[], source: "journal" | "reconstruction", + hasMore: boolean, cursor: string | null, detail: string | null } +``` + +POST, not GET: a cursor in a query string lands in edge access logs, which +`CLAUDE.md` forbids. It is a write-shaped request only in verb, so the +same-origin gate (decision 17) covers it like any other POST. + +`source` is on **every** response, so the client never infers provenance. Note +what `"reconstruction"` means precisely, because the server cannot produce those +lines: it is the server saying **"I have no journal for this agent"**, and it +comes with `lines: []` and a `detail`. Reconstruction itself stays entirely +client-side, exactly where it is today. The field is a routing answer, not a +payload description. + +`limit` is counted in **turns**, not lines — a single assistant turn can flatten +to many lines, and a client asking for "50 more" means 50 more things that were +said. `before` is an **opaque cursor** echoed from a previous response; its +contents are the server's business and the client must never construct one. + +Paginated from the tail. One session's file is 1.5 MB, and the existing "Show +earlier" is already an incremental reveal, so the whole file is never sent and +never read: the reader walks backwards from the end in bounded chunks and stops +once it has the requested turns. At the measured ~2 KB per record, 50 turns is +~100 KB read. Two caps, both refused rather than truncated silently: bytes +scanned per request, and lines returned. + +## Path safety + +A session id becomes a path, so it is treated as hostile input at every step: + +1. It must match the canonical uuid shape **before any filesystem call**. +2. The resolved `realpath` must lie inside a configured root. +3. Roots are a **list**, not a string: `CLAUDE_CONFIG_DIR` gives one machine + several Claude homes. Roots are searched in order and the first holding the + session wins — session ids are globally unique, so that is a lookup, not a + guess. + +## Demo mode + +`--demo` has no herdr and no journals, and `README.md` screenshots come from +`--demo`. `docs/roadmap.md` already carries one feature invisible there (the +approve path); adding a second is a choice, not an accident. + +So the demo backend ships a small synthetic journal for one demo agent, with +invented content per house rule 2, and `hasJournal` true for it. "Show earlier" +then works in the mode the screenshots come from. + +This route is therefore registered **unconditionally**, not inside the +`deps.actions` block. It reads a file and never touches herdr, so gating it on a +herdr dependency it does not use would repeat the `/ack` mistake recorded in +`routes.ts`: the one feature that works without herdr being the one visibly +broken in `--demo`. + +## The risk worth stating + +This file is Claude Code's private format, not a documented API, and it will +change without notice. The mitigations are structural rather than hopeful: +unknown record types are tolerated instead of fatal, a bad line is skipped, and +any failure degrades to exactly today's behaviour. The Claude Code version the +shape was verified against goes in the adapter's header and is updated whenever +it is re-checked — the same discipline `docs/gotchas.md` applies to every other +measured claim in this repo. + +--- + +## Files + +| File | Change | +|---|---| +| `src/server/journal/registry.ts` | new — harness → adapter; adding one is a line | +| `src/server/journal/claude.ts` | new — the only adapter in v1 | +| `src/server/journal/files.ts` | new — containment, roots, bounded tail reader | +| `src/server/journal/text.ts` | new — truncation, ANSI stripping, tool summaries | +| `src/server/journal/types.ts` | new — adapter interface, transcript entry | +| `scripts/gen-herdr-types.ts` | emit `agent_session`; it is absent today | +| `src/shared/herdr-api.d.ts` | regenerated by `make types`, never hand-edited | +| `src/server/herdr/adapter.ts` | map `agent_session` into the server-side ref map | +| `src/shared/types.ts` | `Agent.hasJournal: boolean` | +| `src/server/routes.ts` | `POST /api/agents/:id/history` | +| `src/web/components/AgentTerminal.tsx` | "Show earlier" fetches when `hasJournal` | +| `src/web/demo/backend.ts` | synthetic journal for one demo agent | +| `docs/decisions.md` | the new axis, and decisions 1–6 above | +| `docs/architecture.md` | `journal/` in the dependency diagram | + +## Tests + +- **Adapter**, over fixture JSONL with invented content: turn extraction, tool + folding, list-content `user` records folded rather than rendered, sidechain and + thinking dropped, unknown record types tolerated, one bad line skipped without + losing the file. +- **Containment**: `../` refused, a uuid-shaped path resolving outside the root + refused, a non-uuid refused before any filesystem call. +- **Tail reader**: asserts the bytes actually read for a tail request, and that a + record split across a chunk boundary is recovered. +- **Route**: pagination and cursor, `source` on every response, fallback to + `reconstruction` with a `detail`, and the same-origin gate applying to it. +- **Client**: `hasJournal` chooses the source; journal lines carrying a menu are + stripped of markers and option rows before they enter the buffer. +- **Mutation pass**, per house rule 4: break each guard and watch the test fail + before trusting it. + +## Documentation + +`docs/gotchas.md` gains the alternate-screen finding — why scrollback cannot be +read from herdr at all — since it is the measurement that justifies this entire +feature and will otherwise be rediscovered. `docs/roadmap.md`'s +`MAX_READ_LINES` entry is resolved by it and should say so rather than being +deleted. From 5f79f384f3bf43d3fdf3cd67d0eef31c46396888 Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 20:46:45 +0700 Subject: [PATCH 02/22] fix: a shutdown you asked for is not a tunnel that failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `^C` signals the whole foreground process group, so cloudflared dies at the same moment paddock's teardown runs. Two paths then reported the same event: `teardown` printed its closing line, and the race watching `child.exited` printed `cloudflared exited 143 — the URL is gone` beside it, followed by the held cloudflared tail as the diagnosis. 143 is the signal the operator sent, and the URL going away is what they asked for. Worse, on a tunnel that had been up for half an hour the tail is whatever cloudflared last happened to say — its SUCCESSFUL startup connectivity prechecks — presented as the explanation of a crash. The one message that mattered was buried in it. The run already knows: `stopping` is set synchronously by `teardown`. Gate the failure branch on it, and take the exit status from `teardown`, which answers with the outcome it already had. Nothing is silenced. cloudflared's own shutdown lines still print live — `teardown` drops the display first, so the log sink is pass-through again — `tunnel closed` is still the closing report, and a kill that FAILED still warns, still names the command to check by hand, and still ends the run non-zero. Quieting a diagnosis is only safe on the one path where there is nothing to diagnose. Two tests, both watched fail first: a requested stop reports no failure and no tail, and a requested stop whose kill was refused still exits non-zero — the one thing the quieting must not take with it. Co-Authored-By: Claude Opus 5 --- docs/gotchas.md | 1 + src/server/tunnel/run.ts | 21 ++++++++++ tests/tunnel-run.test.ts | 86 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+) diff --git a/docs/gotchas.md b/docs/gotchas.md index 253dd3e..5acd94a 100644 --- a/docs/gotchas.md +++ b/docs/gotchas.md @@ -11,6 +11,7 @@ one, recorded here so they are not reintroduced. | Sensitive paths in access logs | Payload sent as a GET query string | POST bodies only | | A subprocess's log lines flash and vanish | Child output and a once-a-second `ESC[H ESC[J` repaint share stdout | Buffer the child's lines while the display owns the screen, print the tail on every failure path — never silence them | | The screen claims a tunnel is up while it is dying | `^C` signals the whole process group, so the child begins shutdown before the draw timer is cleared | Clear the block in teardown; a stale frame asserting the opposite is worse than no frame | +| A shutdown the operator ASKED for is reported as the tunnel failing | Same process group: the child dies at the same moment the teardown runs, and the race watching `child.exited` cannot tell a requested death from a crash — so `^C` printed `cloudflared exited 143 — the URL is gone` plus a tail that, on a tunnel up for half an hour, was its SUCCESSFUL startup prechecks | Gate that branch on `stopping`. Quieting a diagnosis is only safe where there is nothing to diagnose: the live shutdown lines, the closing report, and a failed kill's warning and non-zero exit all remain | | The URL is the fourth line printed | Boot diagnostics log as they happen, and the port is not bound until after them | Collect boot facts, emit one summary line, then the banner (`boot-log.ts`) | | Service worker silently disabled | Auth check gates every route including `/sw.js` | No app token; Access is the gate — its cookie rides a same-origin fetch, a bearer token has nothing to ride | | Works on one hostname, not another | Hostname allowlist in the client | Derive the WebSocket URL from `location`, unconditionally | diff --git a/src/server/tunnel/run.ts b/src/server/tunnel/run.ts index e12b5ef..c220a45 100644 --- a/src/server/tunnel/run.ts +++ b/src/server/tunnel/run.ts @@ -423,6 +423,27 @@ export async function runTunnel(deps: TunnelDeps): Promise { ]); if (outcome.kind === "child") { + /** + * A death this run ASKED for is not a failure, and must not be reported as + * one. + * + * `^C` reaches cloudflared straight from the tty — same foreground process + * group — so the child dies at the same moment index.ts's handler calls + * `teardown`. Both land: the teardown prints its closing report, and this + * branch used to print `cloudflared exited 143 — the URL is gone` beside + * it, followed by the tail as its diagnosis. On a tunnel that had been up + * for half an hour that tail is whatever cloudflared last happened to say + * — its SUCCESSFUL startup prechecks — presented as the explanation of a + * crash. 143 is the signal the operator sent, and the URL going away is + * what they asked for. + * + * Nothing is swallowed. cloudflared's shutdown lines still print live + * (`teardown` drops the display first, so `onLog` is pass-through again), + * `teardown` still says `tunnel closed`, and a kill that FAILED still + * warns and still ends the run non-zero — the exit status comes from + * `teardown`, which answers with the outcome it already had. + */ + if (stopping) return (await teardown()) ? 0 : 1; warn(`paddock: cloudflared exited ${outcome.code} — the URL is gone`); // The lines above are the diagnosis. Without them this message names the // exit code and nothing that would explain it. diff --git a/tests/tunnel-run.test.ts b/tests/tunnel-run.test.ts index c6add4e..c7a5019 100644 --- a/tests/tunnel-run.test.ts +++ b/tests/tunnel-run.test.ts @@ -503,3 +503,89 @@ test("teardown clears the block instead of leaving it claiming the tunnel is up" expect(out.writes.slice(marker).join("")).toContain("\x1b[H\x1b[J"); } finally { c.restore(); out.restore(); } }); + +// A `^C` reaches cloudflared straight from the tty, so the child dies at the +// same moment paddock's teardown runs — and the race in `runTunnel` used to +// take that death for an unexplained one. The operator got +// `cloudflared exited 143 — the URL is gone` plus a 50-line tail, which on a +// long-lived tunnel is the last thing cloudflared happened to say: its +// SUCCESSFUL startup prechecks, printed as if they explained a crash. +test("a requested stop is not reported as a tunnel that failed", async () => { + const out = captureStdout(); + const c = capture(); + const reg: { teardown: (() => Promise) | null } = { teardown: null }; + const sink: { emit: ((l: string) => void) | null } = { emit: null }; + let code: number; + try { + let release: (n: number) => void = () => {}; + const exited = new Promise((r) => { release = r; }); + const run = runTunnel({ + ...base(), + isTty: true, + port: 0, + startTunnel: fakeTunnel({ + onSink: (emit) => { + sink.emit = emit; + // Held back while the display owns the screen: the buffer a real run + // arrives at teardown with. + void Bun.sleep(5).then(() => emit("INF Registered tunnel connection")); + }, + exited, + // The child is already on its way out by the time the kill lands. + stop: async () => { + sink.emit?.("INF Initiating graceful shutdown due to signal interrupt"); + release(143); + }, + }), + registerShutdown: (fn) => { reg.teardown = fn; }, + }); + await Bun.sleep(10); + expect(await reg.teardown!()).toBe(true); + code = await run; + } finally { c.restore(); out.restore(); } + + const text = c.text(); + // Not silence: cloudflared's own account of the shutdown still comes through, + // and the teardown's closing line is the report. + expect(text).toContain("graceful shutdown due to signal interrupt"); + expect(text).toContain("tunnel closed"); + // 143 is the signal the operator sent, and the URL going away is what they + // asked for. Neither is a failure to warn about. + expect(text).not.toContain("the URL is gone"); + // And there is nothing to diagnose, so no tail. + expect(text).not.toContain("line(s) from cloudflared"); + expect(code).toBe(0); +}); + +test("a requested stop whose kill failed still ends the run non-zero", async () => { + // The one thing that must survive the quieting above. A `stop()` that was + // refused means a cloudflared may still be holding a public URL with no + // paddock behind it, and the exit status is all a wrapper script has. + const out = captureStdout(); + const c = capture(); + const reg: { teardown: (() => Promise) | null } = { teardown: null }; + let code: number; + try { + let release: (n: number) => void = () => {}; + const exited = new Promise((r) => { release = r; }); + const run = runTunnel({ + ...base(), + isTty: true, + port: 0, + startTunnel: fakeTunnel({ + exited, + stop: async () => { + release(143); + throw new Error("kill: operation not permitted"); + }, + }), + registerShutdown: (fn) => { reg.teardown = fn; }, + }); + await Bun.sleep(10); + expect(await reg.teardown!()).toBe(false); + code = await run; + } finally { c.restore(); out.restore(); } + + expect(code).toBe(1); + expect(c.text()).toContain("the tunnel may still be up"); +}); From b2aef7bec5e6b1419b75212d3091fad960baef0e Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 20:50:04 +0700 Subject: [PATCH 03/22] docs: implementation plan for journal history Nine tasks, each ending in an independently testable deliverable and its own commit: generate agent_session, hasJournal on the wire with session ids off it, the registry, path containment plus a bounded tail reader, text shaping, the Claude adapter, the route, the terminal view, and a demo journal so --demo can still take the screenshots. Written against docs/design/2026-08-20-journal-history-design.md. Tests come first in every task, with the mutation pass house rule 4 asks for on each of the three guards that matter: containment, menu stripping, and the tool-result exclusion. Co-Authored-By: Claude Opus 5 --- docs/plans/2026-08-20-journal-history.md | 1732 ++++++++++++++++++++++ 1 file changed, 1732 insertions(+) create mode 100644 docs/plans/2026-08-20-journal-history.md diff --git a/docs/plans/2026-08-20-journal-history.md b/docs/plans/2026-08-20-journal-history.md new file mode 100644 index 0000000..947065b --- /dev/null +++ b/docs/plans/2026-08-20-journal-history.md @@ -0,0 +1,1732 @@ +# Journal History Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace paddock's reconstructed scrollback with the coding agent's own on-disk session log, so "Show earlier" goes deeper and stops having gaps. + +**Architecture:** A new `src/server/journal/` reads the harness's JSONL session log, flattens it to text lines server-side, and serves them from a paginated `POST /api/agents/:id/history`. herdr supplies the session id on `agent.list`. The client keeps its current terminal view and only changes where earlier lines come from. Where no journal exists, today's client-side reconstruction is untouched. + +**Tech Stack:** Bun, TypeScript, Hono, React, `bun:test`. + +**Spec:** `docs/design/2026-08-20-journal-history-design.md` — read it first; this plan argues from it. + +## Global Constraints + +- **This repository is public.** No hostnames, home paths, usernames, employer terms, or real agent names — in code, comments, fixtures, tests, or commit messages. Fixtures use invented names: `api-refactor`, `flaky-test-fix`, `docs-cleanup`, `schema-migration`. Run `make check-clean` before every commit; if it fails, fix the content, never the denylist. +- **Dependency direction:** `herdr/socket → herdr/adapter → state/store → ws/hub → web/`. `journal/` is a NEW axis beside `herdr/` — it knows about *harnesses*, never about herdr, and never imports from `herdr/`. `adapter.ts` must not import `journal/` either; the predicate it needs is injected (Task 2). +- **`src/shared/types.ts` is the one payload contract.** Never redeclare a payload shape on the UI side. +- **`src/shared/herdr-api.d.ts` is generated** by `make types`. Never hand-edit it. +- **Never swallow errors.** No `2>/dev/null`, no empty catch blocks. A skipped JSONL line is logged; a failed journal read is reported in the response `detail` and logged once per agent. +- **Never put payloads in a GET query string.** POST bodies only. +- **Gate before every commit:** `make check && make check-clean && make test`. +- **House rule 4:** before trusting a new test, break the thing it guards and watch it go red. +- Claude Code journal shape was verified against the version recorded in `journal/claude.ts`'s header. Re-verify and update that header whenever the shape is re-checked. + +--- + +### Task 1: `agent_session` reaches paddock's generated types + +herdr already sends it; paddock's generated `herdr-api.d.ts` does not declare it, so nothing downstream can read it. + +**Files:** +- Modify: `scripts/gen-herdr-types.ts` (the `HerdrAgentRaw` template literal, around line 62) +- Modify: `src/shared/herdr-api.d.ts` (regenerated, never hand-edited) +- Test: `tests/herdr-types-guard.test.ts` + +**Interfaces:** +- Consumes: nothing. +- Produces: `HerdrAgentSession` (`{ agent: string; kind: string; source: string; value: string }`) and `HerdrAgentRaw.agent_session?: HerdrAgentSession | null`, both exported from `@shared/herdr-api`. + +- [ ] **Step 1: Write the failing test** + +Add to `tests/herdr-types-guard.test.ts`: + +```ts +test("HerdrAgentRaw declares agent_session, the key journal history needs", () => { + // A generated file, so this asserts the GENERATOR emitted it. herdr 0.8.2 + // sends agent_session on every agent.list row; without it in the declared + // shape, journal/ has no session id to look up and the feature is dead at + // the type level rather than at runtime. + const src = readFileSync("src/shared/herdr-api.d.ts", "utf8"); + expect(src).toContain("export interface HerdrAgentSession"); + expect(src).toMatch(/agent_session\?: HerdrAgentSession \| null;/); +}); +``` + +Ensure `readFileSync` is imported at the top of that file: `import { readFileSync } from "node:fs";` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `bun test tests/herdr-types-guard.test.ts` +Expected: FAIL — `expect(received).toContain("export interface HerdrAgentSession")`. + +- [ ] **Step 3: Emit the type from the generator** + +In `scripts/gen-herdr-types.ts`, immediately before the `/** One entry from \`agent.list\` ... */` block, add: + +```ts +/** + * The harness session herdr has associated with a pane, or null. + * + * \`kind\` is "id" for a session identifier and \`value\` is that id; + * \`agent\` names the harness ("claude", "codex"). This is the key + * \`src/server/journal/\` uses to find the harness's own log, and it is why + * that feature needs no second herdr call: it rides on \`agent.list\`. + */ +export interface HerdrAgentSession { + agent: string; + kind: string; + source: string; + value: string; +} +``` + +Then add this line inside the `HerdrAgentRaw` interface body, after `agent_status`: + +```ts + agent_session?: HerdrAgentSession | null; +``` + +- [ ] **Step 4: Regenerate and verify** + +Run: `make types && bun test tests/herdr-types-guard.test.ts` +Expected: PASS. `git diff src/shared/herdr-api.d.ts` shows only the two additions. + +- [ ] **Step 5: Confirm the live schema agrees** + +Run: `herdr api schema --json | grep -c agent_session` +Expected: non-zero. If zero, STOP — the installed herdr predates the field and the rest of this plan cannot work; report it rather than proceeding. + +- [ ] **Step 6: Commit** + +```bash +make check && make check-clean && make test +git add scripts/gen-herdr-types.ts src/shared/herdr-api.d.ts tests/herdr-types-guard.test.ts +git commit -m "feat: generate agent_session, the key a journal lookup needs" +``` + +--- + +### Task 2: `hasJournal` on the wire, session ids off it + +The browser must learn *whether* history exists without ever receiving a filesystem key. + +**Files:** +- Modify: `src/shared/types.ts` (the `Agent` interface) +- Modify: `src/server/herdr/adapter.ts` (`AdaptContext`, `toAgent`, new `sessionRefs`) +- Modify: `src/server/supervisor.ts:257` (the `toAgents` call) +- Test: `tests/adapter.test.ts` + +**Interfaces:** +- Consumes: `HerdrAgentSession`, `HerdrAgentRaw` from Task 1. +- Produces: + - `Agent.hasJournal: boolean` (required, not optional). + - `AdaptContext.hasJournal?: (session: HerdrAgentSession | null | undefined) => boolean` — injected predicate, defaults to `() => false`. + - `sessionRefs(rows: HerdrAgentRaw[]): Map` keyed by `pane_id`. + +- [ ] **Step 1: Write the failing tests** + +Add to `tests/adapter.test.ts` (reuse that file's existing `raw()` helper; pass `agent_session` through it): + +```ts +test("hasJournal is false when herdr sends no session", () => { + const [a] = toAgents([raw({ pane_id: "w1:p1", name: "api-refactor" })], ctx()); + expect(a!.hasJournal).toBe(false); +}); + +test("hasJournal asks the injected predicate, never the harness name directly", () => { + // Injected, because `adapter.ts` sits on the herdr axis and `journal/` sits + // on the harness axis. A direct import would tie the two together and put + // harness knowledge in the herdr adapter. + const session = { agent: "claude", kind: "id", source: "herdr:claude", value: "u" }; + const [a] = toAgents([raw({ pane_id: "w1:p1", agent_session: session })], { + ...ctx(), + hasJournal: (s) => s?.agent === "claude", + }); + expect(a!.hasJournal).toBe(true); +}); + +test("sessionRefs keys by pane id and drops rows with no session", () => { + const session = { agent: "claude", kind: "id", source: "herdr:claude", value: "u1" }; + const refs = sessionRefs([ + raw({ pane_id: "w1:p1", agent_session: session }), + raw({ pane_id: "w1:p2" }), + ]); + expect(refs.get("w1:p1")).toEqual(session); + expect(refs.has("w1:p2")).toBe(false); +}); + +test("the session id is NOT on the wire type", () => { + // A session id is a filesystem key. The browser cannot need one, and paddock + // does not hand filesystem keys to clients. Asserted on the serialized shape + // because that is what actually crosses the socket. + const session = { agent: "claude", kind: "id", source: "herdr:claude", value: "secret-uuid" }; + const [a] = toAgents([raw({ pane_id: "w1:p1", agent_session: session })], ctx()); + expect(JSON.stringify(a)).not.toContain("secret-uuid"); +}); +``` + +Import `sessionRefs` alongside `toAgents` at the top of the file. + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `bun test tests/adapter.test.ts` +Expected: FAIL — `sessionRefs` is not exported; `hasJournal` is undefined. + +- [ ] **Step 3: Add the field to the payload contract** + +In `src/shared/types.ts`, inside `interface Agent`, after `acknowledgedAt`: + +```ts + /** + * Whether paddock can read this agent's own session log, which decides + * WHICH history source the terminal view uses (see + * `docs/design/2026-08-20-journal-history-design.md`). + * + * A boolean and nothing more, deliberately. The session id it is derived + * from is a filesystem key that stays on the server: the UI's only question + * is "fetch, or use my local reconstruction?". + * + * Required, not optional — an optional field lets a future edit drop it + * silently, and the terminal would fall back to reconstruction for every + * agent with nothing to notice. + */ + hasJournal: boolean; +``` + +- [ ] **Step 4: Implement in the adapter** + +In `src/server/herdr/adapter.ts`, extend `AdaptContext`: + +```ts +export interface AdaptContext { + hostId: string; + labels: Map; + now: number; + /** + * Whether a journal adapter exists for this session. INJECTED rather than + * imported: `journal/` is a harness-axis module and this file is the herdr + * adapter, so importing it here would cross the two axes permanently. + * Defaults to false, which is exactly "paddock reads no journals". + */ + hasJournal?: (session: HerdrAgentSession | null | undefined) => boolean; +} +``` + +Add to the object literal returned by `toAgent`, after `acknowledgedAt`: + +```ts + hasJournal: ctx.hasJournal?.(rawAgent.agent_session) ?? false, +``` + +Add at the end of the file: + +```ts +/** + * Session ids by pane id, for the server side only. + * + * Separate from `toAgents` because the result must NOT travel with the agent: + * `Agent` crosses the socket to the browser and this does not. + */ +export function sessionRefs(rows: HerdrAgentRaw[]): Map { + const out = new Map(); + for (const row of rows) { + if (row.agent_session) out.set(row.pane_id, row.agent_session); + } + return out; +} +``` + +Import the type: add `HerdrAgentSession` to the existing `@shared/herdr-api` import. + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `bun test tests/adapter.test.ts` +Expected: PASS. `bunx tsc --noEmit` will now fail wherever an `Agent` literal is built without `hasJournal` — fix each by adding `hasJournal: false` (tests, `src/web/demo/backend.ts`, fixtures). That is the required-field guarantee working. + +- [ ] **Step 6: Commit** + +```bash +make check && make check-clean && make test +git add -A +git commit -m "feat: hasJournal on the wire, session ids off it" +``` + +--- + +### Task 3: the journal registry and its types + +Pure module: no filesystem, no herdr. It answers "which adapter, if any". + +**Files:** +- Create: `src/server/journal/types.ts` +- Create: `src/server/journal/registry.ts` +- Test: `tests/journal-registry.test.ts` + +**Interfaces:** +- Consumes: `HerdrAgentSession` (Task 1). +- Produces: + - `interface JournalEntry { role: "user" | "assistant"; at: string | null; text: string; tools: string[] }` + - `interface JournalAdapter { name: string; verifiedAgainst: string; locate(value: string, roots: readonly string[]): Promise; parse(chunk: string): JournalEntry[] }` + - `interface JournalRoots { claude: readonly string[] }` + - `adapterFor(session: HerdrAgentSession | null | undefined): JournalAdapter | null` + - `hasAdapter(session: HerdrAgentSession | null | undefined): boolean` + +- [ ] **Step 1: Write the failing test** + +Create `tests/journal-registry.test.ts`: + +```ts +import { expect, test } from "bun:test"; +import { adapterFor, hasAdapter } from "@server/journal/registry"; + +const claude = { agent: "claude", kind: "id", source: "herdr:claude", value: "u1" }; + +test("a claude session resolves to the claude adapter", () => { + expect(adapterFor(claude)?.name).toBe("claude"); + expect(hasAdapter(claude)).toBe(true); +}); + +test("a harness with no adapter is an ordinary no, not an error", () => { + // The route reports this as `source: "reconstruction"`, so an unknown + // harness must be a null rather than a throw. + expect(adapterFor({ ...claude, agent: "some-other-harness" })).toBeNull(); + expect(hasAdapter({ ...claude, agent: "some-other-harness" })).toBe(false); +}); + +test("a session that is not an id is refused", () => { + // `kind` can name something that is not a session identifier. Only "id" is + // a value this code knows how to turn into a path. + expect(hasAdapter({ ...claude, kind: "path" })).toBe(false); +}); + +test("no session at all is false, never a throw", () => { + expect(hasAdapter(null)).toBe(false); + expect(hasAdapter(undefined)).toBe(false); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `bun test tests/journal-registry.test.ts` +Expected: FAIL — cannot find module `@server/journal/registry`. + +- [ ] **Step 3: Write the types** + +Create `src/server/journal/types.ts`: + +```ts +/** + * The journal axis: reading a coding agent's OWN session log. + * + * This module tree knows about harnesses (Claude Code, codex, pi) and nothing + * about herdr. It must never import from `@server/herdr/` — see + * `docs/architecture.md`. herdr's only contribution is the session id, handed + * across as a plain string by the caller. + */ + +/** One turn, already stripped of everything not being served. */ +export interface JournalEntry { + role: "user" | "assistant"; + /** ISO timestamp as the harness wrote it, or null if the record had none. */ + at: string | null; + /** Prose only. ANSI removed, menus removed, truncated. */ + text: string; + /** One-line tool summaries, e.g. "Bash ×3". Never tool OUTPUT. */ + tools: string[]; +} + +export interface JournalAdapter { + /** Harness name, matching herdr's `agent_session.agent`. */ + name: string; + /** + * The harness version this adapter's record shape was last verified against. + * A private on-disk format with no compatibility promise, so this is the + * only honest way to record what "known good" means. + */ + verifiedAgainst: string; + /** Absolute path of the session's log, or null when it cannot be found. */ + locate(value: string, roots: readonly string[]): Promise; + /** Parse a raw slice of the log. Unknown records are ignored, never fatal. */ + parse(chunk: string): JournalEntry[]; +} + +/** Where each harness keeps its logs. A LIST: one machine can hold several. */ +export interface JournalRoots { + claude: readonly string[]; +} +``` + +- [ ] **Step 4: Write the registry** + +Create `src/server/journal/registry.ts`: + +```ts +import { claudeAdapter } from "@server/journal/claude"; +import type { HerdrAgentSession } from "@shared/herdr-api"; +import type { JournalAdapter } from "@server/journal/types"; + +/** + * The SINGLE decision site for "does this agent have a readable history". + * + * Adding a harness is one entry here plus its adapter module — never a new + * branch in the route and never a condition in the client. + */ +const ADAPTERS: readonly JournalAdapter[] = [claudeAdapter]; + +export function adapterFor(session: HerdrAgentSession | null | undefined): JournalAdapter | null { + if (!session) return null; + // Only an id can become a path. Any other `kind` is a value this code has no + // way to resolve, and guessing is how a lookup becomes a traversal. + if (session.kind !== "id") return null; + return ADAPTERS.find((a) => a.name === session.agent) ?? null; +} + +export function hasAdapter(session: HerdrAgentSession | null | undefined): boolean { + return adapterFor(session) !== null; +} +``` + +- [ ] **Step 5: Stub the claude adapter so the registry compiles** + +Create `src/server/journal/claude.ts` with the minimum this task needs; Task 6 fills it in: + +```ts +import type { JournalAdapter } from "@server/journal/types"; + +export const claudeAdapter: JournalAdapter = { + name: "claude", + verifiedAgainst: "unverified", + async locate() { return null; }, + parse() { return []; }, +}; +``` + +- [ ] **Step 6: Run test to verify it passes** + +Run: `bun test tests/journal-registry.test.ts` +Expected: PASS (4 tests). + +- [ ] **Step 7: Commit** + +```bash +make check && make check-clean && make test +git add src/server/journal tests/journal-registry.test.ts +git commit -m "feat: the journal registry — one decision site for readable history" +``` + +--- + +### Task 4: safe paths and a bounded tail reader + +A session id becomes a filesystem path, so it is hostile input. This task is the containment boundary, and it is worth its own review gate. + +**Files:** +- Create: `src/server/journal/files.ts` +- Test: `tests/journal-files.test.ts` + +**Interfaces:** +- Consumes: nothing. +- Produces: + - `isSessionId(value: string): boolean` + - `claudeRoots(env: Record, home: string): string[]` + - `containedRealpath(root: string, candidate: string): Promise` + - `tailChunk(path: string, endByte: number, maxBytes: number): Promise<{ text: string; startByte: number }>` + - `MAX_TAIL_BYTES = 512_000` + +- [ ] **Step 1: Write the failing tests** + +Create `tests/journal-files.test.ts`: + +```ts +import { expect, test } from "bun:test"; +import { mkdtemp, mkdir, writeFile, symlink } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + claudeRoots, containedRealpath, isSessionId, MAX_TAIL_BYTES, tailChunk, +} from "@server/journal/files"; + +const UUID = "f4971cd4-d53b-430a-8fc6-a0d4572103ae"; + +test("only a canonical uuid is a session id", () => { + expect(isSessionId(UUID)).toBe(true); + expect(isSessionId("../../etc/passwd")).toBe(false); + expect(isSessionId(`${UUID}/../..`)).toBe(false); + expect(isSessionId("")).toBe(false); + expect(isSessionId(`${UUID}.jsonl`)).toBe(false); +}); + +test("claudeRoots defaults to the home projects dir", () => { + expect(claudeRoots({}, "/srv/operator")).toEqual(["/srv/operator/.claude/projects"]); +}); + +test("claudeRoots takes several config dirs, comma-separated and in order", () => { + // One machine can hold several Claude homes — a per-profile CLAUDE_CONFIG_DIR + // is the case that forces a list rather than a string. + expect(claudeRoots({ CLAUDE_CONFIG_DIR: "/a, /b" }, "/srv/operator")) + .toEqual(["/a/projects", "/b/projects"]); +}); + +test("containedRealpath accepts a file inside the root", async () => { + const root = await mkdtemp(join(tmpdir(), "paddock-j-")); + await mkdir(join(root, "proj")); + const file = join(root, "proj", `${UUID}.jsonl`); + await writeFile(file, "{}\n"); + expect(await containedRealpath(root, file)).toBe(file); +}); + +test("containedRealpath refuses a path that escapes via ..", async () => { + const root = await mkdtemp(join(tmpdir(), "paddock-j-")); + expect(await containedRealpath(root, join(root, "..", "escape.jsonl"))).toBeNull(); +}); + +test("containedRealpath refuses a symlink pointing outside the root", async () => { + // The check is on the RESOLVED path, not the requested one: a symlink inside + // the root is the way a string that looks contained stops being contained. + const root = await mkdtemp(join(tmpdir(), "paddock-j-")); + const outside = await mkdtemp(join(tmpdir(), "paddock-out-")); + const target = join(outside, "secrets.jsonl"); + await writeFile(target, "{}\n"); + const link = join(root, `${UUID}.jsonl`); + await symlink(target, link); + expect(await containedRealpath(root, link)).toBeNull(); +}); + +test("containedRealpath returns null for a file that does not exist", async () => { + const root = await mkdtemp(join(tmpdir(), "paddock-j-")); + expect(await containedRealpath(root, join(root, `${UUID}.jsonl`))).toBeNull(); +}); + +test("tailChunk reads from the END and reports where it started", async () => { + const root = await mkdtemp(join(tmpdir(), "paddock-j-")); + const file = join(root, "big.jsonl"); + const body = Array.from({ length: 100 }, (_, i) => `line-${i}`).join("\n"); + await writeFile(file, body); + const { text, startByte } = await tailChunk(file, body.length, 40); + expect(text.endsWith("line-99")).toBe(true); + expect(startByte).toBe(body.length - 40); + expect(text.length).toBe(40); +}); + +test("tailChunk never reads before the start of the file", async () => { + const root = await mkdtemp(join(tmpdir(), "paddock-j-")); + const file = join(root, "small.jsonl"); + await writeFile(file, "abc"); + const { text, startByte } = await tailChunk(file, 3, 999); + expect(text).toBe("abc"); + expect(startByte).toBe(0); +}); + +test("the tail cap is bounded, so one request cannot read a whole huge log", () => { + // Measured: a real session is 1.5 MB / 729 records, ~2 KB per record. This + // cap is ~250 records' worth per request, well above one page of history. + expect(MAX_TAIL_BYTES).toBe(512_000); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `bun test tests/journal-files.test.ts` +Expected: FAIL — cannot find module `@server/journal/files`. + +- [ ] **Step 3: Implement** + +Create `src/server/journal/files.ts`: + +```ts +import { realpath } from "node:fs/promises"; +import { join, resolve, sep } from "node:path"; + +/** + * Bytes one request may read from a journal. + * + * Measured on a real session: 1.5 MB across 729 records, ~2 KB per record. So + * this is ~250 records per request — far more than one page of "show earlier", + * and far less than a whole log. A cap on the REQUEST, not on the file: paging + * backwards still reaches the beginning, one bounded read at a time. + */ +export const MAX_TAIL_BYTES = 512_000; + +/** A session id as the harness writes it: canonical 8-4-4-4-12 hex. */ +const SESSION_ID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** + * Whether a value may be turned into a path AT ALL. + * + * Anchored on both ends and checked BEFORE any filesystem call. This is the + * cheap half of containment: nothing with a separator, a dot segment, or an + * extension ever reaches `realpath`. + */ +export function isSessionId(value: string): boolean { + return SESSION_ID_RE.test(value); +} + +/** + * Claude Code's project roots, in search order. + * + * A LIST because `CLAUDE_CONFIG_DIR` is per-profile and one machine can hold + * several Claude homes. Comma-separated, trimmed, empties dropped. + */ +export function claudeRoots(env: Record, home: string): string[] { + const configured = (env.CLAUDE_CONFIG_DIR ?? "") + .split(",") + .map((s) => s.trim()) + .filter((s) => s !== ""); + const dirs = configured.length > 0 ? configured : [join(home, ".claude")]; + return dirs.map((d) => join(d, "projects")); +} + +/** + * The resolved path, if and only if it really sits inside `root`. + * + * Resolved with `realpath`, never compared as strings: a symlink inside the + * root is exactly how a path that LOOKS contained stops being contained, and a + * journal root is a directory the operator's tools write into freely. + * + * Returns null rather than throwing for a missing file — "no journal here" is + * an ordinary answer this feature reports as a fallback, not an exception. + */ +export async function containedRealpath(root: string, candidate: string): Promise { + let real: string; + let realRoot: string; + try { + real = await realpath(resolve(candidate)); + realRoot = await realpath(resolve(root)); + } catch { + return null; + } + const prefix = realRoot.endsWith(sep) ? realRoot : realRoot + sep; + return real.startsWith(prefix) ? real : null; +} + +/** + * The last `maxBytes` of the file ending at `endByte`, and where that slice + * began. + * + * Reads BACKWARDS from a byte offset rather than loading the file: paging is + * the whole reason the route is cursored, and a 1.5 MB read per "show earlier" + * tap on a phone is the cost this avoids. `startByte` is what the caller + * returns as the next cursor. + */ +export async function tailChunk( + path: string, + endByte: number, + maxBytes: number, +): Promise<{ text: string; startByte: number }> { + const capped = Math.min(maxBytes, MAX_TAIL_BYTES); + const startByte = Math.max(0, endByte - capped); + const text = await Bun.file(path).slice(startByte, endByte).text(); + return { text, startByte }; +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `bun test tests/journal-files.test.ts` +Expected: PASS (10 tests). + +- [ ] **Step 5: Mutation-check the containment guard (house rule 4)** + +Temporarily change `containedRealpath`'s last line to `return real;` and re-run. +Expected: the `..` and symlink tests go RED. Restore, confirm green again. Do the same for `isSessionId` by making it `return true` — the traversal tests must go red. + +- [ ] **Step 6: Commit** + +```bash +make check && make check-clean && make test +git add src/server/journal/files.ts tests/journal-files.test.ts +git commit -m "feat: journal path containment and a bounded tail reader" +``` + +--- + +### Task 5: text shaping — what is served and what is never served + +The exposure decision lives here: prose is kept, tool output is dropped, and menus are stripped so a stale prompt cannot read as the live one. + +**Files:** +- Create: `src/server/journal/text.ts` +- Test: `tests/journal-text.test.ts` + +**Interfaces:** +- Consumes: `JournalEntry` (Task 3). +- Produces: + - `stripAnsi(text: string): string` + - `stripMenu(text: string): string` + - `summariseTool(name: string, input: unknown): string` + - `clamp(text: string, max: number): string` + - `toLines(entries: readonly JournalEntry[]): string[]` + - `MAX_TEXT_CHARS = 4_000` + +- [ ] **Step 1: Write the failing tests** + +Create `tests/journal-text.test.ts`: + +```ts +import { expect, test } from "bun:test"; +import { + clamp, MAX_TEXT_CHARS, stripAnsi, stripMenu, summariseTool, toLines, +} from "@server/journal/text"; + +test("ansi escapes are removed", () => { + expect(stripAnsi("hello")).toBe("hello"); +}); + +test("a cursor marker is stripped from journal text", () => { + // THE hazard. A journal turn can carry an ALREADY ANSWERED menu, and blended + // straight above the live screen it reads as the question being asked now. + // `prompt-parse.ts` records this exact failure. Only the live screen may + // render a selectable menu. + expect(stripMenu("❯ 1. Yes")).toBe(""); + expect(stripMenu(" ❯ 2. No, keep it")).toBe(""); +}); + +test("a numbered option row is stripped even without a cursor", () => { + expect(stripMenu(" 2. No")).toBe(""); + expect(stripMenu("1. Approve this change")).toBe(""); +}); + +test("ordinary prose that merely starts with a number survives", () => { + // Over-stripping would silently eat real content, which is worse than the + // hazard it guards: "2. " here is prose the agent wrote, not an option row. + expect(stripMenu("2026 was the year")).toBe("2026 was the year"); + expect(stripMenu("I found 3 failures")).toBe("I found 3 failures"); +}); + +test("a tool call becomes a name and a short hint, never its output", () => { + expect(summariseTool("Bash", { command: "bun test", description: "run tests" })) + .toBe("Bash · run tests"); + expect(summariseTool("Read", { file_path: "/srv/project/src/timer.ts" })) + .toBe("Read · timer.ts"); + expect(summariseTool("Write", {})).toBe("Write"); +}); + +test("a tool hint never carries a whole command line", () => { + // The hint is orientation, not a transcript. An unbounded command would put + // arbitrary shell text — and anything interpolated into it — on the wire. + const long = "x".repeat(500); + expect(summariseTool("Bash", { description: long }).length).toBeLessThanOrEqual(80); +}); + +test("clamp truncates to AT MOST max characters, ellipsis included", () => { + // The ellipsis counts. A clamp that returns max+1 makes every caller's cap + // a lie by one character, which is how `summariseTool` would exceed its own. + expect(clamp("abcdef", 3)).toBe("ab…"); + expect(clamp("abcdef", 3).length).toBe(3); + expect(clamp("abc", 10)).toBe("abc"); +}); + +test("toLines renders a turn with a speaker and folds its tools", () => { + const lines = toLines([ + { role: "user", at: "2026-08-20T13:04:00Z", text: "fix the flaky test", tools: [] }, + { role: "assistant", at: "2026-08-20T13:05:00Z", text: "Found it: the timer resets.", tools: ["Bash ×3", "Read timer.ts"] }, + ]); + expect(lines).toEqual([ + "you · 13:04", + "fix the flaky test", + "", + "agent · 13:05", + "▸ Bash ×3 · Read timer.ts", + "Found it: the timer resets.", + "", + ]); +}); + +test("toLines drops a turn left empty by stripping", () => { + // A turn that was only a menu must not leave a bare speaker line behind. + expect(toLines([{ role: "assistant", at: null, text: "", tools: [] }])).toEqual([]); +}); + +test("the text cap is bounded", () => { + expect(MAX_TEXT_CHARS).toBe(4_000); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `bun test tests/journal-text.test.ts` +Expected: FAIL — cannot find module `@server/journal/text`. + +- [ ] **Step 3: Implement** + +Create `src/server/journal/text.ts`: + +```ts +import type { JournalEntry } from "@server/journal/types"; + +/** Ceiling on one turn's prose. Generous for a message, bounded on the wire. */ +export const MAX_TEXT_CHARS = 4_000; + +/** Ceiling on a tool summary line. Orientation, never a transcript. */ +const MAX_TOOL_HINT = 80; + +// eslint-disable-next-line no-control-regex +const ANSI_RE = /\[[0-9;?]*[ -/]*[@-~]|[()][A-Za-z0-9]|./g; + +export function stripAnsi(text: string): string { + return text.replace(ANSI_RE, ""); +} + +/** + * A cursor-marked row, e.g. `❯ 1. Yes`, or a bare numbered option row. + * + * Requires the row to be ONLY the option — anchored both ends, short label — + * so ordinary prose that happens to open with a number survives. Over-stripping + * silently eats real content, which is a worse failure than the one this + * guards. + */ +const MENU_RE = /^\s*(?:❯\s*)?\d{1,2}\.\s+\S[^\n]{0,60}$/; +const CURSOR_ONLY_RE = /^\s*❯\s*\S[^\n]{0,60}$/; + +/** + * Remove an option row from journal text, leaving "" if that is all it was. + * + * WHY: journal lines are blended directly above the live screen with no + * divider (design decision 3). A menu from an already-answered question would + * then read as the live prompt — the failure `prompt-parse.ts` already records + * in its own scoping comment. Only the live screen may show a selectable menu. + */ +export function stripMenu(text: string): string { + if (MENU_RE.test(text) || CURSOR_ONLY_RE.test(text)) return ""; + return text; +} + +/** + * Truncate to AT MOST `max` characters, ellipsis included, so a cut is never + * mistaken for the end and a caller's cap is never off by one. + */ +export function clamp(text: string, max: number): string { + return text.length <= max ? text : text.slice(0, Math.max(0, max - 1)) + "…"; +} + +/** + * One line for a tool call: its name, and a short hint at what it touched. + * + * Never its RESULT. Tool results are where file contents, command output and + * any secret that passed through the agent live, and design decision 4 keeps + * them off the wire entirely. + */ +export function summariseTool(name: string, input: unknown): string { + const obj = (typeof input === "object" && input !== null ? input : {}) as Record; + const raw = + typeof obj.description === "string" ? obj.description + : typeof obj.file_path === "string" ? obj.file_path.split("/").pop() ?? "" + : typeof obj.pattern === "string" ? obj.pattern + : ""; + const hint = stripAnsi(raw).replace(/\s+/g, " ").trim(); + // Clamped on the FINISHED line, not on the hint, so the cap holds whatever + // the tool name's length happens to be. + return clamp(hint === "" ? name : `${name} · ${hint}`, MAX_TOOL_HINT); +} + +/** `13:04` from an ISO stamp, or "" when the record carried none. */ +function hhmm(at: string | null): string { + if (at === null) return ""; + const d = new Date(at); + return Number.isNaN(d.getTime()) + ? "" + : `${String(d.getUTCHours()).padStart(2, "0")}:${String(d.getUTCMinutes()).padStart(2, "0")}`; +} + +/** + * Flatten turns to the lines the terminal renders. + * + * Server-side, because the client must gain no per-harness knowledge — the same + * reason `parsePrompt` lives on this side of the socket. + */ +export function toLines(entries: readonly JournalEntry[]): string[] { + const out: string[] = []; + for (const e of entries) { + const body = clamp(stripMenu(stripAnsi(e.text)).trim(), MAX_TEXT_CHARS); + if (body === "" && e.tools.length === 0) continue; + const who = e.role === "user" ? "you" : "agent"; + const time = hhmm(e.at); + out.push(time === "" ? who : `${who} · ${time}`); + if (e.tools.length > 0) out.push(`▸ ${e.tools.join(" · ")}`); + if (body !== "") out.push(...body.split("\n")); + out.push(""); + } + return out; +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `bun test tests/journal-text.test.ts` +Expected: PASS (10 tests). + +- [ ] **Step 5: Mutation-check the menu guard (house rule 4)** + +Make `stripMenu` `return text;` and re-run. +Expected: the two menu tests go RED. Restore and confirm green. + +- [ ] **Step 6: Commit** + +```bash +make check && make check-clean && make test +git add src/server/journal/text.ts tests/journal-text.test.ts +git commit -m "feat: journal text shaping — prose kept, tool output and menus never served" +``` + +--- + +### Task 6: the Claude adapter + +**Files:** +- Modify: `src/server/journal/claude.ts` (replacing Task 3's stub) +- Create: `tests/fixtures/journal/claude-session.jsonl` +- Test: `tests/journal-claude.test.ts` + +**Interfaces:** +- Consumes: `JournalAdapter`, `JournalEntry` (Task 3); `isSessionId`, `containedRealpath` (Task 4); `summariseTool` (Task 5). +- Produces: `claudeAdapter: JournalAdapter` with a real `locate` and `parse`. + +- [ ] **Step 1: Write the fixture** + +Create `tests/fixtures/journal/claude-session.jsonl` — invented content only, per house rule 2: + +```jsonl +{"type":"user","timestamp":"2026-08-20T13:04:00Z","message":{"role":"user","content":"fix the flaky test"}} +{"type":"assistant","timestamp":"2026-08-20T13:04:30Z","message":{"role":"assistant","content":[{"type":"thinking","thinking":"private reasoning that must not be served"},{"type":"text","text":"Looking at the timer now."},{"type":"tool_use","name":"Bash","input":{"command":"bun test","description":"run tests"}}]}} +{"type":"user","timestamp":"2026-08-20T13:04:31Z","message":{"role":"user","content":[{"type":"tool_result","content":"SECRET_TOKEN=abc123 leaked in output"}]}} +{"type":"assistant","timestamp":"2026-08-20T13:05:00Z","message":{"role":"assistant","content":[{"type":"text","text":"Found it: the timer resets."}]}} +{"type":"assistant","timestamp":"2026-08-20T13:05:10Z","isSidechain":true,"message":{"role":"assistant","content":[{"type":"text","text":"subagent chatter"}]}} +{"type":"mode","timestamp":"2026-08-20T13:05:20Z","mode":"default"} +not valid json at all +{"type":"assistant","timestamp":"2026-08-20T13:06:00Z","message":{"role":"assistant","content":[{"type":"text","text":"❯ 1. Yes"}]}} +``` + +- [ ] **Step 2: Write the failing tests** + +Create `tests/journal-claude.test.ts`: + +```ts +import { expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { claudeAdapter } from "@server/journal/claude"; + +const chunk = readFileSync("tests/fixtures/journal/claude-session.jsonl", "utf8"); +const entries = claudeAdapter.parse(chunk); + +test("a typed user message becomes a user turn", () => { + expect(entries[0]).toEqual({ + role: "user", at: "2026-08-20T13:04:00Z", text: "fix the flaky test", tools: [], + }); +}); + +test("assistant text and its tool call arrive as one turn", () => { + expect(entries[1]!.role).toBe("assistant"); + expect(entries[1]!.text).toBe("Looking at the timer now."); + expect(entries[1]!.tools).toEqual(["Bash · run tests"]); +}); + +test("a tool RESULT is never served", () => { + // This is where file contents, command output and secrets live. Asserted on + // the whole parse, because one leak anywhere is the whole failure. + expect(JSON.stringify(entries)).not.toContain("SECRET_TOKEN"); +}); + +test("a user record whose content is a LIST is not a typed message", () => { + // Folding these is what stops a session rendering hundreds of fabricated + // "you" turns: tool-result traffic is written as role user. + expect(entries.filter((e) => e.role === "user")).toHaveLength(1); +}); + +test("thinking blocks are dropped", () => { + expect(JSON.stringify(entries)).not.toContain("private reasoning"); +}); + +test("subagent traffic is dropped", () => { + expect(JSON.stringify(entries)).not.toContain("subagent chatter"); +}); + +test("bookkeeping records are ignored, not turned into turns", () => { + expect(entries.every((e) => e.text !== "default")).toBe(true); +}); + +test("one unparseable line is skipped without losing the file", () => { + // The record AFTER the broken line must still be present: a private format + // will grow rows this parser has never seen, and one of them must not cost + // the operator their whole history. + expect(entries.at(-1)!.text).toBe("❯ 1. Yes"); +}); + +test("the adapter records the harness version its shape was verified against", () => { + expect(claudeAdapter.verifiedAgainst).not.toBe("unverified"); +}); + +test("locate refuses a value that is not a session id, before touching disk", async () => { + expect(await claudeAdapter.locate("../../etc/passwd", ["/nonexistent"])).toBeNull(); +}); +``` + +- [ ] **Step 3: Run tests to verify they fail** + +Run: `bun test tests/journal-claude.test.ts` +Expected: FAIL — the stub returns `[]`, so `entries[0]` is undefined. + +- [ ] **Step 4: Implement** + +Replace `src/server/journal/claude.ts` entirely: + +```ts +import { readdir } from "node:fs/promises"; +import { join } from "node:path"; +import { containedRealpath, isSessionId } from "@server/journal/files"; +import { summariseTool } from "@server/journal/text"; +import type { JournalAdapter, JournalEntry } from "@server/journal/types"; + +/** + * Claude Code's journal adapter. + * + * WHY THIS EXISTS. A pane running Claude sits on the terminal's ALTERNATE + * SCREEN, which has no scrollback ring, so `pane.read` can never return more + * than the viewport however much is asked for — see + * `docs/design/2026-08-20-journal-history-design.md` for the measurements. The + * history does exist, in Claude Code's own session log, and herdr hands us its + * uuid on `agent_session`. + * + * SHAPE OF THE SOURCE. This is a PRIVATE on-disk format with no compatibility + * promise; it will change without notice. Every unknown record type is ignored + * rather than fatal, and one unparseable line is skipped rather than costing + * the file. `verifiedAgainst` records when the shape was last checked by hand — + * update it whenever you re-check, the way `docs/gotchas.md` treats every other + * measured claim in this repo. + * + * {"type":"user", "message":{"role":"user","content":"…" | [ {type:"tool_result"} ]}} + * {"type":"assistant", "message":{"role":"assistant","content":[ {type:"text"|"thinking"|"tool_use"} ]}} + * + * A `user` record whose content is a LIST is tool-result traffic, not something + * a person typed. `isSidechain` marks subagent traffic. + */ +export const claudeAdapter: JournalAdapter = { + name: "claude", + verifiedAgainst: "Claude Code 2.1.220, checked 2026-08-20", + + async locate(value, roots) { + // Checked before any filesystem call — see files.ts. + if (!isSessionId(value)) return null; + for (const root of roots) { + let projects: string[]; + try { + projects = await readdir(root); + } catch { + continue; // a root that does not exist is not an error, it is a miss + } + for (const project of projects) { + const found = await containedRealpath(root, join(root, project, `${value}.jsonl`)); + if (found !== null) return found; + } + } + return null; + }, + + parse(chunk) { + const out: JournalEntry[] = []; + for (const line of chunk.split("\n")) { + if (line.trim() === "") continue; + let rec: Record; + try { + rec = JSON.parse(line) as Record; + } catch { + // A partial first line is NORMAL: a tail read starts mid-record. A + // genuinely corrupt line costs itself and nothing else. + continue; + } + const entry = toEntry(rec); + if (entry !== null) out.push(entry); + } + return out; + }, +}; + +function toEntry(rec: Record): JournalEntry | null { + const type = rec.type; + if (type !== "user" && type !== "assistant") return null; // bookkeeping rows + if (rec.isSidechain === true) return null; // subagent traffic + + const at = typeof rec.timestamp === "string" ? rec.timestamp : null; + const message = rec.message as { content?: unknown } | undefined; + const content = message?.content; + + if (type === "user") { + // A STRING is a person typing. A LIST is tool-result traffic wearing the + // user role, and rendering those would fabricate hundreds of "you" turns. + if (typeof content !== "string" || content.trim() === "") return null; + return { role: "user", at, text: content, tools: [] }; + } + + if (!Array.isArray(content)) return null; + const texts: string[] = []; + const tools: string[] = []; + for (const part of content) { + const p = part as Record; + if (p.type === "text" && typeof p.text === "string") texts.push(p.text); + else if (p.type === "tool_use" && typeof p.name === "string") { + tools.push(summariseTool(p.name, p.input)); + } + // "thinking" and everything unknown falls through deliberately. + } + if (texts.length === 0 && tools.length === 0) return null; + return { role: "assistant", at, text: texts.join("\n"), tools }; +} +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `bun test tests/journal-claude.test.ts` +Expected: PASS (10 tests). + +- [ ] **Step 6: Mutation-check the exposure guard (house rule 4)** + +In `toEntry`, change the `user` branch to accept a list as well (`if (Array.isArray(content)) return { role: "user", at, text: JSON.stringify(content), tools: [] }`). +Expected: the `SECRET_TOKEN` and "list is not a typed message" tests go RED. Restore and confirm green. + +- [ ] **Step 7: Commit** + +```bash +make check && make check-clean && make test +git add src/server/journal/claude.ts tests/journal-claude.test.ts tests/fixtures/journal +git commit -m "feat: the Claude Code journal adapter" +``` + +--- + +### Task 7: the history route + +**Files:** +- Create: `src/server/journal/read.ts` +- Modify: `src/server/routes.ts` (`AppDeps`, and a new route registered unconditionally) +- Modify: `src/server/index.ts` (wire the journal reader and the `hasJournal` predicate) +- Modify: `src/server/supervisor.ts:257` (pass `hasJournal`, capture `sessionRefs`) +- Test: `tests/journal-route.test.ts` + +**Interfaces:** +- Consumes: `adapterFor` (Task 3), `claudeRoots`/`tailChunk`/`MAX_TAIL_BYTES` (Task 4), `toLines` (Task 5), `claudeAdapter` (Task 6), `sessionRefs` (Task 2). +- Produces: + - `interface JournalReader { read(session: HerdrAgentSession | null | undefined, before: number | null, limit: number): Promise }` + - `interface JournalPage { lines: string[]; source: "journal" | "reconstruction"; hasMore: boolean; cursor: string | null; detail: string | null }` + - `createJournalReader(roots: JournalRoots): JournalReader` + - `AppDeps.journal?: JournalReader` + - `AppDeps.sessionFor?: (agentId: string) => HerdrAgentSession | null` + +- [ ] **Step 1: Write the failing tests** + +Create `tests/journal-route.test.ts`: + +```ts +import { expect, test } from "bun:test"; +import { createApp } from "@server/routes"; +import { AgentStore } from "@server/state/store"; +import { Hub } from "@server/ws/hub"; +import type { Agent } from "@shared/types"; + +const NOW = 1_700_000_000_000; +const health = () => ({ + ok: true, hostId: "dev-box", agents: 1, clients: 0, herdrConnected: true, + lastEventAt: NOW, lastNotifyError: null, version: "0.0.0-dev", latestKnown: null, + herdrProtocol: null, schemaWarning: null, +}); + +function agent(over: Partial = {}): Agent { + return { + hostId: "dev-box", agentId: "w1:p1", name: "docs-cleanup", + task: "Tidy the README", state: "working", workspaceId: "w1", + workspaceLabel: "docs", cwd: "/srv/project", stateSince: NOW, updatedAt: NOW, + acknowledgedAt: null, hasJournal: true, ...over, + }; +} + +function harness(page = { lines: ["you · 13:04", "hi", ""], source: "journal" as const, hasMore: true, cursor: "120", detail: null }) { + const store = new AgentStore("dev-box"); + store.replaceAll([agent()], NOW); + const calls: unknown[] = []; + const app = createApp({ + store, now: () => NOW, health, hub: new Hub({ now: () => NOW }), + sessionFor: () => ({ agent: "claude", kind: "id", source: "herdr:claude", value: "u1" }), + journal: { async read(_s, before, limit) { calls.push({ before, limit }); return page; } }, + }); + return { app, calls }; +} + +const post = (app: ReturnType, body: object) => + app.request("/api/agents/w1:p1/history", { + method: "POST", + headers: { "content-type": "application/json", origin: "http://localhost" }, + body: JSON.stringify(body), + }); + +test("returns lines, provenance and a cursor", async () => { + const { app } = harness(); + const res = await post(app, {}); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.lines).toEqual(["you · 13:04", "hi", ""]); + expect(body.source).toBe("journal"); + expect(body.hasMore).toBe(true); + expect(body.cursor).toBe("120"); +}); + +test("the cursor is passed through as a number", async () => { + const { app, calls } = harness(); + await post(app, { before: "120", limit: 25 }); + expect(calls[0]).toEqual({ before: 120, limit: 25 }); +}); + +test("a non-numeric cursor is refused rather than coerced", async () => { + // The cursor is opaque to the client and MUST be one this server issued. + // Coercing garbage to 0 would silently serve the top of the file instead. + const { app } = harness(); + expect((await post(app, { before: "../etc" })).status).toBe(400); +}); + +test("an unknown agent is 404, not an empty page", async () => { + const { app } = harness(); + const res = await app.request("/api/agents/nope/history", { + method: "POST", + headers: { "content-type": "application/json", origin: "http://localhost" }, + body: "{}", + }); + expect(res.status).toBe(404); +}); + +test("no journal reports reconstruction with a reason, and 200", async () => { + // The UI falls back quietly, so this is a normal answer rather than an error + // — but the reason still travels, because nothing may be swallowed. + const { app } = harness({ + lines: [], source: "reconstruction", hasMore: false, cursor: null, + detail: "no journal adapter for this harness", + }); + const res = await post(app, {}); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.source).toBe("reconstruction"); + expect(body.lines).toEqual([]); + expect(body.detail).toContain("no journal"); +}); + +test("the route exists with no actions dep — it never touches herdr", async () => { + // Registered unconditionally, unlike the action routes. Gating a + // filesystem read on a herdr dependency is the /ack mistake: the one + // feature that works without herdr being the one broken in --demo. + const store = new AgentStore("dev-box"); + store.replaceAll([agent()], NOW); + const app = createApp({ + store, now: () => NOW, health, hub: new Hub({ now: () => NOW }), + sessionFor: () => null, + journal: { async read() { return { lines: [], source: "reconstruction" as const, hasMore: false, cursor: null, detail: "no session" }; } }, + }); + expect((await post(app, {})).status).toBe(200); +}); + +test("the same-origin gate covers it like any other POST", async () => { + const { app } = harness(); + const res = await app.request("/api/agents/w1:p1/history", { + method: "POST", + headers: { "content-type": "application/json", origin: "https://evil.example" }, + body: "{}", + }); + expect(res.status).toBe(403); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `bun test tests/journal-route.test.ts` +Expected: FAIL — `sessionFor`/`journal` are not on `AppDeps`; the route 404s. + +- [ ] **Step 3: Write the reader** + +Create `src/server/journal/read.ts`: + +```ts +import { adapterFor } from "@server/journal/registry"; +import { claudeRoots, MAX_TAIL_BYTES, tailChunk } from "@server/journal/files"; +import { toLines } from "@server/journal/text"; +import type { JournalRoots } from "@server/journal/types"; +import type { HerdrAgentSession } from "@shared/herdr-api"; + +export interface JournalPage { + lines: string[]; + /** + * `"reconstruction"` is the server saying "I have no journal for this + * agent" — it always comes with `lines: []` and a `detail`. Reconstruction + * itself is entirely client-side. This field is a ROUTING answer, not a + * description of the payload. + */ + source: "journal" | "reconstruction"; + hasMore: boolean; + /** Opaque to the client: a byte offset it echoes back, never constructs. */ + cursor: string | null; + detail: string | null; +} + +export interface JournalReader { + read( + session: HerdrAgentSession | null | undefined, + before: number | null, + limit: number, + ): Promise; +} + +const none = (detail: string): JournalPage => ({ + lines: [], source: "reconstruction", hasMore: false, cursor: null, detail, +}); + +export function createJournalReader(roots: JournalRoots): JournalReader { + return { + async read(session, before, limit) { + const adapter = adapterFor(session); + if (adapter === null || !session) return none("no journal adapter for this harness"); + + const path = await adapter.locate(session.value, roots.claude); + if (path === null) return none("session log not found — compacted, rotated or removed"); + + let size: number; + try { + size = Bun.file(path).size; + } catch (err) { + return none(`could not read the session log: ${String(err)}`); + } + + const end = before ?? size; + if (end <= 0) return { lines: [], source: "journal", hasMore: false, cursor: null, detail: null }; + + const { text, startByte } = await tailChunk(path, end, MAX_TAIL_BYTES); + // The first line of a tail read is usually a PARTIAL record. Dropping it + // is correct rather than lossy: the next page, which starts earlier, + // contains it whole. + const usable = startByte > 0 ? text.slice(text.indexOf("\n") + 1) : text; + const entries = adapter.parse(usable).slice(-limit); + return { + lines: toLines(entries), + source: "journal", + hasMore: startByte > 0, + cursor: startByte > 0 ? String(startByte) : null, + detail: null, + }; + }, + }; +} + +/** Roots for the harnesses paddock reads, from the real environment. */ +export function defaultRoots(env: Record, home: string): JournalRoots { + return { claude: claudeRoots(env, home) }; +} +``` + +- [ ] **Step 4: Add the route** + +In `src/server/routes.ts`, add to `AppDeps` after `settings?`: + +```ts + /** Reads a harness's own session log. Omit in tests that do not exercise it. */ + journal?: JournalReader; + /** The server-side session id for an agent. Never crosses the socket. */ + sessionFor?: (agentId: string) => HerdrAgentSession | null; +``` + +Import them: `import type { JournalReader } from "@server/journal/read";` and add `HerdrAgentSession` to the `@shared/herdr-api` import. + +Register the route immediately after the `/api/agents/:id/ack` route — **outside** the `deps.actions` block: + +```ts + /** + * Earlier history from the agent's OWN session log. + * + * Registered unconditionally, like `/ack` and unlike the action routes: + * this reads a file and never touches herdr, so gating it on a herdr + * dependency it does not use would repeat the mistake `/ack`'s comment + * records — the one feature that works without herdr being the one + * visibly broken in `--demo`. + * + * POST, not GET: a cursor in a query string lands in edge access logs. + */ + app.post("/api/agents/:id/history", async (c) => { + const agent = deps.store.snapshot().find((a) => a.agentId === c.req.param("id")); + if (!agent) return c.json({ ok: false, detail: "unknown agent" }, 404); + if (!deps.journal) return c.json({ ok: false, detail: "journal reading is not configured" }, 404); + + const body = await jsonBody(c); + // Refused, never coerced: the cursor is opaque and must be one this + // server issued. Folding garbage to 0 would silently serve the top of + // the file instead of the page the operator asked for. + let before: number | null = null; + if (body.before !== undefined && body.before !== null) { + if (typeof body.before !== "string" || !/^\d+$/.test(body.before)) { + return c.json({ ok: false, detail: "before must be a cursor from a previous response" }, 400); + } + before = Number(body.before); + } + const limit = typeof body.limit === "number" && body.limit > 0 && body.limit <= 200 + ? Math.floor(body.limit) + : 50; + + const page = await deps.journal.read(deps.sessionFor?.(agent.agentId) ?? null, before, limit); + if (page.detail !== null) reportJournalMiss(agent.agentId, page.detail); + return c.json({ ok: true, ...page }); + }); +``` + +Add beside `reportRefusal`: + +```ts +/** + * A journal that could not be read is quiet in the UI and loud here. + * + * The operator sees the old behaviour — falling back to reconstruction is a + * working dashboard, and a banner for a pane that never had a journal would be + * noise. The host does not get to be quiet: `CLAUDE.md` forbids swallowing + * errors, and "history silently stopped going deeper" is otherwise invisible. + * Once per agent, because it is reported on every page request. + */ +const journalMissesSeen = new Set(); + +function reportJournalMiss(agentId: string, detail: string): void { + if (journalMissesSeen.has(agentId)) return; + journalMissesSeen.add(agentId); + warn(`paddock: no journal history for \`${agentId}\` — ${detail}`); +} +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `bun test tests/journal-route.test.ts` +Expected: PASS (7 tests). + +- [ ] **Step 6: Wire it in the composition root** + +In `src/server/supervisor.ts`, capture the refs and inject the predicate. Replace the `toAgents` call at line 257: + +```ts + const agents = toAgents(list.agents ?? [], { + hostId: this.opts.store.hostId, + labels: this.labels, + now, + hasJournal: hasAdapter, + }); + // Server-side only: the ids these hold are filesystem keys and must not + // travel with the agent. Replaced wholesale each reconcile, so a closed + // pane's id does not linger. + this.sessions = sessionRefs(list.agents ?? []); +``` + +Add the field and accessor to the `Supervisor` class: + +```ts + private sessions = new Map(); + + sessionFor(agentId: string): HerdrAgentSession | null { + return this.sessions.get(agentId) ?? null; + } +``` + +Update its imports: `sessionRefs` from `@server/herdr/adapter`, `hasAdapter` from `@server/journal/registry`, and the `HerdrAgentSession` type. + +In `src/server/index.ts`, add to `appDeps`: + +```ts + journal: createJournalReader(defaultRoots(process.env, homedir())), + sessionFor: (id: string) => supervisor?.sessionFor(id) ?? null, +``` + +with `import { homedir } from "node:os";` and `import { createJournalReader, defaultRoots } from "@server/journal/read";`. + +- [ ] **Step 7: Verify against a live herdr** + +Run `paddock` where herdr is running, open an agent, and check the route directly: + +```bash +curl -s -X POST -H 'content-type: application/json' -H 'Origin: http://127.0.0.1:8787' \ + -d '{"limit":5}' http://127.0.0.1:8787/api/agents//history | head -20 +``` + +Expected: `"source":"journal"` and real lines for a Claude pane; `"source":"reconstruction"` with a `detail` for a plain shell pane. Put the observed result in the commit message — house rule 3. + +- [ ] **Step 8: Commit** + +```bash +make check && make check-clean && make test +git add -A +git commit -m "feat: POST /api/agents/:id/history, served from the agent's own log" +``` + +--- + +### Task 8: the terminal view uses it + +**Files:** +- Modify: `src/web/api.ts` (new client call) +- Modify: `src/web/components/AgentTerminal.tsx` (the "Show earlier" handler and its label, around lines 461 and 565) +- Modify: `docs/decisions.md`, `docs/architecture.md`, `docs/gotchas.md`, `docs/roadmap.md` +- Test: `tests/journal-terminal.test.tsx` + +**Interfaces:** +- Consumes: the route from Task 7; `Agent.hasJournal` from Task 2. +- Produces: `fetchHistory(agentId: string, before: string | null, limit?: number): Promise<{ lines: string[]; source: string; hasMore: boolean; cursor: string | null }>` in `src/web/api.ts`. + +- [ ] **Step 1: Write the failing test** + +Create `tests/journal-terminal.test.tsx`, following the existing pattern in `tests/terminal-render.test.tsx` (import `tests/support/dom.ts` first, use `render`/`settle` from `tests/support/render.tsx`, and `stubFetch`): + +```tsx +test("an agent with a journal fetches earlier lines instead of reading the cache", async () => { + const fetched: string[] = []; + stubFetch((url) => { + fetched.push(url); + return { ok: true, lines: ["you · 13:04", "fix the flaky test", ""], source: "journal", hasMore: false, cursor: null, detail: null }; + }); + const el = render( {}} />); + await settle(); + click(el.querySelector(".term-earlier")!); + await settle(); + expect(fetched.some((u) => u.endsWith("/history"))).toBe(true); + expect(el.textContent).toContain("fix the flaky test"); +}); + +test("an agent with no journal never calls the route", async () => { + // Nothing regresses for a plain shell pane: it keeps the client-side + // reconstruction it has today. + const fetched: string[] = []; + stubFetch((url) => { fetched.push(url); return { ok: true, lines: [], source: "visible" }; }); + const el = render( {}} />); + await settle(); + const earlier = el.querySelector(".term-earlier"); + if (earlier) click(earlier); + await settle(); + expect(fetched.some((u) => u.endsWith("/history"))).toBe(false); +}); + +test("a journal line carrying a menu cannot render as a live option", async () => { + // Belt and braces over the server's stripMenu: the blend has no divider, so + // a stale "❯ 1. Yes" above the live screen would read as the live prompt. + stubFetch(() => ({ ok: true, lines: ["agent · 13:06", "❯ 1. Yes", ""], source: "journal", hasMore: false, cursor: null, detail: null })); + const el = render( {}} />); + await settle(); + click(el.querySelector(".term-earlier")!); + await settle(); + expect(el.querySelectorAll("button.term-option")).toHaveLength(0); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `bun test tests/journal-terminal.test.tsx` +Expected: FAIL — the component never requests `/history`. + +- [ ] **Step 3: Add the client call** + +In `src/web/api.ts`, beside the other POST helpers: + +```ts +/** + * Earlier history from the agent's own session log. + * + * `before` is OPAQUE — echo back what the last response gave and never build + * one. `source` says which history the server actually had; `"reconstruction"` + * means "use your local one", and arrives with no lines. + */ +export async function fetchHistory( + agentId: string, + before: string | null, + limit = 50, +): Promise<{ lines: string[]; source: string; hasMore: boolean; cursor: string | null }> { + const res = await post(`/api/agents/${encodeURIComponent(agentId)}/history`, { before, limit }); + return res as { lines: string[]; source: string; hasMore: boolean; cursor: string | null }; +} +``` + +Match the file's existing `post` helper and error handling rather than inventing a second style. + +- [ ] **Step 4: Use it in the terminal** + +In `AgentTerminal.tsx`, add state beside `shownHistory`: + +```tsx + // Journal-sourced lines, oldest first, and the cursor for the next page. + // Kept separate from `history.settled` because the two sources never mix for + // one agent (design decision 2) — this is which one is in play, not a merge. + const [journalLines, setJournalLines] = useState([]); + const [journalCursor, setJournalCursor] = useState(null); + const [journalDone, setJournalDone] = useState(false); +``` + +Replace the `revealed` computation: + +```tsx + const revealed = agent.hasJournal + ? journalLines + : shownHistory > 0 + ? history.settled.slice(Math.max(0, history.settled.length - shownHistory)) + : []; +``` + +Replace the "Show earlier" button's condition and handler: + +```tsx + {!error && (agent.hasJournal ? !journalDone : history.settled.length > revealed.length) && ( + + )} +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `bun test tests/journal-terminal.test.tsx` +Expected: PASS (3 tests). + +- [ ] **Step 6: Write the documentation** + +`docs/decisions.md` — append decision 18 covering the six decisions in the design doc's "Decisions" section, in the same voice as decisions 12 and 17: what was chosen, what was rejected, and why. State plainly that journal lines are prose and will not look like the live screen, and that menus are stripped because a stale prompt blended above the screen would read as the live one. + +`docs/architecture.md` — add `journal/` to the dependency description as a new axis beside `herdr/`, noting it knows harnesses rather than herdr and that `adapter.ts` receives its predicate by injection rather than importing it. + +`docs/gotchas.md` — add, under the herdr section: + +```markdown +- **A coding agent's pane has no scrollback to read, at any price.** It runs on + the terminal's alternate screen, which keeps nothing behind the viewport: + every such pane reports `scroll.max_offset_from_bottom: 0`. Measured against + herdr 0.8.0, asking anyway costs ~35 ms per line past the viewport — 300 lines + 10.7 s (past `HERDR_TIMEOUT_MS`), and 500/1000/2000 lines each ~15.8 s while + returning LESS than `visible` returns in 2 ms. The bytes were never retained. + This is why history comes from the harness's own log — see + `src/server/journal/`. +``` + +`docs/roadmap.md` — mark the `MAX_READ_LINES` entry resolved by this work, in the same struck-through-with-explanation style the file already uses. Do not delete it. + +- [ ] **Step 7: Verify on a real device** + +Run `paddock`, open it on a phone, open a Claude agent, tap **Show earlier** twice. +Expected: earlier turns appear, prepended, without the scroll position jumping; no option buttons render for journal content. Record what you saw in the commit message. + +- [ ] **Step 8: Commit** + +```bash +make check && make check-clean && make test +git add -A +git commit -m "feat: show earlier reads the agent's own log" +``` + +--- + +### Task 9: `--demo` can demonstrate it + +`README.md` screenshots come from `--demo`, and `docs/roadmap.md` already carries one feature invisible there. Adding a second silently is a choice, not an accident. + +**Files:** +- Modify: `src/web/demo/backend.ts` +- Test: `tests/demo.test.ts` + +**Interfaces:** +- Consumes: `Agent.hasJournal` (Task 2), the `/history` response shape (Task 7). +- Produces: nothing new. + +- [ ] **Step 1: Write the failing test** + +Add to `tests/demo.test.ts`: + +```ts +test("one demo agent has a journal, so --demo can demonstrate Show earlier", () => { + // README screenshots come from --demo. A feature invisible there cannot be + // screenshotted, and the roadmap already records one such gap. + const agents = demoAgents(); + expect(agents.filter((a) => a.hasJournal).length).toBeGreaterThan(0); +}); + +test("the demo journal uses invented content", () => { + // House rule 2. Fixtures and demo data never carry real agent names. + const lines = demoHistory("d1:p1").lines; + expect(lines.join("\n")).toContain("flaky-test-fix"); + expect(lines.length).toBeGreaterThan(3); +}); +``` + +Adjust the imported helper names to whatever `src/web/demo/backend.ts` actually exports; do not invent a second demo API. + +- [ ] **Step 2: Run test to verify it fails** + +Run: `bun test tests/demo.test.ts` +Expected: FAIL — no demo agent has `hasJournal`, and `demoHistory` does not exist. + +- [ ] **Step 3: Implement** + +In `src/web/demo/backend.ts`, set `hasJournal: true` on exactly one seeded agent, and add a canned history response for it — invented content only, in the shape `toLines` produces: + +```ts +/** + * A short canned transcript for the one demo agent that has a journal. + * + * Invented content, per house rule 2 — never copied from a real session. It + * exists so `Show earlier` is demonstrable in the mode README screenshots come + * from, rather than being a feature only a live herdr can show. + */ +const DEMO_HISTORY: string[] = [ + "you · 13:04", + "the flaky-test-fix run keeps timing out — take a look", + "", + "agent · 13:05", + "▸ Bash · run the suite", + "Reproduced it: the retry budget is exhausted before the first assertion.", + "", +]; +``` + +Serve it from the demo backend's `/history` branch with `source: "journal"`, `hasMore: false`, `cursor: null`, `detail: null`. + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `bun test tests/demo.test.ts` +Expected: PASS. + +- [ ] **Step 5: See it** + +Run: `bun run build:web && bun src/server/index.ts --demo`, open `http://127.0.0.1:8787`, open the demo agent, tap **Show earlier**. +Expected: the canned turns appear above the screen. + +- [ ] **Step 6: Commit** + +```bash +make check && make check-clean && make test +git add -A +git commit -m "feat: --demo can demonstrate journal history" +``` + +--- + +## Self-Review + +**Spec coverage.** Every section of `docs/design/2026-08-20-journal-history-design.md` maps to a task: the `agent_session` type gap → Task 1; decision 5 (session ids off the wire) → Task 2; the registry seam → Task 3; path safety and the bounded reader → Task 4; decision 4 (exposure) and decision 3's menu stripping → Tasks 5 and 6; the route, its cursor rules and decision 6's logging → Task 7; decisions 1–3 in the client, plus all four docs → Task 8; demo mode → Task 9. The "risk worth stating" is carried by `verifiedAgainst` (Tasks 3 and 6) and asserted by a test. + +**Known gaps this plan deliberately leaves.** Codex, pi and OpenCode adapters are out of scope — the registry is the seam, and adding one is an entry plus a module. Paging beyond `MAX_TAIL_BYTES` per request works by repeated requests rather than a streaming read. + +**One thing an implementer must not "fix".** `source: "reconstruction"` comes back with `lines: []` and a `200`. That is correct: the server is answering "I have no journal", and the client already holds the reconstruction. Turning it into a `404` or an error would make a working fallback look like a failure. From 21283d3a4b9af2f50b20e72fd9fe954e1d5dab3d Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 20:57:46 +0700 Subject: [PATCH 04/22] feat: generate agent_session, the key a journal lookup needs the generator now emits HerdrAgentSession (recording the harness session associated with a pane) and adds the optional field to HerdrAgentRaw. herdr has sent this field since 0.8.2; without it in the declared shape, src/server/journal/ has no way to find the harness's session log. the schema-drift tests move agent_session from ignored to declared. Co-Authored-By: Claude Opus 5 --- scripts/gen-herdr-types.ts | 16 ++++++++++++++++ src/shared/herdr-api.d.ts | 16 ++++++++++++++++ tests/herdr-schema-drift.test.ts | 2 +- tests/herdr-types-guard.test.ts | 10 ++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/scripts/gen-herdr-types.ts b/scripts/gen-herdr-types.ts index 8bc036e..8a20eb7 100644 --- a/scripts/gen-herdr-types.ts +++ b/scripts/gen-herdr-types.ts @@ -59,10 +59,26 @@ export type HerdrReadSource = ${readSources.map((s) => `"${s}"`).join(" | ")}; export type HerdrReadFormat = ${readFormats.map((s) => `"${s}"`).join(" | ")}; +/** + * The harness session herdr has associated with a pane, or null. + * + * \`kind\` is "id" for a session identifier and \`value\` is that id; + * \`agent\` names the harness ("claude", "codex"). This is the key + * \`src/server/journal/\` uses to find the harness's own log, and it is why + * that feature needs no second herdr call: it rides on \`agent.list\`. + */ +export interface HerdrAgentSession { + agent: string; + kind: string; + source: string; + value: string; +} + /** One entry from \`agent.list\` -> result.agents[]. */ export interface HerdrAgentRaw { agent?: string | null; agent_status: HerdrAgentStatus; + agent_session?: HerdrAgentSession | null; cwd: string; foreground_cwd?: string; focused: boolean; diff --git a/src/shared/herdr-api.d.ts b/src/shared/herdr-api.d.ts index a7c0a49..1861625 100644 --- a/src/shared/herdr-api.d.ts +++ b/src/shared/herdr-api.d.ts @@ -22,10 +22,26 @@ export type HerdrReadSource = "visible" | "recent" | "recent_unwrapped" | "detec export type HerdrReadFormat = "text" | "ansi"; +/** + * The harness session herdr has associated with a pane, or null. + * + * `kind` is "id" for a session identifier and `value` is that id; + * `agent` names the harness ("claude", "codex"). This is the key + * `src/server/journal/` uses to find the harness's own log, and it is why + * that feature needs no second herdr call: it rides on `agent.list`. + */ +export interface HerdrAgentSession { + agent: string; + kind: string; + source: string; + value: string; +} + /** One entry from `agent.list` -> result.agents[]. */ export interface HerdrAgentRaw { agent?: string | null; agent_status: HerdrAgentStatus; + agent_session?: HerdrAgentSession | null; cwd: string; foreground_cwd?: string; focused: boolean; diff --git a/tests/herdr-schema-drift.test.ts b/tests/herdr-schema-drift.test.ts index 0472e3e..5a06e7c 100644 --- a/tests/herdr-schema-drift.test.ts +++ b/tests/herdr-schema-drift.test.ts @@ -27,6 +27,7 @@ import type { const DECLARED_FIELD_FLAGS = { agent: true, agent_status: true, + agent_session: true, cwd: true, foreground_cwd: true, focused: true, @@ -47,7 +48,6 @@ const DECLARED_FIELDS = Object.keys(DECLARED_FIELD_FLAGS) as (keyof HerdrAgentRa // does not model (as of protocol 19). Named explicitly so a new upstream // field shows up here as a decision to make, not a silently ignored column. const IGNORED_FIELDS = [ - "agent_session", "display_agent", "interactive_ready", "launch_pending", diff --git a/tests/herdr-types-guard.test.ts b/tests/herdr-types-guard.test.ts index 69d5fe4..cf92724 100644 --- a/tests/herdr-types-guard.test.ts +++ b/tests/herdr-types-guard.test.ts @@ -78,3 +78,13 @@ test("gen-herdr-types refuses an older herdr and leaves the contract untouched", expect(r.stderr.toString()).toContain("older"); expect(readFileSync(out, "utf8")).toBe(before); }); + +test("HerdrAgentRaw declares agent_session, the key journal history needs", () => { + // A generated file, so this asserts the GENERATOR emitted it. herdr 0.8.2 + // sends agent_session on every agent.list row; without it in the declared + // shape, journal/ has no session id to look up and the feature is dead at + // the type level rather than at runtime. + const src = readFileSync("src/shared/herdr-api.d.ts", "utf8"); + expect(src).toContain("export interface HerdrAgentSession"); + expect(src).toMatch(/agent_session\?: HerdrAgentSession \| null;/); +}); From 205d691f2381dc90bf5d724c3ca79b81c095c00b Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 21:06:11 +0700 Subject: [PATCH 05/22] feat: hasJournal on the wire, session ids off it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The terminal view needs to know WHICH history source to use for a given agent, without ever receiving the session id that source is keyed by — a session id is a filesystem key, and paddock does not hand filesystem keys to the browser. Agent.hasJournal is a required boolean derived in the adapter via an INJECTED predicate (AdaptContext.hasJournal), not a direct import of journal/: adapter.ts sits on the herdr axis, journal/ sits on the harness axis, and importing across them would fuse the two permanently. Defaults to false, which is exactly "paddock reads no journals" until a later task wires in the real predicate. sessionRefs() is a separate, server-only export (pane_id -> session) so the session id never has a path onto the wire type by construction. Making hasJournal required (not optional) surfaced every place an Agent literal is built without it, via tsc — 18 files, all fixed with hasJournal: false except the new adapter tests exercising the predicate itself. Co-Authored-By: Claude Opus 5 --- src/server/demo.ts | 1 + src/server/herdr/adapter.ts | 24 +++++++++++++++++++- src/shared/types.ts | 14 ++++++++++++ src/web/demo/backend.ts | 1 + tests/acknowledge-ui.test.ts | 2 +- tests/acknowledge.test.ts | 2 +- tests/action-routes.test.ts | 2 +- tests/adapter.test.ts | 40 ++++++++++++++++++++++++++++++++- tests/grouping.test.ts | 2 +- tests/hub.test.ts | 2 +- tests/notifier-inflight.test.ts | 2 +- tests/notifier-settle.test.ts | 2 +- tests/notifier-timing.test.ts | 2 +- tests/notifier.test.ts | 2 +- tests/notify-wiring.test.ts | 1 + tests/origin-gate.test.ts | 2 +- tests/origin-tunnel.test.ts | 2 +- tests/store.test.ts | 1 + tests/support/render.tsx | 2 +- tests/tunnel-public-url.test.ts | 2 +- tests/web-store.test.ts | 2 +- 21 files changed, 94 insertions(+), 16 deletions(-) diff --git a/src/server/demo.ts b/src/server/demo.ts index 87e3430..5900c08 100644 --- a/src/server/demo.ts +++ b/src/server/demo.ts @@ -28,6 +28,7 @@ export function demoAgents(now: number): Agent[] { stateSince: now - s.ageMs, updatedAt: now, acknowledgedAt: null, + hasJournal: false, })); } diff --git a/src/server/herdr/adapter.ts b/src/server/herdr/adapter.ts index 7aaed16..b894728 100644 --- a/src/server/herdr/adapter.ts +++ b/src/server/herdr/adapter.ts @@ -1,10 +1,17 @@ import { carryAcknowledged, type Agent, type AgentState } from "@shared/types"; -import type { HerdrAgentRaw, HerdrStatusChanged, HerdrWorkspaceRaw } from "@shared/herdr-api"; +import type { HerdrAgentRaw, HerdrAgentSession, HerdrStatusChanged, HerdrWorkspaceRaw } from "@shared/herdr-api"; export interface AdaptContext { hostId: string; labels: Map; now: number; + /** + * Whether a journal adapter exists for this session. INJECTED rather than + * imported: `journal/` is a harness-axis module and this file is the herdr + * adapter, so importing it here would cross the two axes permanently. + * Defaults to false, which is exactly "paddock reads no journals". + */ + hasJournal?: (session: HerdrAgentSession | null | undefined) => boolean; } /** Leading status glyphs some agents prepend to the terminal title. */ @@ -51,6 +58,7 @@ export function toAgent(rawAgent: HerdrAgentRaw, ctx: AdaptContext): Agent | nul stateSince: ctx.now, updatedAt: ctx.now, acknowledgedAt: null, + hasJournal: ctx.hasJournal?.(rawAgent.agent_session) ?? false, }; } @@ -180,3 +188,17 @@ function baseName(cwd: string): string | null { function paneSuffix(paneId: string): string { return paneId.slice(paneId.lastIndexOf(":") + 1); } + +/** + * Session ids by pane id, for the server side only. + * + * Separate from `toAgents` because the result must NOT travel with the agent: + * `Agent` crosses the socket to the browser and this does not. + */ +export function sessionRefs(rows: HerdrAgentRaw[]): Map { + const out = new Map(); + for (const row of rows) { + if (row.agent_session) out.set(row.pane_id, row.agent_session); + } + return out; +} diff --git a/src/shared/types.ts b/src/shared/types.ts index 12178e1..9f7c270 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -74,6 +74,20 @@ export interface Agent { * herdr's `done` stays true, paddock just stops surfacing it. */ acknowledgedAt: number | null; + /** + * Whether paddock can read this agent's own session log, which decides + * WHICH history source the terminal view uses (see + * `docs/design/2026-08-20-journal-history-design.md`). + * + * A boolean and nothing more, deliberately. The session id it is derived + * from is a filesystem key that stays on the server: the UI's only question + * is "fetch, or use my local reconstruction?". + * + * Required, not optional — an optional field lets a future edit drop it + * silently, and the terminal would fall back to reconstruction for every + * agent with nothing to notice. + */ + hasJournal: boolean; } /** diff --git a/src/web/demo/backend.ts b/src/web/demo/backend.ts index e61371e..e8385d7 100644 --- a/src/web/demo/backend.ts +++ b/src/web/demo/backend.ts @@ -43,6 +43,7 @@ const agents: Agent[] = SEED.map((s) => ({ stateSince: Date.now() - s.ageMs, updatedAt: Date.now(), acknowledgedAt: null, + hasJournal: false, })); /** Cursor position on the blocked agent's menu, moved by the arrow keys. */ diff --git a/tests/acknowledge-ui.test.ts b/tests/acknowledge-ui.test.ts index 57bdfef..617d0b7 100644 --- a/tests/acknowledge-ui.test.ts +++ b/tests/acknowledge-ui.test.ts @@ -5,7 +5,7 @@ import type { Agent } from "@shared/types"; const base: Agent = { hostId: "dev-box", agentId: "w1:p1", name: "docs-cleanup", task: "Tidy the README", state: "done", workspaceId: "w1", workspaceLabel: null, cwd: "/srv/project", - stateSince: 1, updatedAt: 1, acknowledgedAt: null, + stateSince: 1, updatedAt: 1, acknowledgedAt: null, hasJournal: false, }; test("offered on a fresh done agent", () => { diff --git a/tests/acknowledge.test.ts b/tests/acknowledge.test.ts index 7376944..23749d1 100644 --- a/tests/acknowledge.test.ts +++ b/tests/acknowledge.test.ts @@ -10,7 +10,7 @@ function agent(over: Partial = {}): Agent { hostId: "dev-box", agentId: "w1:p1", name: "api-refactor", task: "Extract auth middleware", state: "done", workspaceId: "w1", workspaceLabel: "api work", cwd: "/srv/project", - stateSince: NOW, updatedAt: NOW, acknowledgedAt: null, ...over, + stateSince: NOW, updatedAt: NOW, acknowledgedAt: null, hasJournal: false, ...over, }; } diff --git a/tests/action-routes.test.ts b/tests/action-routes.test.ts index e92b840..70aa7a7 100644 --- a/tests/action-routes.test.ts +++ b/tests/action-routes.test.ts @@ -12,7 +12,7 @@ function agent(over: Partial = {}): Agent { hostId: "dev-box", agentId: "w1:p1", name: "api-refactor", task: "Extract auth middleware", state: "blocked", workspaceId: "w1", workspaceLabel: "api work", cwd: "/srv/project", - stateSince: NOW, updatedAt: NOW, acknowledgedAt: null, ...over, + stateSince: NOW, updatedAt: NOW, acknowledgedAt: null, hasJournal: false, ...over, }; } diff --git a/tests/adapter.test.ts b/tests/adapter.test.ts index 4bb5c2f..a99305e 100644 --- a/tests/adapter.test.ts +++ b/tests/adapter.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test"; -import { applyStatusEvent, toAgent, toAgents } from "@server/herdr/adapter"; +import { applyStatusEvent, sessionRefs, toAgent, toAgents } from "@server/herdr/adapter"; import type { HerdrAgentRaw } from "@shared/herdr-api"; const NOW = 1_700_000_000_000; @@ -172,3 +172,41 @@ test("rows that are not agents are dropped before labelling", () => { const agents = toAgents([raw({ agent: null }), raw({ agent_status: "unknown" }), raw()], ctx); expect(agents).toHaveLength(1); }); + +// --- hasJournal: injected predicate, session ids stay off the wire --------- + +test("hasJournal is false when herdr sends no session", () => { + const [a] = toAgents([raw({ pane_id: "w1:p1", name: "api-refactor" })], ctx); + expect(a!.hasJournal).toBe(false); +}); + +test("hasJournal asks the injected predicate, never the harness name directly", () => { + // Injected, because `adapter.ts` sits on the herdr axis and `journal/` sits + // on the harness axis. A direct import would tie the two together and put + // harness knowledge in the herdr adapter. + const session = { agent: "claude", kind: "id", source: "herdr:claude", value: "u" }; + const [a] = toAgents([raw({ pane_id: "w1:p1", agent_session: session })], { + ...ctx, + hasJournal: (s) => s?.agent === "claude", + }); + expect(a!.hasJournal).toBe(true); +}); + +test("sessionRefs keys by pane id and drops rows with no session", () => { + const session = { agent: "claude", kind: "id", source: "herdr:claude", value: "u1" }; + const refs = sessionRefs([ + raw({ pane_id: "w1:p1", agent_session: session }), + raw({ pane_id: "w1:p2" }), + ]); + expect(refs.get("w1:p1")).toEqual(session); + expect(refs.has("w1:p2")).toBe(false); +}); + +test("the session id is NOT on the wire type", () => { + // A session id is a filesystem key. The browser cannot need one, and paddock + // does not hand filesystem keys to clients. Asserted on the serialized shape + // because that is what actually crosses the socket. + const session = { agent: "claude", kind: "id", source: "herdr:claude", value: "secret-uuid" }; + const [a] = toAgents([raw({ pane_id: "w1:p1", agent_session: session })], ctx); + expect(JSON.stringify(a)).not.toContain("secret-uuid"); +}); diff --git a/tests/grouping.test.ts b/tests/grouping.test.ts index 66a42c7..cf353f7 100644 --- a/tests/grouping.test.ts +++ b/tests/grouping.test.ts @@ -9,7 +9,7 @@ function agent(name: string, state: Agent["state"], stateSince = NOW): Agent { return { hostId: "dev-box", agentId: name, name, task: `task for ${name}`, state, workspaceId: "w1", workspaceLabel: null, cwd: "/srv/project", - stateSince, updatedAt: stateSince, acknowledgedAt: null, + stateSince, updatedAt: stateSince, acknowledgedAt: null, hasJournal: false, }; } diff --git a/tests/hub.test.ts b/tests/hub.test.ts index acff238..665d909 100644 --- a/tests/hub.test.ts +++ b/tests/hub.test.ts @@ -9,7 +9,7 @@ function agent(over: Partial = {}): Agent { hostId: "dev-box", agentId: "w1:p1", name: "api-refactor", task: "Extract auth middleware", state: "working", workspaceId: "w1", workspaceLabel: null, cwd: "/srv/project", stateSince: NOW, updatedAt: NOW, - acknowledgedAt: null, ...over, + acknowledgedAt: null, hasJournal: false, ...over, }; } diff --git a/tests/notifier-inflight.test.ts b/tests/notifier-inflight.test.ts index a9bf48e..5752ced 100644 --- a/tests/notifier-inflight.test.ts +++ b/tests/notifier-inflight.test.ts @@ -8,7 +8,7 @@ const agent = (over: Partial = {}): Agent => ({ hostId: "dev-box", agentId: "w1:p1", name: "schema-migration", task: "Backfilling the index", state: "working", workspaceId: "w1", workspaceLabel: null, cwd: "/srv/project", stateSince: NOW, - updatedAt: NOW, acknowledgedAt: null, ...over, + updatedAt: NOW, acknowledgedAt: null, hasJournal: false, ...over, }); /** diff --git a/tests/notifier-settle.test.ts b/tests/notifier-settle.test.ts index deffae9..32e8081 100644 --- a/tests/notifier-settle.test.ts +++ b/tests/notifier-settle.test.ts @@ -8,7 +8,7 @@ const agent = (over: Partial = {}): Agent => ({ hostId: "dev-box", agentId: "w1:p1", name: "api-refactor", task: "Extract auth middleware", state: "working", workspaceId: "w1", workspaceLabel: null, cwd: "/srv/project", stateSince: NOW, - updatedAt: NOW, acknowledgedAt: null, ...over, + updatedAt: NOW, acknowledgedAt: null, hasJournal: false, ...over, }); /** diff --git a/tests/notifier-timing.test.ts b/tests/notifier-timing.test.ts index ff1244d..887b6c3 100644 --- a/tests/notifier-timing.test.ts +++ b/tests/notifier-timing.test.ts @@ -8,7 +8,7 @@ const agent = (over: Partial = {}): Agent => ({ hostId: "dev-box", agentId: "w1:p1", name: "flaky-test-fix", task: "Re-running the suite", state: "working", workspaceId: "w1", workspaceLabel: null, cwd: "/srv/project", stateSince: NOW, - updatedAt: NOW, acknowledgedAt: null, ...over, + updatedAt: NOW, acknowledgedAt: null, hasJournal: false, ...over, }); function harness(o: { mutedUntil?: number | null; cooldownMs?: number; failWith?: string } = {}) { diff --git a/tests/notifier.test.ts b/tests/notifier.test.ts index 1d9b318..d0c7961 100644 --- a/tests/notifier.test.ts +++ b/tests/notifier.test.ts @@ -7,7 +7,7 @@ const agent = (over: Partial = {}): Agent => ({ hostId: "dev-box", agentId: "w1:p1", name: "api-refactor", task: "Extract auth middleware", state: "working", workspaceId: "w1", workspaceLabel: null, cwd: "/srv/project", stateSince: NOW, - updatedAt: NOW, acknowledgedAt: null, ...over, + updatedAt: NOW, acknowledgedAt: null, hasJournal: false, ...over, }); interface HarnessOpts { diff --git a/tests/notify-wiring.test.ts b/tests/notify-wiring.test.ts index 1ba2777..45dc60c 100644 --- a/tests/notify-wiring.test.ts +++ b/tests/notify-wiring.test.ts @@ -15,6 +15,7 @@ const agent = (state: Agent["state"]): Agent => ({ stateSince: 0, updatedAt: 0, acknowledgedAt: null, + hasJournal: false, }); // The regression this guards: wiring the notifier by REPLACING diff --git a/tests/origin-gate.test.ts b/tests/origin-gate.test.ts index 7950373..3c79c0c 100644 --- a/tests/origin-gate.test.ts +++ b/tests/origin-gate.test.ts @@ -30,7 +30,7 @@ function agent(state: Agent["state"] = "blocked"): Agent { hostId: "dev-box", agentId: "w1:p1", name: "api-refactor", task: "Extract auth middleware", state, workspaceId: "w1", workspaceLabel: "api work", cwd: "/srv/project", - stateSince: NOW, updatedAt: NOW, acknowledgedAt: null, + stateSince: NOW, updatedAt: NOW, acknowledgedAt: null, hasJournal: false, }; } diff --git a/tests/origin-tunnel.test.ts b/tests/origin-tunnel.test.ts index 122c2b6..7219fb4 100644 --- a/tests/origin-tunnel.test.ts +++ b/tests/origin-tunnel.test.ts @@ -37,7 +37,7 @@ function agent(): Agent { hostId: "dev-box", agentId: "w1:p1", name: "docs-cleanup", task: "Tidy the README", state: "done", workspaceId: "w1", workspaceLabel: "docs", cwd: "/srv/project", - stateSince: NOW, updatedAt: NOW, acknowledgedAt: null, + stateSince: NOW, updatedAt: NOW, acknowledgedAt: null, hasJournal: false, }; } diff --git a/tests/store.test.ts b/tests/store.test.ts index 2bb9df0..8922753 100644 --- a/tests/store.test.ts +++ b/tests/store.test.ts @@ -17,6 +17,7 @@ function agent(over: Partial = {}): Agent { stateSince: NOW, updatedAt: NOW, acknowledgedAt: null, + hasJournal: false, ...over, }; } diff --git a/tests/support/render.tsx b/tests/support/render.tsx index b845a2f..cfbdc8a 100644 --- a/tests/support/render.tsx +++ b/tests/support/render.tsx @@ -39,7 +39,7 @@ export function agent(over: Partial = {}): Agent { hostId: "dev-box", agentId: "w1:p1", name: "api-refactor", task: "Extract auth middleware", state: "working", workspaceId: "w1", workspaceLabel: "api work", cwd: "/srv/project", - stateSince: 0, updatedAt: 0, acknowledgedAt: null, ...over, + stateSince: 0, updatedAt: 0, acknowledgedAt: null, hasJournal: false, ...over, }; } diff --git a/tests/tunnel-public-url.test.ts b/tests/tunnel-public-url.test.ts index e7bb0eb..4be62f2 100644 --- a/tests/tunnel-public-url.test.ts +++ b/tests/tunnel-public-url.test.ts @@ -14,7 +14,7 @@ const agent = (over: Partial = {}): Agent => ({ hostId: "dev-box", agentId: "w1:p1", name: "api-refactor", task: "Extract auth middleware", state: "working", workspaceId: "w1", workspaceLabel: "api work", cwd: "/srv/project", - stateSince: NOW, updatedAt: NOW, acknowledgedAt: null, ...over, + stateSince: NOW, updatedAt: NOW, acknowledgedAt: null, hasJournal: false, ...over, }); test("the tunnel URL is used for deeplinks and never saved", async () => { diff --git a/tests/web-store.test.ts b/tests/web-store.test.ts index c63c5f8..fef02df 100644 --- a/tests/web-store.test.ts +++ b/tests/web-store.test.ts @@ -11,7 +11,7 @@ function agent(over: Partial = {}): Agent { hostId: "dev-box", agentId: "w1:p1", name: "api-refactor", task: "Extract auth middleware", state: "working", workspaceId: "w1", workspaceLabel: null, cwd: "/srv/project", stateSince: NOW, updatedAt: NOW, - acknowledgedAt: null, ...over, + acknowledgedAt: null, hasJournal: false, ...over, }; } From 98860869d79438d5313109dc5a64e4e4e4d81e17 Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 21:10:59 +0700 Subject: [PATCH 06/22] =?UTF-8?q?feat:=20the=20journal=20registry=20?= =?UTF-8?q?=E2=80=94=20one=20decision=20site=20for=20readable=20history?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The journal module is the entry point for reading a coding agent's own session log. This commit establishes the types, registry logic, and the adapter pattern that routes each harness to its corresponding adapter. The registry is the single decision site: adding a harness requires one entry in ADAPTERS plus its adapter module — never a new condition in the route layer or client. Stub the Claude adapter with Task 6 to complete the implementation. Co-Authored-By: Claude Opus 5 --- src/server/journal/claude.ts | 8 +++++++ src/server/journal/registry.ts | 23 ++++++++++++++++++++ src/server/journal/types.ts | 39 ++++++++++++++++++++++++++++++++++ tests/journal-registry.test.ts | 27 +++++++++++++++++++++++ 4 files changed, 97 insertions(+) create mode 100644 src/server/journal/claude.ts create mode 100644 src/server/journal/registry.ts create mode 100644 src/server/journal/types.ts create mode 100644 tests/journal-registry.test.ts diff --git a/src/server/journal/claude.ts b/src/server/journal/claude.ts new file mode 100644 index 0000000..2ec77b3 --- /dev/null +++ b/src/server/journal/claude.ts @@ -0,0 +1,8 @@ +import type { JournalAdapter } from "@server/journal/types"; + +export const claudeAdapter: JournalAdapter = { + name: "claude", + verifiedAgainst: "unverified", + async locate() { return null; }, + parse() { return []; }, +}; diff --git a/src/server/journal/registry.ts b/src/server/journal/registry.ts new file mode 100644 index 0000000..1dedc45 --- /dev/null +++ b/src/server/journal/registry.ts @@ -0,0 +1,23 @@ +import { claudeAdapter } from "@server/journal/claude"; +import type { HerdrAgentSession } from "@shared/herdr-api"; +import type { JournalAdapter } from "@server/journal/types"; + +/** + * The SINGLE decision site for "does this agent have a readable history". + * + * Adding a harness is one entry here plus its adapter module — never a new + * branch in the route and never a condition in the client. + */ +const ADAPTERS: readonly JournalAdapter[] = [claudeAdapter]; + +export function adapterFor(session: HerdrAgentSession | null | undefined): JournalAdapter | null { + if (!session) return null; + // Only an id can become a path. Any other `kind` is a value this code has no + // way to resolve, and guessing is how a lookup becomes a traversal. + if (session.kind !== "id") return null; + return ADAPTERS.find((a) => a.name === session.agent) ?? null; +} + +export function hasAdapter(session: HerdrAgentSession | null | undefined): boolean { + return adapterFor(session) !== null; +} diff --git a/src/server/journal/types.ts b/src/server/journal/types.ts new file mode 100644 index 0000000..44439d1 --- /dev/null +++ b/src/server/journal/types.ts @@ -0,0 +1,39 @@ +/** + * The journal axis: reading a coding agent's OWN session log. + * + * This module tree knows about harnesses (Claude Code, codex, pi) and nothing + * about herdr. It must never import from `@server/herdr/` — see + * `docs/architecture.md`. herdr's only contribution is the session id, handed + * across as a plain string by the caller. + */ + +/** One turn, already stripped of everything not being served. */ +export interface JournalEntry { + role: "user" | "assistant"; + /** ISO timestamp as the harness wrote it, or null if the record had none. */ + at: string | null; + /** Prose only. ANSI removed, menus removed, truncated. */ + text: string; + /** One-line tool summaries, e.g. "Bash ×3". Never tool OUTPUT. */ + tools: string[]; +} + +export interface JournalAdapter { + /** Harness name, matching herdr's `agent_session.agent`. */ + name: string; + /** + * The harness version this adapter's record shape was last verified against. + * A private on-disk format with no compatibility promise, so this is the + * only honest way to record what "known good" means. + */ + verifiedAgainst: string; + /** Absolute path of the session's log, or null when it cannot be found. */ + locate(value: string, roots: readonly string[]): Promise; + /** Parse a raw slice of the log. Unknown records are ignored, never fatal. */ + parse(chunk: string): JournalEntry[]; +} + +/** Where each harness keeps its logs. A LIST: one machine can hold several. */ +export interface JournalRoots { + claude: readonly string[]; +} diff --git a/tests/journal-registry.test.ts b/tests/journal-registry.test.ts new file mode 100644 index 0000000..9a81bea --- /dev/null +++ b/tests/journal-registry.test.ts @@ -0,0 +1,27 @@ +import { expect, test } from "bun:test"; +import { adapterFor, hasAdapter } from "@server/journal/registry"; + +const claude = { agent: "claude", kind: "id", source: "herdr:claude", value: "u1" }; + +test("a claude session resolves to the claude adapter", () => { + expect(adapterFor(claude)?.name).toBe("claude"); + expect(hasAdapter(claude)).toBe(true); +}); + +test("a harness with no adapter is an ordinary no, not an error", () => { + // The route reports this as `source: "reconstruction"`, so an unknown + // harness must be a null rather than a throw. + expect(adapterFor({ ...claude, agent: "some-other-harness" })).toBeNull(); + expect(hasAdapter({ ...claude, agent: "some-other-harness" })).toBe(false); +}); + +test("a session that is not an id is refused", () => { + // `kind` can name something that is not a session identifier. Only "id" is + // a value this code knows how to turn into a path. + expect(hasAdapter({ ...claude, kind: "path" })).toBe(false); +}); + +test("no session at all is false, never a throw", () => { + expect(hasAdapter(null)).toBe(false); + expect(hasAdapter(undefined)).toBe(false); +}); From d92f71392e37b6639aa3668bece973df5b4aa87e Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 21:16:25 +0700 Subject: [PATCH 07/22] feat: journal path containment and a bounded tail reader a session id arriving over the wire becomes a filesystem path, so it is hostile input before it ever touches disk. isSessionId rejects anything that is not a canonical uuid before a single filesystem call is made, and containedRealpath resolves both the candidate and the root with realpath and compares the resolved forms, because a symlink inside the root is exactly how a path that looks contained stops being contained. tailChunk bounds a single request to MAX_TAIL_BYTES so paging backwards through a huge log stays cheap one page at a time. Co-Authored-By: Claude Opus 5 --- src/server/journal/files.ts | 84 +++++++++++++++++++++++++++++++++++++ tests/journal-files.test.ts | 84 +++++++++++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+) create mode 100644 src/server/journal/files.ts create mode 100644 tests/journal-files.test.ts diff --git a/src/server/journal/files.ts b/src/server/journal/files.ts new file mode 100644 index 0000000..d5d788d --- /dev/null +++ b/src/server/journal/files.ts @@ -0,0 +1,84 @@ +import { realpath } from "node:fs/promises"; +import { join, resolve, sep } from "node:path"; + +/** + * Bytes one request may read from a journal. + * + * Measured on a real session: 1.5 MB across 729 records, ~2 KB per record. So + * this is ~250 records per request — far more than one page of "show earlier", + * and far less than a whole log. A cap on the REQUEST, not on the file: paging + * backwards still reaches the beginning, one bounded read at a time. + */ +export const MAX_TAIL_BYTES = 512_000; + +/** A session id as the harness writes it: canonical 8-4-4-4-12 hex. */ +const SESSION_ID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** + * Whether a value may be turned into a path AT ALL. + * + * Anchored on both ends and checked BEFORE any filesystem call. This is the + * cheap half of containment: nothing with a separator, a dot segment, or an + * extension ever reaches `realpath`. + */ +export function isSessionId(value: string): boolean { + return SESSION_ID_RE.test(value); +} + +/** + * Claude Code's project roots, in search order. + * + * A LIST because `CLAUDE_CONFIG_DIR` is per-profile and one machine can hold + * several Claude homes. Comma-separated, trimmed, empties dropped. + */ +export function claudeRoots(env: Record, home: string): string[] { + const configured = (env.CLAUDE_CONFIG_DIR ?? "") + .split(",") + .map((s) => s.trim()) + .filter((s) => s !== ""); + const dirs = configured.length > 0 ? configured : [join(home, ".claude")]; + return dirs.map((d) => join(d, "projects")); +} + +/** + * The resolved path, if and only if it really sits inside `root`. + * + * Resolved with `realpath`, never compared as strings: a symlink inside the + * root is exactly how a path that LOOKS contained stops being contained, and a + * journal root is a directory the operator's tools write into freely. + * + * Returns null rather than throwing for a missing file — "no journal here" is + * an ordinary answer this feature reports as a fallback, not an exception. + */ +export async function containedRealpath(root: string, candidate: string): Promise { + let real: string; + let realRoot: string; + try { + real = await realpath(resolve(candidate)); + realRoot = await realpath(resolve(root)); + } catch { + return null; + } + const prefix = realRoot.endsWith(sep) ? realRoot : realRoot + sep; + return real.startsWith(prefix) ? real : null; +} + +/** + * The last `maxBytes` of the file ending at `endByte`, and where that slice + * began. + * + * Reads BACKWARDS from a byte offset rather than loading the file: paging is + * the whole reason the route is cursored, and a 1.5 MB read per "show earlier" + * tap on a phone is the cost this avoids. `startByte` is what the caller + * returns as the next cursor. + */ +export async function tailChunk( + path: string, + endByte: number, + maxBytes: number, +): Promise<{ text: string; startByte: number }> { + const capped = Math.min(maxBytes, MAX_TAIL_BYTES); + const startByte = Math.max(0, endByte - capped); + const text = await Bun.file(path).slice(startByte, endByte).text(); + return { text, startByte }; +} diff --git a/tests/journal-files.test.ts b/tests/journal-files.test.ts new file mode 100644 index 0000000..1828b30 --- /dev/null +++ b/tests/journal-files.test.ts @@ -0,0 +1,84 @@ +import { expect, test } from "bun:test"; +import { mkdtemp, mkdir, writeFile, symlink } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + claudeRoots, containedRealpath, isSessionId, MAX_TAIL_BYTES, tailChunk, +} from "@server/journal/files"; + +const UUID = "f4971cd4-d53b-430a-8fc6-a0d4572103ae"; + +test("only a canonical uuid is a session id", () => { + expect(isSessionId(UUID)).toBe(true); + expect(isSessionId("../../etc/passwd")).toBe(false); + expect(isSessionId(`${UUID}/../..`)).toBe(false); + expect(isSessionId("")).toBe(false); + expect(isSessionId(`${UUID}.jsonl`)).toBe(false); +}); + +test("claudeRoots defaults to the home projects dir", () => { + expect(claudeRoots({}, "/srv/operator")).toEqual(["/srv/operator/.claude/projects"]); +}); + +test("claudeRoots takes several config dirs, comma-separated and in order", () => { + // One machine can hold several Claude homes — a per-profile CLAUDE_CONFIG_DIR + // is the case that forces a list rather than a string. + expect(claudeRoots({ CLAUDE_CONFIG_DIR: "/a, /b" }, "/srv/operator")) + .toEqual(["/a/projects", "/b/projects"]); +}); + +test("containedRealpath accepts a file inside the root", async () => { + const root = await mkdtemp(join(tmpdir(), "paddock-j-")); + await mkdir(join(root, "proj")); + const file = join(root, "proj", `${UUID}.jsonl`); + await writeFile(file, "{}\n"); + expect(await containedRealpath(root, file)).toBe(file); +}); + +test("containedRealpath refuses a path that escapes via ..", async () => { + const root = await mkdtemp(join(tmpdir(), "paddock-j-")); + expect(await containedRealpath(root, join(root, "..", "escape.jsonl"))).toBeNull(); +}); + +test("containedRealpath refuses a symlink pointing outside the root", async () => { + // The check is on the RESOLVED path, not the requested one: a symlink inside + // the root is the way a string that looks contained stops being contained. + const root = await mkdtemp(join(tmpdir(), "paddock-j-")); + const outside = await mkdtemp(join(tmpdir(), "paddock-out-")); + const target = join(outside, "secrets.jsonl"); + await writeFile(target, "{}\n"); + const link = join(root, `${UUID}.jsonl`); + await symlink(target, link); + expect(await containedRealpath(root, link)).toBeNull(); +}); + +test("containedRealpath returns null for a file that does not exist", async () => { + const root = await mkdtemp(join(tmpdir(), "paddock-j-")); + expect(await containedRealpath(root, join(root, `${UUID}.jsonl`))).toBeNull(); +}); + +test("tailChunk reads from the END and reports where it started", async () => { + const root = await mkdtemp(join(tmpdir(), "paddock-j-")); + const file = join(root, "big.jsonl"); + const body = Array.from({ length: 100 }, (_, i) => `line-${i}`).join("\n"); + await writeFile(file, body); + const { text, startByte } = await tailChunk(file, body.length, 40); + expect(text.endsWith("line-99")).toBe(true); + expect(startByte).toBe(body.length - 40); + expect(text.length).toBe(40); +}); + +test("tailChunk never reads before the start of the file", async () => { + const root = await mkdtemp(join(tmpdir(), "paddock-j-")); + const file = join(root, "small.jsonl"); + await writeFile(file, "abc"); + const { text, startByte } = await tailChunk(file, 3, 999); + expect(text).toBe("abc"); + expect(startByte).toBe(0); +}); + +test("the tail cap is bounded, so one request cannot read a whole huge log", () => { + // Measured: a real session is 1.5 MB / 729 records, ~2 KB per record. This + // cap is ~250 records' worth per request, well above one page of history. + expect(MAX_TAIL_BYTES).toBe(512_000); +}); From 59c1ce2dc083c25f69f762325e94cc3da7f18e7c Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 21:23:16 +0700 Subject: [PATCH 08/22] fix: journal containment tests must prove the guard, and root failure must not be silent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found two gaps in the containment work. First, the ".." escape test never created its target, so realpath threw ENOENT and the function returned null from the missing-file catch before the containment comparison ran at all — a regression that broke .. specifically, while leaving symlink containment intact, would have gone undetected. The test now writes the escape target first, so the prefix check is what rejects it; the mutation check (containedRealpath -> return real) now turns this test red alongside the symlink test, where before only the symlink test caught it. Second, containedRealpath resolved the root and the candidate inside one try/catch, so a root that fails to resolve — a misconfigured CLAUDE_CONFIG_DIR, a permissions problem, a disk error — was indistinguishable from the ordinary "this session has no journal". The two resolutions are now separate: a candidate that doesn't exist still returns null quietly, but a root that doesn't resolve logs loudly before returning null, so the caller-facing behavior is unchanged but the failure is no longer invisible on the host. Co-Authored-By: Claude Opus 5 --- src/server/journal/files.ts | 23 +++++++++++++++++++---- tests/journal-files.test.ts | 9 ++++++++- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/server/journal/files.ts b/src/server/journal/files.ts index d5d788d..c4d5a3e 100644 --- a/src/server/journal/files.ts +++ b/src/server/journal/files.ts @@ -47,18 +47,33 @@ export function claudeRoots(env: Record, home: strin * root is exactly how a path that LOOKS contained stops being contained, and a * journal root is a directory the operator's tools write into freely. * - * Returns null rather than throwing for a missing file — "no journal here" is - * an ordinary answer this feature reports as a fallback, not an exception. + * The two `realpath` calls are resolved separately, and deliberately NOT + * folded into one try/catch, because they fail for different reasons: + * + * - The ROOT failing to resolve is a configuration problem — a misconfigured + * `CLAUDE_CONFIG_DIR`, a permissions error, or a disk error. That is not + * "no journal here"; it is a host-side fault this process should be loud + * about, even though the caller still only ever sees `null`. + * - The CANDIDATE not existing is an ordinary "no journal here" — the answer + * this feature reports as a fallback for a session with no log, not an + * exception. */ export async function containedRealpath(root: string, candidate: string): Promise { - let real: string; let realRoot: string; try { - real = await realpath(resolve(candidate)); realRoot = await realpath(resolve(root)); + } catch (err) { + console.error(`journal: root does not resolve, check CLAUDE_CONFIG_DIR: ${root}`, err); + return null; + } + + let real: string; + try { + real = await realpath(resolve(candidate)); } catch { return null; } + const prefix = realRoot.endsWith(sep) ? realRoot : realRoot + sep; return real.startsWith(prefix) ? real : null; } diff --git a/tests/journal-files.test.ts b/tests/journal-files.test.ts index 1828b30..9ceaa02 100644 --- a/tests/journal-files.test.ts +++ b/tests/journal-files.test.ts @@ -36,8 +36,15 @@ test("containedRealpath accepts a file inside the root", async () => { }); test("containedRealpath refuses a path that escapes via ..", async () => { + // The escape target must actually EXIST: otherwise realpath throws ENOENT + // and the function returns null from the missing-file catch before the + // containment comparison ever runs, and this test would pass even if that + // comparison were deleted. Creating the file makes realpath succeed, so the + // prefix check is what does the rejecting — mirrors the symlink test below. const root = await mkdtemp(join(tmpdir(), "paddock-j-")); - expect(await containedRealpath(root, join(root, "..", "escape.jsonl"))).toBeNull(); + const escape = join(root, "..", "escape.jsonl"); + await writeFile(escape, "{}\n"); + expect(await containedRealpath(root, escape)).toBeNull(); }); test("containedRealpath refuses a symlink pointing outside the root", async () => { From d10a10227f0167b5a0afa07ad0d127e7e359f712 Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 21:28:56 +0700 Subject: [PATCH 09/22] =?UTF-8?q?feat:=20journal=20text=20shaping=20?= =?UTF-8?q?=E2=80=94=20prose=20kept,=20tool=20output=20and=20menus=20never?= =?UTF-8?q?=20served?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the exposure decision for served history: ANSI is stripped, an already-answered menu is stripped so it cannot blend into the live screen and read as the current prompt, tool calls collapse to name + short hint (never their output), and everything is clamped so no unbounded string reaches the wire. Mutation-checked stripMenu by neutering it to a no-op — both menu tests went red, confirming they actually exercise the guard. Co-Authored-By: Claude Opus 5 --- src/server/journal/text.ts | 96 ++++++++++++++++++++++++++++++++++++++ tests/journal-text.test.ts | 77 ++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+) create mode 100644 src/server/journal/text.ts create mode 100644 tests/journal-text.test.ts diff --git a/src/server/journal/text.ts b/src/server/journal/text.ts new file mode 100644 index 0000000..2a36e48 --- /dev/null +++ b/src/server/journal/text.ts @@ -0,0 +1,96 @@ +import type { JournalEntry } from "@server/journal/types"; + +/** Ceiling on one turn's prose. Generous for a message, bounded on the wire. */ +export const MAX_TEXT_CHARS = 4_000; + +/** Ceiling on a tool summary line. Orientation, never a transcript. */ +const MAX_TOOL_HINT = 80; + +// eslint-disable-next-line no-control-regex +const ANSI_RE = /\[[0-9;?]*[ -/]*[@-~]|[()][A-Za-z0-9]|./g; + +export function stripAnsi(text: string): string { + return text.replace(ANSI_RE, ""); +} + +/** + * A cursor-marked row, e.g. `❯ 1. Yes`, or a bare numbered option row. + * + * Requires the row to be ONLY the option — anchored both ends, short label — + * so ordinary prose that happens to open with a number survives. Over-stripping + * silently eats real content, which is a worse failure than the one this + * guards. + */ +const MENU_RE = /^\s*(?:❯\s*)?\d{1,2}\.\s+\S[^\n]{0,60}$/; +const CURSOR_ONLY_RE = /^\s*❯\s*\S[^\n]{0,60}$/; + +/** + * Remove an option row from journal text, leaving "" if that is all it was. + * + * WHY: journal lines are blended directly above the live screen with no + * divider (design decision 3). A menu from an already-answered question would + * then read as the live prompt — the failure `prompt-parse.ts` already records + * in its own scoping comment. Only the live screen may show a selectable menu. + */ +export function stripMenu(text: string): string { + if (MENU_RE.test(text) || CURSOR_ONLY_RE.test(text)) return ""; + return text; +} + +/** + * Truncate to AT MOST `max` characters, ellipsis included, so a cut is never + * mistaken for the end and a caller's cap is never off by one. + */ +export function clamp(text: string, max: number): string { + return text.length <= max ? text : text.slice(0, Math.max(0, max - 1)) + "…"; +} + +/** + * One line for a tool call: its name, and a short hint at what it touched. + * + * Never its RESULT. Tool results are where file contents, command output and + * any secret that passed through the agent live, and design decision 4 keeps + * them off the wire entirely. + */ +export function summariseTool(name: string, input: unknown): string { + const obj = (typeof input === "object" && input !== null ? input : {}) as Record; + const raw = + typeof obj.description === "string" ? obj.description + : typeof obj.file_path === "string" ? obj.file_path.split("/").pop() ?? "" + : typeof obj.pattern === "string" ? obj.pattern + : ""; + const hint = stripAnsi(raw).replace(/\s+/g, " ").trim(); + // Clamped on the FINISHED line, not on the hint, so the cap holds whatever + // the tool name's length happens to be. + return clamp(hint === "" ? name : `${name} · ${hint}`, MAX_TOOL_HINT); +} + +/** `13:04` from an ISO stamp, or "" when the record carried none. */ +function hhmm(at: string | null): string { + if (at === null) return ""; + const d = new Date(at); + return Number.isNaN(d.getTime()) + ? "" + : `${String(d.getUTCHours()).padStart(2, "0")}:${String(d.getUTCMinutes()).padStart(2, "0")}`; +} + +/** + * Flatten turns to the lines the terminal renders. + * + * Server-side, because the client must gain no per-harness knowledge — the same + * reason `parsePrompt` lives on this side of the socket. + */ +export function toLines(entries: readonly JournalEntry[]): string[] { + const out: string[] = []; + for (const e of entries) { + const body = clamp(stripMenu(stripAnsi(e.text)).trim(), MAX_TEXT_CHARS); + if (body === "" && e.tools.length === 0) continue; + const who = e.role === "user" ? "you" : "agent"; + const time = hhmm(e.at); + out.push(time === "" ? who : `${who} · ${time}`); + if (e.tools.length > 0) out.push(`▸ ${e.tools.join(" · ")}`); + if (body !== "") out.push(...body.split("\n")); + out.push(""); + } + return out; +} diff --git a/tests/journal-text.test.ts b/tests/journal-text.test.ts new file mode 100644 index 0000000..24ae982 --- /dev/null +++ b/tests/journal-text.test.ts @@ -0,0 +1,77 @@ +import { expect, test } from "bun:test"; +import { + clamp, MAX_TEXT_CHARS, stripAnsi, stripMenu, summariseTool, toLines, +} from "@server/journal/text"; + +test("ansi escapes are removed", () => { + expect(stripAnsi("hello")).toBe("hello"); +}); + +test("a cursor marker is stripped from journal text", () => { + // THE hazard. A journal turn can carry an ALREADY ANSWERED menu, and blended + // straight above the live screen it reads as the question being asked now. + // `prompt-parse.ts` records this exact failure. Only the live screen may + // render a selectable menu. + expect(stripMenu("❯ 1. Yes")).toBe(""); + expect(stripMenu(" ❯ 2. No, keep it")).toBe(""); +}); + +test("a numbered option row is stripped even without a cursor", () => { + expect(stripMenu(" 2. No")).toBe(""); + expect(stripMenu("1. Approve this change")).toBe(""); +}); + +test("ordinary prose that merely starts with a number survives", () => { + // Over-stripping would silently eat real content, which is worse than the + // hazard it guards: "2. " here is prose the agent wrote, not an option row. + expect(stripMenu("2026 was the year")).toBe("2026 was the year"); + expect(stripMenu("I found 3 failures")).toBe("I found 3 failures"); +}); + +test("a tool call becomes a name and a short hint, never its output", () => { + expect(summariseTool("Bash", { command: "bun test", description: "run tests" })) + .toBe("Bash · run tests"); + expect(summariseTool("Read", { file_path: "/srv/project/src/timer.ts" })) + .toBe("Read · timer.ts"); + expect(summariseTool("Write", {})).toBe("Write"); +}); + +test("a tool hint never carries a whole command line", () => { + // The hint is orientation, not a transcript. An unbounded command would put + // arbitrary shell text — and anything interpolated into it — on the wire. + const long = "x".repeat(500); + expect(summariseTool("Bash", { description: long }).length).toBeLessThanOrEqual(80); +}); + +test("clamp truncates to AT MOST max characters, ellipsis included", () => { + // The ellipsis counts. A clamp that returns max+1 makes every caller's cap + // a lie by one character, which is how `summariseTool` would exceed its own. + expect(clamp("abcdef", 3)).toBe("ab…"); + expect(clamp("abcdef", 3).length).toBe(3); + expect(clamp("abc", 10)).toBe("abc"); +}); + +test("toLines renders a turn with a speaker and folds its tools", () => { + const lines = toLines([ + { role: "user", at: "2026-08-20T13:04:00Z", text: "fix the flaky test", tools: [] }, + { role: "assistant", at: "2026-08-20T13:05:00Z", text: "Found it: the timer resets.", tools: ["Bash ×3", "Read timer.ts"] }, + ]); + expect(lines).toEqual([ + "you · 13:04", + "fix the flaky test", + "", + "agent · 13:05", + "▸ Bash ×3 · Read timer.ts", + "Found it: the timer resets.", + "", + ]); +}); + +test("toLines drops a turn left empty by stripping", () => { + // A turn that was only a menu must not leave a bare speaker line behind. + expect(toLines([{ role: "assistant", at: null, text: "", tools: [] }])).toEqual([]); +}); + +test("the text cap is bounded", () => { + expect(MAX_TEXT_CHARS).toBe(4_000); +}); From 37fc5aa4b3f5f211e904d0697c35f26dcbb0668a Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 21:38:35 +0700 Subject: [PATCH 10/22] fix: stripMenu must strip menus, not just single-line toy cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The brief's anchored ^...$ regexes only matched when the ENTIRE turn text was one bare option line. A real prompt is a question plus two or more option lines, so the guard essentially never fired on real journal data — exactly the hazard it exists to prevent. Rewritten to operate line by line: split, drop lines shaped like options, rejoin. Also folds in review findings on the same regexes: an ASCII `>` cursor is treated like `❯`; the 60-char label cap is gone (length must not decide whether a row is an option); `)` is accepted as a separator alongside `.`; a lettered option ("a. Yes") only counts as an option with a cursor present, since a bare lettered row is too easily real prose; and a cursor sitting on non-option prose ("❯ npm install") is kept rather than stripped, since deleting real content is worse than one stray glyph. Re-ran the mutation check with the fix in place: neutering stripMenu to a no-op turned 7 tests red, including the new multi-line case, confirming the guard now exercises the real hazard rather than the toy one. Co-Authored-By: Claude Opus 5 --- src/server/journal/text.ts | 53 ++++++++++++++++++++++++++++---------- tests/journal-text.test.ts | 36 ++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 14 deletions(-) diff --git a/src/server/journal/text.ts b/src/server/journal/text.ts index 2a36e48..aacd744 100644 --- a/src/server/journal/text.ts +++ b/src/server/journal/text.ts @@ -14,27 +14,52 @@ export function stripAnsi(text: string): string { } /** - * A cursor-marked row, e.g. `❯ 1. Yes`, or a bare numbered option row. + * A digit-numbered option row, cursor optional: "1. Yes", "❯ 2. No", + * "1) Yes", "> 3) No, keep it". `>` is treated the same as `❯` — the glyph + * is not universal and ASCII `>` is at least as common a cursor marker. * - * Requires the row to be ONLY the option — anchored both ends, short label — - * so ordinary prose that happens to open with a number survives. Over-stripping - * silently eats real content, which is a worse failure than the one this - * guards. + * No length cap on the label: length must never decide whether a row is an + * option, or a long real option (e.g. "❯ 1. Yes, and also run the full + * regression suite before merging") would survive whole. */ -const MENU_RE = /^\s*(?:❯\s*)?\d{1,2}\.\s+\S[^\n]{0,60}$/; -const CURSOR_ONLY_RE = /^\s*❯\s*\S[^\n]{0,60}$/; +const DIGIT_OPTION_RE = /^\s*(?:[❯>]\s*)?\d+[.)]\s+\S.*$/; /** - * Remove an option row from journal text, leaving "" if that is all it was. + * A lettered option row, cursor REQUIRED: "❯ a. Yes". Without a cursor, a + * bare "a. done" is too easily real prose — over-stripping is the worse + * failure than the one this guards, so a lettered row only counts as an + * option when a cursor marks it as selected. + */ +const CURSOR_LETTER_OPTION_RE = /^\s*[❯>]\s*[A-Za-z][.)]\s+\S.*$/; + +function isOptionRow(line: string): boolean { + return DIGIT_OPTION_RE.test(line) || CURSOR_LETTER_OPTION_RE.test(line); +} + +/** + * Remove option rows from journal text, dropped line by line, leaving "" if + * every line was one. + * + * WHY LINE BY LINE: a real prompt is a question plus two or more option + * lines, not one bare option line on its own. Matching the anchored pattern + * against the WHOLE turn text only ever fires on that single-line toy case; + * a real multi-line menu — "Do you want to proceed?\n❯ 1. Yes\n 2. No" — + * would sail through unchanged. Splitting into lines and dropping only the + * ones shaped like options is what strips the menu down to its question. * - * WHY: journal lines are blended directly above the live screen with no - * divider (design decision 3). A menu from an already-answered question would - * then read as the live prompt — the failure `prompt-parse.ts` already records - * in its own scoping comment. Only the live screen may show a selectable menu. + * WHY a bare cursor on non-option text is KEPT, not stripped: journal lines + * are blended directly above the live screen with no divider (design + * decision 3), and a menu from an already-answered question reading as the + * live prompt is the specific failure `prompt-parse.ts` already records. But + * a cursor glyph sitting on ordinary prose — "❯ npm install" quoted in a + * message — is not that hazard, and deleting it would silently eat real + * content, which is the worse failure this function exists to avoid. */ export function stripMenu(text: string): string { - if (MENU_RE.test(text) || CURSOR_ONLY_RE.test(text)) return ""; - return text; + return text + .split("\n") + .filter((line) => !isOptionRow(line)) + .join("\n"); } /** diff --git a/tests/journal-text.test.ts b/tests/journal-text.test.ts index 24ae982..7860637 100644 --- a/tests/journal-text.test.ts +++ b/tests/journal-text.test.ts @@ -21,6 +21,42 @@ test("a numbered option row is stripped even without a cursor", () => { expect(stripMenu("1. Approve this change")).toBe(""); }); +test("a real multi-line menu is stripped down to its question", () => { + // THE decisive case. A real prompt is a question plus two or more option + // lines, not one bare option line on its own — an anchored ^...$ match + // against the WHOLE turn text only ever fires on the single-line toy case. + expect( + stripMenu("Do you want to proceed?\n❯ 1. Yes\n 2. No, tell it what to do differently"), + ).toBe("Do you want to proceed?"); +}); + +test("an ASCII > cursor is treated like ❯", () => { + expect(stripMenu("> 1. Yes")).toBe(""); +}); + +test("a ) separator is accepted alongside .", () => { + expect(stripMenu("2) No")).toBe(""); +}); + +test("an option row survives no matter how long its label is", () => { + // Length must not decide whether a row is an option: a long real option is + // still an option. + expect( + stripMenu("❯ 1. Yes, and also run the full regression suite before merging"), + ).toBe(""); +}); + +test("a cursor sitting on ordinary prose is kept, not stripped", () => { + // A cursor glyph quoting a shell prompt is not the hazard this guards — + // deleting it would silently eat real content, the worse failure. + expect(stripMenu("❯ npm install")).toBe("❯ npm install"); +}); + +test("a lettered option is stripped only when a cursor marks it", () => { + expect(stripMenu("❯ a. Yes")).toBe(""); + expect(stripMenu("a. done")).toBe("a. done"); +}); + test("ordinary prose that merely starts with a number survives", () => { // Over-stripping would silently eat real content, which is worse than the // hazard it guards: "2. " here is prose the agent wrote, not an option row. From 7f3deccfa9fbf9c3af1bc157e21c6ad511d40881 Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 21:45:07 +0700 Subject: [PATCH 11/22] feat: the Claude Code journal adapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the Task 3 stub with a real locate/parse. locate resolves a session uuid against each configured journal root without ever touching disk on a malformed id, and containedRealpath keeps a resolved path pinned inside that root. parse turns Claude Code's private JSONL record shape into JournalEntry turns: text and tool_use become one assistant turn, thinking blocks and sidechain (subagent) records are dropped, and a user record is only a real turn when its content is a string — a list is tool-result traffic wearing the user role, and letting it through would both fabricate hundreds of "you" turns and leak whatever the tool result carried (the fixture's fake SECRET_TOKEN is exactly that case). One unparseable line costs itself, not the rest of the file, since a tail read can start mid-record. verifiedAgainst now names the harness version this shape was checked against instead of the stub's "unverified", because the format is undocumented and this string is the only record of when it was last confirmed. Mutation-checked per house rule 4: making the user branch also accept a list turned the SECRET_TOKEN and list-is-not-a-message tests red (and only those two); reverting brought the suite back to green. Co-Authored-By: Claude Opus 5 --- src/server/journal/claude.ts | 100 +++++++++++++++++++- tests/fixtures/journal/claude-session.jsonl | 8 ++ tests/journal-claude.test.ts | 57 +++++++++++ 3 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 tests/fixtures/journal/claude-session.jsonl create mode 100644 tests/journal-claude.test.ts diff --git a/src/server/journal/claude.ts b/src/server/journal/claude.ts index 2ec77b3..b612da1 100644 --- a/src/server/journal/claude.ts +++ b/src/server/journal/claude.ts @@ -1,8 +1,100 @@ -import type { JournalAdapter } from "@server/journal/types"; +import { readdir } from "node:fs/promises"; +import { join } from "node:path"; +import { containedRealpath, isSessionId } from "@server/journal/files"; +import { summariseTool } from "@server/journal/text"; +import type { JournalAdapter, JournalEntry } from "@server/journal/types"; +/** + * Claude Code's journal adapter. + * + * WHY THIS EXISTS. A pane running Claude sits on the terminal's ALTERNATE + * SCREEN, which has no scrollback ring, so `pane.read` can never return more + * than the viewport however much is asked for — see + * `docs/design/2026-08-20-journal-history-design.md` for the measurements. The + * history does exist, in Claude Code's own session log, and herdr hands us its + * uuid on `agent_session`. + * + * SHAPE OF THE SOURCE. This is a PRIVATE on-disk format with no compatibility + * promise; it will change without notice. Every unknown record type is ignored + * rather than fatal, and one unparseable line is skipped rather than costing + * the file. `verifiedAgainst` records when the shape was last checked by hand — + * update it whenever you re-check, the way `docs/gotchas.md` treats every other + * measured claim in this repo. + * + * {"type":"user", "message":{"role":"user","content":"…" | [ {type:"tool_result"} ]}} + * {"type":"assistant", "message":{"role":"assistant","content":[ {type:"text"|"thinking"|"tool_use"} ]}} + * + * A `user` record whose content is a LIST is tool-result traffic, not something + * a person typed. `isSidechain` marks subagent traffic. + */ export const claudeAdapter: JournalAdapter = { name: "claude", - verifiedAgainst: "unverified", - async locate() { return null; }, - parse() { return []; }, + verifiedAgainst: "Claude Code 2.1.220, checked 2026-08-20", + + async locate(value, roots) { + // Checked before any filesystem call — see files.ts. + if (!isSessionId(value)) return null; + for (const root of roots) { + let projects: string[]; + try { + projects = await readdir(root); + } catch { + continue; // a root that does not exist is not an error, it is a miss + } + for (const project of projects) { + const found = await containedRealpath(root, join(root, project, `${value}.jsonl`)); + if (found !== null) return found; + } + } + return null; + }, + + parse(chunk) { + const out: JournalEntry[] = []; + for (const line of chunk.split("\n")) { + if (line.trim() === "") continue; + let rec: Record; + try { + rec = JSON.parse(line) as Record; + } catch { + // A partial first line is NORMAL: a tail read starts mid-record. A + // genuinely corrupt line costs itself and nothing else. + continue; + } + const entry = toEntry(rec); + if (entry !== null) out.push(entry); + } + return out; + }, }; + +function toEntry(rec: Record): JournalEntry | null { + const type = rec.type; + if (type !== "user" && type !== "assistant") return null; // bookkeeping rows + if (rec.isSidechain === true) return null; // subagent traffic + + const at = typeof rec.timestamp === "string" ? rec.timestamp : null; + const message = rec.message as { content?: unknown } | undefined; + const content = message?.content; + + if (type === "user") { + // A STRING is a person typing. A LIST is tool-result traffic wearing the + // user role, and rendering those would fabricate hundreds of "you" turns. + if (typeof content !== "string" || content.trim() === "") return null; + return { role: "user", at, text: content, tools: [] }; + } + + if (!Array.isArray(content)) return null; + const texts: string[] = []; + const tools: string[] = []; + for (const part of content) { + const p = part as Record; + if (p.type === "text" && typeof p.text === "string") texts.push(p.text); + else if (p.type === "tool_use" && typeof p.name === "string") { + tools.push(summariseTool(p.name, p.input)); + } + // "thinking" and everything unknown falls through deliberately. + } + if (texts.length === 0 && tools.length === 0) return null; + return { role: "assistant", at, text: texts.join("\n"), tools }; +} diff --git a/tests/fixtures/journal/claude-session.jsonl b/tests/fixtures/journal/claude-session.jsonl new file mode 100644 index 0000000..b859970 --- /dev/null +++ b/tests/fixtures/journal/claude-session.jsonl @@ -0,0 +1,8 @@ +{"type":"user","timestamp":"2026-08-20T13:04:00Z","message":{"role":"user","content":"fix the flaky test"}} +{"type":"assistant","timestamp":"2026-08-20T13:04:30Z","message":{"role":"assistant","content":[{"type":"thinking","thinking":"private reasoning that must not be served"},{"type":"text","text":"Looking at the timer now."},{"type":"tool_use","name":"Bash","input":{"command":"bun test","description":"run tests"}}]}} +{"type":"user","timestamp":"2026-08-20T13:04:31Z","message":{"role":"user","content":[{"type":"tool_result","content":"SECRET_TOKEN=abc123 leaked in output"}]}} +{"type":"assistant","timestamp":"2026-08-20T13:05:00Z","message":{"role":"assistant","content":[{"type":"text","text":"Found it: the timer resets."}]}} +{"type":"assistant","timestamp":"2026-08-20T13:05:10Z","isSidechain":true,"message":{"role":"assistant","content":[{"type":"text","text":"subagent chatter"}]}} +{"type":"mode","timestamp":"2026-08-20T13:05:20Z","mode":"default"} +not valid json at all +{"type":"assistant","timestamp":"2026-08-20T13:06:00Z","message":{"role":"assistant","content":[{"type":"text","text":"❯ 1. Yes"}]}} diff --git a/tests/journal-claude.test.ts b/tests/journal-claude.test.ts new file mode 100644 index 0000000..bcfed3f --- /dev/null +++ b/tests/journal-claude.test.ts @@ -0,0 +1,57 @@ +import { expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { claudeAdapter } from "@server/journal/claude"; + +const chunk = readFileSync("tests/fixtures/journal/claude-session.jsonl", "utf8"); +const entries = claudeAdapter.parse(chunk); + +test("a typed user message becomes a user turn", () => { + expect(entries[0]).toEqual({ + role: "user", at: "2026-08-20T13:04:00Z", text: "fix the flaky test", tools: [], + }); +}); + +test("assistant text and its tool call arrive as one turn", () => { + expect(entries[1]!.role).toBe("assistant"); + expect(entries[1]!.text).toBe("Looking at the timer now."); + expect(entries[1]!.tools).toEqual(["Bash · run tests"]); +}); + +test("a tool RESULT is never served", () => { + // This is where file contents, command output and secrets live. Asserted on + // the whole parse, because one leak anywhere is the whole failure. + expect(JSON.stringify(entries)).not.toContain("SECRET_TOKEN"); +}); + +test("a user record whose content is a LIST is not a typed message", () => { + // Folding these is what stops a session rendering hundreds of fabricated + // "you" turns: tool-result traffic is written as role user. + expect(entries.filter((e) => e.role === "user")).toHaveLength(1); +}); + +test("thinking blocks are dropped", () => { + expect(JSON.stringify(entries)).not.toContain("private reasoning"); +}); + +test("subagent traffic is dropped", () => { + expect(JSON.stringify(entries)).not.toContain("subagent chatter"); +}); + +test("bookkeeping records are ignored, not turned into turns", () => { + expect(entries.every((e) => e.text !== "default")).toBe(true); +}); + +test("one unparseable line is skipped without losing the file", () => { + // The record AFTER the broken line must still be present: a private format + // will grow rows this parser has never seen, and one of them must not cost + // the operator their whole history. + expect(entries.at(-1)!.text).toBe("❯ 1. Yes"); +}); + +test("the adapter records the harness version its shape was verified against", () => { + expect(claudeAdapter.verifiedAgainst).not.toBe("unverified"); +}); + +test("locate refuses a value that is not a session id, before touching disk", async () => { + expect(await claudeAdapter.locate("../../etc/passwd", ["/nonexistent"])).toBeNull(); +}); From f9343de2e1e8583068392c0a21e5e4beb41c0d0f Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 21:53:25 +0700 Subject: [PATCH 12/22] fix: a malformed content element must cost one record, not the file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found that toEntry dereferenced each assistant content-array element without checking it was an object first. A record like {"content":[null,{"type":"text","text":"hi"}]} is valid JSON, so JSON.parse succeeds and the loop then throws on `p.type` with p === null. parse() only wrapped the JSON.parse call in try/catch, so that throw propagated out of the exported parse() and cost the operator their entire history — directly contradicting both types.ts's contract ("Unknown records are ignored, never fatal") and this module's own header ("one unparseable line is skipped rather than costing the file"). Reproduced the crash directly against the pre-fix code: TypeError: null is not an object (evaluating 'p.type'), unhandled, at claude.ts:92 via parse() at claude.ts:64. Fixed at both levels, since they guard different things: toEntry is now called inside the same per-record try (so any throw from a shape nobody has planned for costs only that record, which is the general guarantee this format's private, unversioned nature requires), and the content-part loop now skips any element that isn't an object before touching its fields (so the known null-element case is handled precisely, not just caught). Also addressed: locate's readdir catch treated every failure as an ordinary miss. Split on error code the same way containedRealpath already does for its root argument — ENOENT stays a quiet miss, but anything else (e.g. an unreadable CLAUDE_CONFIG_DIR) now gets a console.error so it leaves a diagnostic trail instead of presenting as "no journal here". Extended the existing fixture and test file rather than replacing them, so prior assertions (entries[0], entries[1], entries.at(-1)) keep the same meaning. New coverage: a null content-array element whose good sibling still survives, a record with no message at all, and a user record whose content is a number — all asserted to produce no entry and not throw, plus a fixed-length check on the whole fixture's output. Co-Authored-By: Claude Opus 5 --- src/server/journal/claude.ts | 29 ++++++++++++++++++--- tests/fixtures/journal/claude-session.jsonl | 3 +++ tests/journal-claude.test.ts | 23 ++++++++++++++++ 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/server/journal/claude.ts b/src/server/journal/claude.ts index b612da1..2931c1a 100644 --- a/src/server/journal/claude.ts +++ b/src/server/journal/claude.ts @@ -38,8 +38,15 @@ export const claudeAdapter: JournalAdapter = { let projects: string[]; try { projects = await readdir(root); - } catch { - continue; // a root that does not exist is not an error, it is a miss + } catch (err) { + // ENOENT is an ordinary miss: this root simply has no journal here. + // Anything else (e.g. EACCES) is a host-side fault — the same + // distinction containedRealpath makes for its root argument — so it + // must be loud even though the caller still only ever sees `null`. + if ((err as NodeJS.ErrnoException)?.code !== "ENOENT") { + console.error(`journal: root did not read, check CLAUDE_CONFIG_DIR: ${root}`, err); + } + continue; } for (const project of projects) { const found = await containedRealpath(root, join(root, project, `${value}.jsonl`)); @@ -61,8 +68,17 @@ export const claudeAdapter: JournalAdapter = { // genuinely corrupt line costs itself and nothing else. continue; } - const entry = toEntry(rec); - if (entry !== null) out.push(entry); + // toEntry is called INSIDE this try, not after it: JSON.parse succeeding + // is no guarantee the record's shape is one toEntry can safely walk (a + // content element can be `null`, a string, a number — anything valid + // JSON allows). This is a private, unversioned format, so a shape + // nobody has seen yet must cost only this record, never the file. + try { + const entry = toEntry(rec); + if (entry !== null) out.push(entry); + } catch { + continue; + } } return out; }, @@ -88,6 +104,11 @@ function toEntry(rec: Record): JournalEntry | null { const texts: string[] = []; const tools: string[] = []; for (const part of content) { + // A content element is allowed to be anything valid JSON permits — this + // is a private, unversioned format. `null`, a bare string, a number: none + // of those are an object, so skip them here rather than relying solely on + // the outer try/catch to survive a shape like `[null, {"type":"text",…}]`. + if (typeof part !== "object" || part === null) continue; const p = part as Record; if (p.type === "text" && typeof p.text === "string") texts.push(p.text); else if (p.type === "tool_use" && typeof p.name === "string") { diff --git a/tests/fixtures/journal/claude-session.jsonl b/tests/fixtures/journal/claude-session.jsonl index b859970..16d417c 100644 --- a/tests/fixtures/journal/claude-session.jsonl +++ b/tests/fixtures/journal/claude-session.jsonl @@ -5,4 +5,7 @@ {"type":"assistant","timestamp":"2026-08-20T13:05:10Z","isSidechain":true,"message":{"role":"assistant","content":[{"type":"text","text":"subagent chatter"}]}} {"type":"mode","timestamp":"2026-08-20T13:05:20Z","mode":"default"} not valid json at all +{"type":"assistant","timestamp":"2026-08-20T13:05:40Z","message":{"role":"assistant","content":[null,{"type":"text","text":"survives next to a null part"}]}} +{"type":"assistant","timestamp":"2026-08-20T13:05:50Z"} +{"type":"user","timestamp":"2026-08-20T13:05:55Z","message":{"role":"user","content":42}} {"type":"assistant","timestamp":"2026-08-20T13:06:00Z","message":{"role":"assistant","content":[{"type":"text","text":"❯ 1. Yes"}]}} diff --git a/tests/journal-claude.test.ts b/tests/journal-claude.test.ts index bcfed3f..c615051 100644 --- a/tests/journal-claude.test.ts +++ b/tests/journal-claude.test.ts @@ -48,6 +48,29 @@ test("one unparseable line is skipped without losing the file", () => { expect(entries.at(-1)!.text).toBe("❯ 1. Yes"); }); +test("a null element in a content array does not throw, and the record's good part survives", () => { + // Any content element can be anything valid JSON allows — this is a + // private, unversioned format. A `null` element must cost nothing: not the + // record it sits in, and not the file around it. + const survivor = entries.find((e) => e.text === "survives next to a null part"); + expect(survivor).toBeDefined(); + expect(survivor!.role).toBe("assistant"); + // The record after this one (across the earlier broken line too) is still there. + expect(entries.at(-1)!.text).toBe("❯ 1. Yes"); +}); + +test("a record with no message at all produces no entry and does not throw", () => { + expect(entries.some((e) => e.at === "2026-08-20T13:05:50Z")).toBe(false); +}); + +test("a user record whose content is a number produces no entry and does not throw", () => { + expect(entries.some((e) => e.at === "2026-08-20T13:05:55Z")).toBe(false); +}); + +test("the full fixture yields exactly the expected turns, nothing lost or fabricated", () => { + expect(entries).toHaveLength(5); +}); + test("the adapter records the harness version its shape was verified against", () => { expect(claudeAdapter.verifiedAgainst).not.toBe("unverified"); }); From 2922f393b3313dcfb4548e25c5c5a37ea050e6ab Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 22:05:13 +0700 Subject: [PATCH 13/22] feat: POST /api/agents/:id/history, served from the agent's own log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exposes the journal reader (Task 3-6) over HTTP. The route is registered unconditionally, like /ack, because it reads a file and never touches herdr — gating it on the herdr actions dependency would repeat the /ack mistake: the one feature that works without herdr being the one visibly broken in --demo. The cursor travels in the POST body, never a query string, so it never lands in an edge access log. supervisor.ts now injects hasJournal (via journal/registry's hasAdapter) into toAgents and captures agent.list's session refs each reconcile, so routes.ts can resolve an agent id to the herdr session read() needs without ever importing herdr itself. Live-verified against a running herdr 0.8.2 with real Claude Code panes: a real pane returned source:"journal" with real lines from its own session log, and paging backward with the returned cursor produced the preceding turns with an earlier cursor — confirms the byte-offset paging actually walks the file rather than repeating a page. An unknown agent id 404s and a non-digit cursor 400s, also live. Every pane on this machine happened to be a Claude Code harness, so the "reconstruction" fallback for a non-journal pane could not be exercised against a live herdr; it is covered instead by two of the seven route tests with an injected reader. The brief's test file needed one change from its verbatim text: the harness() helper's default-valued `page` parameter had no type annotation, so tsc inferred a type from the "journal" literal default and rejected the later "reconstruction" literals passed to it. Annotated as JournalPage to fix, with no change to what the tests assert. Co-Authored-By: Claude Opus 5 --- src/server/index.ts | 3 + src/server/journal/read.ts | 73 +++++++++++++++++++++++ src/server/routes.ts | 59 +++++++++++++++++++ src/server/supervisor.ts | 15 ++++- tests/journal-route.test.ts | 114 ++++++++++++++++++++++++++++++++++++ 5 files changed, 263 insertions(+), 1 deletion(-) create mode 100644 src/server/journal/read.ts create mode 100644 tests/journal-route.test.ts diff --git a/src/server/index.ts b/src/server/index.ts index 447e465..f2cd263 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -14,6 +14,7 @@ import { createActions, type HerdrActions } from "@server/herdr/actions"; import { StreamKeeper } from "@server/herdr/keeper"; import { AgentStore } from "@server/state/store"; import { Supervisor } from "@server/supervisor"; +import { createJournalReader, defaultRoots } from "@server/journal/read"; import { shapeMessage, shapeSummary } from "@server/herdr/shape"; import { Hub } from "@server/ws/hub"; import { hubWebSocket, tryUpgradeWs, type WsData } from "@server/ws/serve"; @@ -516,6 +517,8 @@ const appDeps = { hub, actions, settings, + journal: createJournalReader(defaultRoots(process.env, homedir())), + sessionFor: (id: string) => supervisor?.sessionFor(id) ?? null, health: () => ({ ok: true, hostId, diff --git a/src/server/journal/read.ts b/src/server/journal/read.ts new file mode 100644 index 0000000..afc14fe --- /dev/null +++ b/src/server/journal/read.ts @@ -0,0 +1,73 @@ +import { adapterFor } from "@server/journal/registry"; +import { claudeRoots, MAX_TAIL_BYTES, tailChunk } from "@server/journal/files"; +import { toLines } from "@server/journal/text"; +import type { JournalRoots } from "@server/journal/types"; +import type { HerdrAgentSession } from "@shared/herdr-api"; + +export interface JournalPage { + lines: string[]; + /** + * `"reconstruction"` is the server saying "I have no journal for this + * agent" — it always comes with `lines: []` and a `detail`. Reconstruction + * itself is entirely client-side. This field is a ROUTING answer, not a + * description of the payload. + */ + source: "journal" | "reconstruction"; + hasMore: boolean; + /** Opaque to the client: a byte offset it echoes back, never constructs. */ + cursor: string | null; + detail: string | null; +} + +export interface JournalReader { + read( + session: HerdrAgentSession | null | undefined, + before: number | null, + limit: number, + ): Promise; +} + +const none = (detail: string): JournalPage => ({ + lines: [], source: "reconstruction", hasMore: false, cursor: null, detail, +}); + +export function createJournalReader(roots: JournalRoots): JournalReader { + return { + async read(session, before, limit) { + const adapter = adapterFor(session); + if (adapter === null || !session) return none("no journal adapter for this harness"); + + const path = await adapter.locate(session.value, roots.claude); + if (path === null) return none("session log not found — compacted, rotated or removed"); + + let size: number; + try { + size = Bun.file(path).size; + } catch (err) { + return none(`could not read the session log: ${String(err)}`); + } + + const end = before ?? size; + if (end <= 0) return { lines: [], source: "journal", hasMore: false, cursor: null, detail: null }; + + const { text, startByte } = await tailChunk(path, end, MAX_TAIL_BYTES); + // The first line of a tail read is usually a PARTIAL record. Dropping it + // is correct rather than lossy: the next page, which starts earlier, + // contains it whole. + const usable = startByte > 0 ? text.slice(text.indexOf("\n") + 1) : text; + const entries = adapter.parse(usable).slice(-limit); + return { + lines: toLines(entries), + source: "journal", + hasMore: startByte > 0, + cursor: startByte > 0 ? String(startByte) : null, + detail: null, + }; + }, + }; +} + +/** Roots for the harnesses paddock reads, from the real environment. */ +export function defaultRoots(env: Record, home: string): JournalRoots { + return { claude: claudeRoots(env, home) }; +} diff --git a/src/server/routes.ts b/src/server/routes.ts index 08947ac..590af53 100644 --- a/src/server/routes.ts +++ b/src/server/routes.ts @@ -18,8 +18,10 @@ import { setCookie } from "@server/tunnel/gate"; import { EMBEDDED } from "@server/embedded"; import { allowWrite, hostOf, refusalReason } from "@server/origin"; import { warn } from "@server/term"; +import type { JournalReader } from "@server/journal/read"; import { isNavKey, type NotifyTrigger, type SettingsPatch } from "@shared/types"; import { diffScreens, digestOf } from "@shared/screen"; +import type { HerdrAgentSession } from "@shared/herdr-api"; export interface HealthBody { ok: boolean; @@ -193,6 +195,23 @@ function reportRefusal(origin: string | null, host: string, hosts: readonly stri } } +/** + * A journal that could not be read is quiet in the UI and loud here. + * + * The operator sees the old behaviour — falling back to reconstruction is a + * working dashboard, and a banner for a pane that never had a journal would be + * noise. The host does not get to be quiet: `CLAUDE.md` forbids swallowing + * errors, and "history silently stopped going deeper" is otherwise invisible. + * Once per agent, because it is reported on every page request. + */ +const journalMissesSeen = new Set(); + +function reportJournalMiss(agentId: string, detail: string): void { + if (journalMissesSeen.has(agentId)) return; + journalMissesSeen.add(agentId); + warn(`paddock: no journal history for \`${agentId}\` — ${detail}`); +} + /** * A request body as an object, whatever the client actually sent. * @@ -369,6 +388,10 @@ export interface AppDeps { actions?: HerdrActions; /** Settings store. Omit in tests that only exercise the agent API. */ settings?: SettingsStore; + /** Reads a harness's own session log. Omit in tests that do not exercise it. */ + journal?: JournalReader; + /** The server-side session id for an agent. Never crosses the socket. */ + sessionFor?: (agentId: string) => HerdrAgentSession | null; /** * Clock for `/ack`'s `acknowledgedAt` stamp, every `settings.view()` call * (its `serverNow`), and the mute route's stamped `mutedUntil`. One clock @@ -499,6 +522,42 @@ export function createApp(deps: AppDeps) { return c.json({ ok: true }); }); + /** + * Earlier history from the agent's OWN session log. + * + * Registered unconditionally, like `/ack` and unlike the action routes: + * this reads a file and never touches herdr, so gating it on a herdr + * dependency it does not use would repeat the mistake `/ack`'s comment + * records — the one feature that works without herdr being the one + * visibly broken in `--demo`. + * + * POST, not GET: a cursor in a query string lands in edge access logs. + */ + app.post("/api/agents/:id/history", async (c) => { + const agent = deps.store.snapshot().find((a) => a.agentId === c.req.param("id")); + if (!agent) return c.json({ ok: false, detail: "unknown agent" }, 404); + if (!deps.journal) return c.json({ ok: false, detail: "journal reading is not configured" }, 404); + + const body = await jsonBody(c); + // Refused, never coerced: the cursor is opaque and must be one this + // server issued. Folding garbage to 0 would silently serve the top of + // the file instead of the page the operator asked for. + let before: number | null = null; + if (body.before !== undefined && body.before !== null) { + if (typeof body.before !== "string" || !/^\d+$/.test(body.before)) { + return c.json({ ok: false, detail: "before must be a cursor from a previous response" }, 400); + } + before = Number(body.before); + } + const limit = typeof body.limit === "number" && body.limit > 0 && body.limit <= 200 + ? Math.floor(body.limit) + : 50; + + const page = await deps.journal.read(deps.sessionFor?.(agent.agentId) ?? null, before, limit); + if (page.detail !== null) reportJournalMiss(agent.agentId, page.detail); + return c.json({ ok: true, ...page }); + }); + const pairing = deps.pairing; if (pairing) { /** diff --git a/src/server/supervisor.ts b/src/server/supervisor.ts index 1cef6ae..4017d8a 100644 --- a/src/server/supervisor.ts +++ b/src/server/supervisor.ts @@ -1,5 +1,6 @@ -import { applyStatusEvent, toAgents, workspaceLabels } from "@server/herdr/adapter"; +import { applyStatusEvent, sessionRefs, toAgents, workspaceLabels } from "@server/herdr/adapter"; import { checkAgentShape, type ShapeVerdict } from "@server/herdr/shape"; +import { hasAdapter } from "@server/journal/registry"; import { EVENT_AGENT_DETECTED, EVENT_PANE_CLOSED, @@ -12,6 +13,7 @@ import { import type { AgentStore, Delta } from "@server/state/store"; import type { HerdrAgentRaw, + HerdrAgentSession, HerdrEvent, HerdrStatusChanged, HerdrWorkspaceRaw, @@ -87,6 +89,12 @@ export class Supervisor { // moved during that await — see resubscribe(). private subscriptionGeneration = 0; + private sessions = new Map(); + + sessionFor(agentId: string): HerdrAgentSession | null { + return this.sessions.get(agentId) ?? null; + } + constructor(private readonly opts: SupervisorOptions) { this.now = opts.now ?? Date.now; this.reconcileMs = opts.reconcileMs ?? 30_000; @@ -258,7 +266,12 @@ export class Supervisor { hostId: this.opts.store.hostId, labels: this.labels, now, + hasJournal: hasAdapter, }); + // Server-side only: the ids these hold are filesystem keys and must not + // travel with the agent. Replaced wholesale each reconcile, so a closed + // pane's id does not linger. + this.sessions = sessionRefs(list.agents ?? []); const delta = this.opts.store.replaceAll(agents, now); if (delta.upserted.length || delta.removedIds.length) this.opts.onDelta(delta); diff --git a/tests/journal-route.test.ts b/tests/journal-route.test.ts new file mode 100644 index 0000000..bcc4fd8 --- /dev/null +++ b/tests/journal-route.test.ts @@ -0,0 +1,114 @@ +import { expect, test } from "bun:test"; +import { createApp } from "@server/routes"; +import { AgentStore } from "@server/state/store"; +import { Hub } from "@server/ws/hub"; +import type { Agent } from "@shared/types"; +import type { JournalPage } from "@server/journal/read"; + +const NOW = 1_700_000_000_000; +const health = () => ({ + ok: true, hostId: "dev-box", agents: 1, clients: 0, herdrConnected: true, + lastEventAt: NOW, lastNotifyError: null, version: "0.0.0-dev", latestKnown: null, + herdrProtocol: null, schemaWarning: null, +}); + +function agent(over: Partial = {}): Agent { + return { + hostId: "dev-box", agentId: "w1:p1", name: "docs-cleanup", + task: "Tidy the README", state: "working", workspaceId: "w1", + workspaceLabel: "docs", cwd: "/srv/project", stateSince: NOW, updatedAt: NOW, + acknowledgedAt: null, hasJournal: true, ...over, + }; +} + +function harness(page: JournalPage = { lines: ["you · 13:04", "hi", ""], source: "journal", hasMore: true, cursor: "120", detail: null }) { + const store = new AgentStore("dev-box"); + store.replaceAll([agent()], NOW); + const calls: unknown[] = []; + const app = createApp({ + store, now: () => NOW, health, hub: new Hub({ now: () => NOW }), + sessionFor: () => ({ agent: "claude", kind: "id", source: "herdr:claude", value: "u1" }), + journal: { async read(_s, before, limit) { calls.push({ before, limit }); return page; } }, + }); + return { app, calls }; +} + +const post = (app: ReturnType, body: object) => + app.request("/api/agents/w1:p1/history", { + method: "POST", + headers: { "content-type": "application/json", origin: "http://localhost" }, + body: JSON.stringify(body), + }); + +test("returns lines, provenance and a cursor", async () => { + const { app } = harness(); + const res = await post(app, {}); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.lines).toEqual(["you · 13:04", "hi", ""]); + expect(body.source).toBe("journal"); + expect(body.hasMore).toBe(true); + expect(body.cursor).toBe("120"); +}); + +test("the cursor is passed through as a number", async () => { + const { app, calls } = harness(); + await post(app, { before: "120", limit: 25 }); + expect(calls[0]).toEqual({ before: 120, limit: 25 }); +}); + +test("a non-numeric cursor is refused rather than coerced", async () => { + // The cursor is opaque to the client and MUST be one this server issued. + // Coercing garbage to 0 would silently serve the top of the file instead. + const { app } = harness(); + expect((await post(app, { before: "../etc" })).status).toBe(400); +}); + +test("an unknown agent is 404, not an empty page", async () => { + const { app } = harness(); + const res = await app.request("/api/agents/nope/history", { + method: "POST", + headers: { "content-type": "application/json", origin: "http://localhost" }, + body: "{}", + }); + expect(res.status).toBe(404); +}); + +test("no journal reports reconstruction with a reason, and 200", async () => { + // The UI falls back quietly, so this is a normal answer rather than an error + // — but the reason still travels, because nothing may be swallowed. + const { app } = harness({ + lines: [], source: "reconstruction", hasMore: false, cursor: null, + detail: "no journal adapter for this harness", + }); + const res = await post(app, {}); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.source).toBe("reconstruction"); + expect(body.lines).toEqual([]); + expect(body.detail).toContain("no journal"); +}); + +test("the route exists with no actions dep — it never touches herdr", async () => { + // Registered unconditionally, unlike the action routes. Gating a + // filesystem read on a herdr dependency is the /ack mistake: the one + // feature that works without herdr being the one broken in --demo. + const store = new AgentStore("dev-box"); + store.replaceAll([agent()], NOW); + const app = createApp({ + store, now: () => NOW, health, hub: new Hub({ now: () => NOW }), + sessionFor: () => null, + journal: { async read() { return { lines: [], source: "reconstruction" as const, hasMore: false, cursor: null, detail: "no session" }; } }, + }); + expect((await post(app, {})).status).toBe(200); +}); + +test("the same-origin gate covers it like any other POST", async () => { + const { app } = harness(); + const res = await app.request("/api/agents/w1:p1/history", { + method: "POST", + headers: { "content-type": "application/json", origin: "https://evil.example" }, + body: "{}", + }); + expect(res.status).toBe(403); +}); From 3b8d625c9caf4310328e68df42aa5dad2f413fae Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 22:33:18 +0700 Subject: [PATCH 14/22] fix: a cursor must be a record boundary, and a page nobody can read must say so MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pagination reported `hasMore` from BYTE truncation while cutting the page by TURN count, which lost history three ways: a file the window covered whole said "no more" after `.slice(-limit)` had dropped the rest; a record straddling the tail-read boundary was dropped twice, as a partial head here and a corrupt tail on the next page; and every entry between the chunk start and the last `limit` of them was never revisited, because the cursor pointed at the chunk rather than at where the limit actually cut. The cursor is now a record boundary. The walk goes backwards from `end` — always exact, being either the real file size or a cursor this function issued — and stops when `limit` entries are collected or the chunk's lines run out, so the cursor is the first byte of whichever record was consumed last. Measuring forward from `startByte` cannot do this: a tail read begins at an arbitrary byte, so it splits multi-byte characters, and U+FFFD re-encodes to a different length than the bytes it replaced — measured at +2 to +4 per cut, which puts the cursor inside a record instead of on it. One window has no boundary to offer: a record wider than MAX_TAIL_BYTES fills it with no "\n" to cut on, and Claude Code writes several-hundred-KB tool_result records routinely. Stepping to the window's start is the only way past a record a bounded read cannot serve, so that is what it does — and it now says so. Serving that as an empty page with `hasMore: true` and no detail is a "show earlier" tap that does nothing, hides every turn recorded before the oversized record, and logs nothing anywhere. Also: the read inside `tailChunk` is caught alongside the existing `.size` catch. `size` only stats the file, so a log rotated or made unreadable between the two threw out of the route as a 500 with no detail — a broken dashboard where "no history for this agent" was meant. Tests, in a new tests/journal-read.test.ts because the route's own tests fake the reader and cannot express a losslessness property: pages concatenated oldest-first equal one whole-file parse, under and over the window; hasMore is true whenever earlier turns remain; every cursor is a real record boundary on a file whose cut is forced mid-character; an oversized record is reported and paged past; an unreadable log is a detail, not a throw. The unreadable case uses a directory rather than `chmod 0o000`, which root ignores — that would pass locally and fail in a CI running as root. bun test 967 pass, 0 fail, 100 files make check clean make check-clean clean Verified live against herdr 0.8.2 with seven real panes, not fixtures: - 15 real session logs, 33 KB to 37 MB, each paged to the beginning: 194 pages, 43,456 lines, no stall, every cursor a true record boundary, and each file's pages equal to a whole-file parse minus exactly the records too wide to serve. Two of the fifteen hit the oversized-record branch — 13 and 15 pages of it — so that path is ordinary, not hypothetical. - All seven live agents paged to the beginning through POST /api/agents/:id/history, cursors strictly descending. - One 5.4 MB log, 17 pages through the route, byte-identical to a single whole-file parse. Co-Authored-By: Claude Opus 5 --- src/server/journal/read.ts | 116 ++++++++++++++-- tests/journal-read.test.ts | 262 +++++++++++++++++++++++++++++++++++++ 2 files changed, 369 insertions(+), 9 deletions(-) create mode 100644 tests/journal-read.test.ts diff --git a/src/server/journal/read.ts b/src/server/journal/read.ts index afc14fe..2e265aa 100644 --- a/src/server/journal/read.ts +++ b/src/server/journal/read.ts @@ -1,7 +1,7 @@ import { adapterFor } from "@server/journal/registry"; import { claudeRoots, MAX_TAIL_BYTES, tailChunk } from "@server/journal/files"; import { toLines } from "@server/journal/text"; -import type { JournalRoots } from "@server/journal/types"; +import type { JournalEntry, JournalRoots } from "@server/journal/types"; import type { HerdrAgentSession } from "@shared/herdr-api"; export interface JournalPage { @@ -50,17 +50,115 @@ export function createJournalReader(roots: JournalRoots): JournalReader { const end = before ?? size; if (end <= 0) return { lines: [], source: "journal", hasMore: false, cursor: null, detail: null }; - const { text, startByte } = await tailChunk(path, end, MAX_TAIL_BYTES); - // The first line of a tail read is usually a PARTIAL record. Dropping it - // is correct rather than lossy: the next page, which starts earlier, - // contains it whole. - const usable = startByte > 0 ? text.slice(text.indexOf("\n") + 1) : text; - const entries = adapter.parse(usable).slice(-limit); + let text: string; + let startByte: number; + try { + ({ text, startByte } = await tailChunk(path, end, MAX_TAIL_BYTES)); + } catch (err) { + // Distinct detail from the `.size` failure above: this is "the file + // moved or lost permissions between locate() and the read", not "we + // never got as far as opening it". + return none(`could not read a page of the session log: ${String(err)}`); + } + + /** + * The cursor MUST be a record boundary — the byte where a "\n"-delimited + * line starts — never a raw byte count, and never derived by measuring + * the DECODED text forward from `startByte`. Two things break that: + * + * - `hasMore`/`cursor` used to come from whether the tail read was + * BYTE-truncated, while the page was actually cut off by ENTRY + * COUNT (`.slice(-limit)`). A chunk read in one un-truncated pass + * (small file, or the last chunk of a big one) then reported + * `hasMore: false` even though it held more entries than `limit` + * and had just discarded the rest — false "no more". And when a + * chunk held more entries than `limit`, the discarded interior + * entries were never revisited, because the cursor still pointed at + * the whole chunk's start rather than where `limit` actually cut. + * - `startByte` marks an ARBITRARY byte, not a record boundary — a + * tail read routinely begins mid multi-byte character. Decoding + * that leading fragment replaces the stray bytes with U+FFFD, which + * re-encodes to a DIFFERENT byte length than the original bytes + * occupied. Measuring line lengths forward from `startByte` and + * accumulating them (as this function used to) bakes that + * discrepancy into every offset after it — the cursor lands a few + * bytes inside a record instead of at its start, so a record is + * silently dropped once as a "partial first line" and again as a + * corrupt trailing fragment on the next page's parse. + * + * The fix for both is to walk BACKWARDS from `end`, which is always + * exact — it is either the file's real size or a cursor this same + * function issued, and by induction every cursor this function issues + * is itself exact. Working from that trustworthy edge, a line's byte + * length is measured only once corruption (which exists solely at + * `startByte`, the chunk's near edge) is behind it — so every offset + * computed this way is faithful, right down to the boundary of the + * first line we keep. The walk stops the instant `limit` entries are + * collected OR the chunk's lines run out, and either way `cursor` is + * the absolute offset of the START of whichever line was consumed + * LAST — a genuine, uncorrupted record boundary. + */ + const linesArr = text.split("\n"); + // The line nearest `startByte` is usually a PARTIAL record — a tail + // read begins at an arbitrary byte. Excluding it from consideration + // here is correct rather than lossy: the page this cursor leads to + // reads up to exactly this boundary, so the same bytes arrive as that + // page's OWN last line — complete, not partial — measured from ITS + // exact right edge, and get parsed there instead. + const firstUsable = startByte > 0 ? 1 : 0; + + /** + * A window with no complete record in it at all — which is a real shape, + * not a hypothetical: Claude Code writes tool_result records of several + * hundred KB, and one wider than `MAX_TAIL_BYTES` fills an entire window + * with no "\n" to cut on. + * + * There is no record boundary to hand back, so paging steps to the + * window's own start. That is the ONE cursor this function issues which + * is not a boundary, and it is the only way past a record that cannot be + * read at all — the turns recorded EARLIER than it are still perfectly + * readable, so stopping here would hide all of them. + * + * Said out loud, because the alternative is an empty page with + * `hasMore: true` and no explanation: the operator taps "show earlier", + * sees nothing, and nothing anywhere records why. The oversized record + * is genuinely lost — a bounded read cannot serve it — and a loss this + * feature cannot avoid is exactly the kind it has to report. + */ + if (!linesArr.slice(firstUsable).some((l) => l.trim() !== "")) { + return { + lines: [], + source: "journal", + hasMore: startByte > 0, + cursor: startByte > 0 ? String(startByte) : null, + detail: + `no complete record in the ${MAX_TAIL_BYTES}-byte window ending at byte ${end} — ` + + "a single record larger than the window, or a log that shrank while it was read", + }; + } + + const entries: JournalEntry[] = []; + // Falls back to the chunk's own start when literally nothing below is + // consumable (e.g. the chunk holds only the excluded partial first + // line) — still a real, previously-computed byte offset, so the next + // request makes forward progress instead of looping on `before`. + let oldestOffset = startByte; + let pos = end; + for (let i = linesArr.length - 1; i >= firstUsable; i--) { + const line = linesArr[i]!; + const lineStart = pos - Buffer.byteLength(line, "utf8"); + pos = lineStart - 1; // step back over the "\n" this split consumed + if (line.trim() === "") continue; + entries.unshift(...adapter.parse(line)); + oldestOffset = lineStart; + if (entries.length >= limit) break; + } + return { lines: toLines(entries), source: "journal", - hasMore: startByte > 0, - cursor: startByte > 0 ? String(startByte) : null, + hasMore: oldestOffset > 0, + cursor: oldestOffset > 0 ? String(oldestOffset) : null, detail: null, }; }, diff --git a/tests/journal-read.test.ts b/tests/journal-read.test.ts new file mode 100644 index 0000000..ea8bcbf --- /dev/null +++ b/tests/journal-read.test.ts @@ -0,0 +1,262 @@ +import { expect, test } from "bun:test"; +import { mkdir, mkdtemp, writeFile } from "node:fs/promises"; +import { readFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { claudeAdapter } from "@server/journal/claude"; +import { MAX_TAIL_BYTES } from "@server/journal/files"; +import { toLines } from "@server/journal/text"; +import { createJournalReader } from "@server/journal/read"; +import type { JournalRoots } from "@server/journal/types"; +import type { HerdrAgentSession } from "@shared/herdr-api"; + +const UUID = "f4971cd4-d53b-430a-8fc6-a0d4572103ae"; +const SESSION: HerdrAgentSession = { agent: "claude", kind: "id", source: "herdr:claude", value: UUID }; + +/** One synthetic JSONL record, alternating user/assistant, distinguishable by index. */ +function record(i: number, pad = ""): string { + const timestamp = `2026-08-20T00:00:${String(i).padStart(2, "0")}Z`; + return i % 2 === 0 + ? JSON.stringify({ + type: "user", + timestamp, + message: { role: "user", content: `turn ${i}${pad}` }, + }) + : JSON.stringify({ + type: "assistant", + timestamp, + message: { role: "assistant", content: [{ type: "text", text: `turn ${i}${pad}` }] }, + }); +} + +/** A session log with `n` records, plus a `roots` pointing at its temp project dir. */ +async function journal(n: number, pad = ""): Promise<{ roots: JournalRoots; file: string }> { + const root = await mkdtemp(join(tmpdir(), "paddock-jr-")); + const project = join(root, "docs-cleanup"); + await mkdir(project); + const file = join(project, `${UUID}.jsonl`); + const body = Array.from({ length: n }, (_, i) => record(i, pad)).join("\n") + "\n"; + await writeFile(file, body); + return { roots: { claude: [root] }, file }; +} + +/** Pages backward with `limit` until exhausted, oldest-page-first, guarding against a runaway loop. */ +async function pageAll( + reader: ReturnType, + limit: number, +): Promise<{ pages: string[][]; hasMoreHistory: boolean[] }> { + const pages: string[][] = []; + const hasMoreHistory: boolean[] = []; + let before: number | null = null; + for (let guard = 0; guard < 200; guard++) { + const page = await reader.read(SESSION, before, limit); + expect(page.source).toBe("journal"); + expect(page.detail).toBeNull(); + pages.unshift(page.lines); + hasMoreHistory.unshift(page.hasMore); + if (!page.hasMore) return { pages, hasMoreHistory }; + before = Number(page.cursor); + } + throw new Error("paging did not terminate — cursor is not making progress"); +} + +test("paging backwards is lossless and non-overlapping: reassembled pages equal one whole-file parse", async () => { + // The property that matters: walking every page from the tail back to the + // start must yield exactly what a single parse of the whole file yields — + // same entries, same order, no gaps, no duplicates. This is what all three + // loss modes broke. + const { roots, file } = await journal(25); + const reader = createJournalReader(roots); + + const { pages } = await pageAll(reader, 10); + const paged = pages.flat(); + + const expected = toLines(claudeAdapter.parse(readFileSync(file, "utf8"))); + expect(paged).toEqual(expected); +}); + +test("paging is still lossless across a file bigger than MAX_TAIL_BYTES, with the boundary landing mid-record", async () => { + // Padded so records are large and MAX_TAIL_BYTES lands inside one of them, + // not conveniently on a "\n" — the case that pins loss mode 2 (a record + // straddling the tail-read boundary dropped on both sides of it). + const pad = "x".repeat(3_000); + const recordBytes = Buffer.byteLength(record(0, pad) + "\n", "utf8"); + const n = Math.ceil((MAX_TAIL_BYTES * 2.2) / recordBytes); // several chunks' worth + const { roots, file } = await journal(n, pad); + const reader = createJournalReader(roots); + + const { pages } = await pageAll(reader, 20); + const paged = pages.flat(); + + const expected = toLines(claudeAdapter.parse(readFileSync(file, "utf8"))); + expect(paged).toEqual(expected); +}); + +test("hasMore is true when a whole, un-truncated file still holds more entries than limit", async () => { + // Regression for loss mode 1: hasMore used to be derived from BYTE + // truncation alone, so a file small enough to read in one un-truncated + // tailChunk call always reported hasMore:false — even when `limit` had + // just discarded older turns via `.slice(-limit)`. + const { roots } = await journal(25); + const reader = createJournalReader(roots); + + const first = await reader.read(SESSION, null, 4); + expect(first.source).toBe("journal"); + expect(first.hasMore).toBe(true); + expect(first.cursor).not.toBeNull(); + + // And paging must actually be able to reach the beginning from here. + // `hasMoreHistory` is oldest-page-first (unshifted alongside `pages`), so + // index 0 is the LAST page read — the one that terminated the loop. + const { hasMoreHistory } = await pageAll(reader, 4); + expect(hasMoreHistory[0]).toBe(false); +}); + +test("a tailChunk failure after a successful locate() is a detail, not a throw", async () => { + // `Bun.file(path).size` only STATS the file, so it succeeds on a path the + // read then fails on — the log rotated, replaced or made unreadable between + // the two calls. Unhandled, that threw out of the route as a 500 with no + // `detail`: a broken dashboard where "this agent has no history" was meant. + // + // A DIRECTORY where the log should be, rather than `chmod 0o000`: mode bits + // do not apply to root, so a permissions test passes locally and fails in + // any CI that runs as root — the same local/CI split `docs/gotchas.md` + // already records for test-file ordering. A directory stats like a file + // (4096 bytes, so the `end <= 0` early return is not taken) and fails the + // read for every uid. + const root = await mkdtemp(join(tmpdir(), "paddock-jr-")); + await mkdir(join(root, "docs-cleanup")); + await mkdir(join(root, "docs-cleanup", `${UUID}.jsonl`)); + + const page = await createJournalReader({ claude: [root] }).read(SESSION, null, 10); + expect(page.source).toBe("reconstruction"); + expect(page.lines).toEqual([]); + expect(page.detail).toContain("could not read a page of the session log"); +}); + +/** A log with an arbitrary body, for the two cases `journal(n, pad)` cannot shape. */ +async function journalOf(body: string): Promise<{ roots: JournalRoots; file: string }> { + const root = await mkdtemp(join(tmpdir(), "paddock-jr-")); + const project = join(root, "docs-cleanup"); + await mkdir(project); + const file = join(project, `${UUID}.jsonl`); + await writeFile(file, body); + return { roots: { claude: [root] }, file }; +} + +test("every cursor is a real record boundary, even with multi-byte text", async () => { + // The cursor is a BYTE offset into a file, but it is derived from text that + // was DECODED. A tail read starts at an arbitrary byte, so it can split a + // multi-byte character, and the decoder replaces the stray bytes with + // U+FFFD — three bytes on re-encode, which is not what was on disk. An + // offset measured ACROSS that line is skewed by the difference: +2 to +4 + // bytes per mid-character cut, measured. The cursor then names a byte a + // little inside a record instead of the byte the record starts on. + // + // Not lossy on its own — the skew is positive and smaller than a record, so + // the next page's window still holds the previous record whole — but "the + // cursor is a record boundary" is the whole design, and agent prose is full + // of em dashes and non-Latin text. + // + // The cut lands at `size - MAX_TAIL_BYTES`, so WHETHER it splits a character + // is a function of the file's length. Left to chance this test passes on a + // file whose cut happens to land cleanly — it did, before the padding search + // below — so the file is sized until the cut is known to land mid-character. + const body = (pad: number) => { + const records = []; + let bytes = 0; + for (let i = 0; bytes < MAX_TAIL_BYTES * 1.3; i++) { + records.push(JSON.stringify({ + type: "user", + timestamp: "2026-08-20T00:00:00Z", + message: { role: "user", content: `turn ${i}: ${"日本語".repeat(200)}${"y".repeat(pad)}` }, + })); + bytes += Buffer.byteLength(records[records.length - 1]!, "utf8") + 1; + } + return records; + }; + + /** A UTF-8 continuation byte: 10xxxxxx. Landing here is a split character. */ + const isMidChar = (buf: Buffer, at: number) => (buf[at]! & 0xc0) === 0x80; + + let records: string[] | null = null; + for (let pad = 0; pad < 8; pad++) { + const buf = Buffer.from(body(pad).join("\n") + "\n", "utf8"); + if (isMidChar(buf, buf.length - MAX_TAIL_BYTES)) { records = body(pad); break; } + } + expect(records, "no padding put the tail-read cut inside a character").not.toBeNull(); + + const { roots, file } = await journalOf(records!.join("\n") + "\n"); + const reader = createJournalReader(roots); + + // Where each record REALLY starts, measured off the encoded file. + const starts = new Set([0]); + let at = 0; + for (const r of records!) { + at += Buffer.byteLength(r, "utf8") + 1; + starts.add(at); + } + + const cursors: number[] = []; + let before: number | null = null; + for (let guard = 0; guard < 200; guard++) { + const page = await reader.read(SESSION, before, 40); + if (!page.hasMore) break; + cursors.push(Number(page.cursor)); + before = Number(page.cursor); + } + + expect(cursors.length).toBeGreaterThan(1); + expect(cursors.filter((c) => !starts.has(c))).toEqual([]); + // And the multi-byte content still comes back whole and in order. + expect((await pageAll(reader, 40)).pages.flat()) + .toEqual(toLines(claudeAdapter.parse(readFileSync(file, "utf8")))); +}); + +test("a record larger than the window is reported out loud, and paged past", async () => { + // A chunk that holds no complete line at all. Claude Code writes 600 KB + // tool_result records routinely, so a single record wider than the read + // window is ordinary, not hypothetical. + // + // Two wrong answers to avoid. Handing back the same cursor stalls the + // client on "show earlier" for ever. Handing back a cursor with no + // explanation — which is what deriving it from the chunk start alone does — + // serves empty pages that look like the end of the history, and hides + // everything RECORDED BEFORE the oversized record. So: say so, and keep + // going, because the turns further back are still readable. + const huge = JSON.stringify({ + type: "user", + timestamp: "2026-08-20T00:00:00Z", + message: { role: "user", content: "x".repeat(MAX_TAIL_BYTES + 50_000) }, + }); + expect(Buffer.byteLength(huge, "utf8")).toBeGreaterThan(MAX_TAIL_BYTES); + const older = [record(0), record(1)]; + const newer = [record(2), record(3), record(4)]; + const { roots } = await journalOf([...older, huge, ...newer].join("\n") + "\n"); + const reader = createJournalReader(roots); + + const lines: string[][] = []; + const details: string[] = []; + let before: number | null = null; + let terminated = false; + const seen = new Set(); + for (let guard = 0; guard < 200; guard++) { + const page = await reader.read(SESSION, before, 10); + lines.unshift(page.lines); + if (page.detail !== null) details.push(page.detail); + if (!page.hasMore) { terminated = true; break; } + const next = Number(page.cursor); + if (seen.has(next)) throw new Error(`the cursor stalled at ${next}`); + seen.add(next); + before = next; + } + + expect(terminated).toBe(true); + // Accounted for, not swallowed. + expect(details.length).toBeGreaterThan(0); + expect(details[0]).toContain("larger than"); + // Everything readable on BOTH sides of it is still served, in order. The + // oversized record itself is the only thing missing, and it is the thing + // the detail names. + expect(lines.flat()).toEqual(toLines(claudeAdapter.parse([...older, ...newer].join("\n")))); +}); From 3a4e76e9e94f99a590c9bb307a6218d4ab73546d Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 22:56:00 +0700 Subject: [PATCH 15/22] feat: show earlier reads the agent's own log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires the phone up to the history route from the previous task: an agent with a journal fetches earlier turns from POST /api/agents/:id/history instead of replaying client-side reconstruction, while a plain shell pane keeps exactly the behaviour it had before. The two sources never coexist for one agent (design decision 18) — a journal-capable agent's "Show earlier" state is separate from the reconstructed-history state, not merged with it. The journal page size is its own constant (20 turns), distinct from the reconstructed path's line-counted HISTORY_PAGE, because a turn routinely flattens to several lines and reusing that number would dump 250+ lines on a phone in one tap. source: "reconstruction" is read as "fall back silently," never as an error, since that is the server's normal way of saying it has no journal for this agent. Verified against a live herdr and a live paddock instance through headless Chrome (CDP): tapping "Show earlier" twice grew the pane and fetched exactly one POST to /history per tap, with the scroll distance from the bottom held constant (229px before and after both taps) rather than jumping, and zero live option buttons ever rendered from journal content. A real phone check remains outstanding. Co-Authored-By: Claude Opus 5 --- docs/architecture.md | 22 +++++++ docs/decisions.md | 71 +++++++++++++++++++++ docs/gotchas.md | 11 ++++ docs/roadmap.md | 26 ++++++-- src/shared/types.ts | 21 +++++++ src/web/api.ts | 21 ++++++- src/web/components/AgentTerminal.tsx | 73 ++++++++++++++++++--- tests/journal-terminal.test.tsx | 94 ++++++++++++++++++++++++++++ 8 files changed, 322 insertions(+), 17 deletions(-) create mode 100644 tests/journal-terminal.test.tsx diff --git a/docs/architecture.md b/docs/architecture.md index 99593be..87f5a6a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -53,6 +53,10 @@ One process. No relay hop, no plugin, no polling loop. | `web/components/AgentDetail.tsx` | The detail sheet: one agent's output, and — while `blocked` — its parsed prompt options plus a free-text reply box, fetched on open, on a state change, and on the explicit Refresh control. Split into a stateful `AgentDetail` and a hook-free `AgentDetailView`, so the markup is testable with `renderToStaticMarkup` and no DOM. Mounted with `key={openAgent.agentId}` in `App.tsx` so switching the selected agent unmounts the old instance rather than reusing its in-flight state. Attribution across *time* — one agent's successive prompts — is handled instead by tagging the typed reply and the action result with the prompt they belong to: keying on `state` would unmount the sheet on the very delta a successful answer causes. The result line sits outside the `blocked`-only section for that same reason. | | `web/release-notice.ts` | Whether the new-release banner is still owed, and the only owner of its dismissal key. `shouldShowRelease` is pure and separate from the storage access, because the behaviour worth asserting is that a NEWER release re-shows after an older one was dismissed. Dismissal stores the version, never a boolean — see decision 16. Fails open on a `localStorage` throw, same posture as `install.ts`: this is read during render. | | `web/components/ReleaseBanner.tsx` | "The binary on the host is behind." Distinct from `UpdateBar`, which means "this TAB is running stale JavaScript" and has a button that fixes it — this one names the command instead, because nothing tappable here could update the host. `--accent`, not `--warn`: a new release must not read as urgently as a stale connection. | +| `server/journal/read.ts` | `createJournalReader`, paging an agent's own session log backward from the tail in bounded chunks, in TURNS (`limit`), not lines. Behind `POST /api/agents/:id/history` (`routes.ts`), registered unconditionally like `/ack` — it reads a file and never touches herdr. | +| `server/journal/registry.ts` | `adapterFor` / `hasAdapter`: the single decision site for "does this agent have a readable history." Adding a harness is one entry here plus its adapter module, never a new branch in the route or a condition in the client. | +| `server/journal/claude.ts` | The one adapter in v1: Claude Code's own `.jsonl` transcript format, turned into turns. | +| `server/journal/text.ts` | Truncation, ANSI stripping, tool-call summarising, and `stripMenu` — the same menu-marker strip `prompt-parse.ts` needs, applied here so a stale option row from an old turn can never blend into the live screen (decision 18). | | `web/` | React + Tailwind, single screen. | ## The dependency rule @@ -78,6 +82,24 @@ other module has to. `notify/` (`notifier.ts` plus its `telegram.ts` transport) hangs off `index.ts` as a second leaf, alongside `hub.ts`, not chained into the line above: nothing in `herdr/`, `state/store.ts` or `ws/hub.ts` imports it or knows it exists. + +`journal/` is a second axis beside `herdr/`, not a branch off it. It knows +HARNESSES (Claude Code's own transcript format today), never herdr — nothing +under `server/journal/` imports `server/herdr/*` or `@shared/herdr-api`'s +socket-shaped types, only the one field (`agent_session`) `adapter.ts` maps +out of a herdr payload. `adapter.ts` in turn does not import `server/journal/*` +either: `toAgent`'s `AdaptContext` takes a `hasJournal` PREDICATE, and +`supervisor.ts` (the composition point that already knows both trees) is what +passes `journal/registry.ts`'s `hasAdapter` in. Wiring it as an injected +function rather than an import is what keeps `adapter.ts` — the one file +permitted to know herdr's wire shapes — from also having to know what a +"journal" is. `routes.ts` is the other place the two axes meet: `AppDeps` +carries both a `journal?: JournalReader` and a `sessionFor?:` accessor into +herdr's session-ref map, and the `/history` handler reads across both to turn +an agent id into a page of text. That is the route layer taking a dependency +on each axis it needs, the same way it already depended on `state/store.ts` +and `herdr/actions.ts` before this feature existed — not a new coupling +between `journal/` and `herdr/` themselves. `index.ts` composes the two leaves with `fanOut(hub, notifier)` and passes the result as `Supervisor`'s single `onDelta`, so one delta reaches both without either learning the other is there. It deliberately does not live inside diff --git a/docs/decisions.md b/docs/decisions.md index 09d80fc..7f7d6de 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -377,3 +377,74 @@ session does not silently re-litigate them. anyway means `publicUrl` names a hostname this deployment is not reached on. Telling the second operator to check their proxy would send them to a file that is already correct. + +18. **The journal — flattened server-side, never mixed with reconstruction, + menus stripped, prose only, no session id on the wire, quiet in the UI.** + `POST /api/agents/:id/history` reads a harness's own session log so "Show + earlier" can answer with what was actually said, instead of the client's + best guess from screen snapshots. Six decisions went into it. + + **The journal is flattened server-side; the client only ever sees lines.** + `journal/` returns text, and the terminal renders it the way it renders any + other history — no per-harness knowledge crosses into `web/`. Same + reasoning as `parsePrompt` living in `src/server/`: harness- and + protocol-shaped assumptions stay on the server side of the socket. A + structured-turns payload was considered, so a future conversation view + could reuse the route unchanged, and rejected — it would put a second + renderer, and per-harness rendering rules, in `web/`. Pushing history over + the WebSocket was rejected too: it needs file watching and a per-agent + buffer for every open pane, which is a lot of machinery for an affordance + the operator taps. + + **Journal history and reconstruction never coexist for one agent.** Where + a journal is readable it is the ONLY source above the live screen, and the + reconstructed path (`web/history.ts`) is switched off for that agent. + Where one is not, nothing changes from before this feature. Two sources + for one range means reconciling overlapping text produced by two different + mechanisms — guesswork of exactly the kind this feature exists to remove. + + **One continuous scroll, with menus stripped from journal lines.** Journal + text joins the buffer above the live screen with no labelled divider, and + that cost is stated plainly: those lines are a RECONSTRUCTION RENDERED AS + PROSE, and will not look like the live screen below them — they cannot + reproduce the box drawing and colour the agent actually painted. That + sharp edge is cosmetic and accepted. A different one is not: a journal + turn can contain an old prompt menu — `❯ 1. Yes / 2. No` — which, blended + directly above the live screen with no divider, reads as the question + being asked NOW. `prompt-parse.ts` already records this exact failure + mode in its own scoping comment (a marker left on an already-answered + question reappearing as the live menu's selection), so cursor markers and + option rows are stripped from journal-derived lines before they ever + leave the server (`stripMenu` in `src/server/journal/text.ts`). Only the + live screen may ever render a selectable menu; the client additionally + never treats a journal line as a source of option buttons — those come + from `/prompt` alone. + + **Prose is served; tool output is not.** The journal holds far more than + the screen ever showed: every file the agent read, every command's + output, any secret that passed through either. paddock has no + authentication of its own (decision 3), so what this route serves is + bounded at the source rather than at the gate. Kept: assistant text, and + user text the operator actually typed. Summarised: a `tool_use` becomes + one line (`▸ Bash · `). Dropped entirely: every `tool_result`, + subagent traffic, and thinking blocks. + + **The session id never reaches the browser.** `adapter.ts` maps + `agent_session` into a server-side map of `agentId → session ref`; the + wire type `Agent` gains exactly one field, `hasJournal: boolean`, which is + all the UI needs to choose a history source. A session id is a filesystem + key, and the browser has no use for one paddock could not itself resolve. + + **A missing journal is quiet in the UI and loud on the host.** The + operator sees the old behaviour, not an error: falling back to + reconstruction is a working dashboard, and a red banner for a pane that + never had a journal would be noise for the common case (a plain shell + pane has no journal by definition). The server does not get to be quiet — + `CLAUDE.md` forbids swallowing errors — so each cause (no adapter for + this harness, no session ref from herdr, file missing, permission denied) + logs once per agent on the host and travels in the response's `detail`; + an unparseable line skips that line, never the whole file. On the client, + `source: "reconstruction"` is read as this same signal, not a failure: it + means "the server has no journal for this agent," arrives with + `lines: []`, and the terminal falls back to its existing client-side + reconstruction without surfacing anything to the operator. diff --git a/docs/gotchas.md b/docs/gotchas.md index 5acd94a..5e8dc75 100644 --- a/docs/gotchas.md +++ b/docs/gotchas.md @@ -223,6 +223,17 @@ one, recorded here so they are not reintroduced. `pane_closed`). Matching on the subscribe name for the underscored ones silently never fires. +- **A coding agent's pane has no scrollback to read, at any price.** It runs + on the terminal's alternate screen, which keeps nothing behind the + viewport: every such pane reports `scroll.max_offset_from_bottom: 0`. + Measured against herdr 0.8.0, asking anyway costs ~35 ms per line past the + viewport — 300 lines 10.7 s (past `HERDR_TIMEOUT_MS`), and 500/1000/2000 + lines each ~15.8 s while returning LESS than `visible` returns in 2 ms. The + bytes were never retained; there is no cheaper way to ask herdr for them, + and no larger timeout recovers content that was not kept. This is why + history comes from the harness's own log instead — see + `src/server/journal/`. + ## Build and tooling - **Bun's runtime module resolver does not try `.d.ts` on extensionless diff --git a/docs/roadmap.md b/docs/roadmap.md index efb4e0c..aa8ce15 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -52,6 +52,14 @@ surprise. There is no explicit "show history" control and no hidden second request; see `README.md`'s "What it does not do". + **Narrower since the journal route shipped:** for an agent whose + `hasJournal` is true, "Show earlier" now IS an explicit second request — + `POST /api/agents/:id/history`, reading the harness's own session log + rather than the reconstructed viewport buffer. The two sources never mix + for one agent (`docs/decisions.md` decision 18); this entry's description + stands unchanged for every agent without a journal, which remains most of + them in v2. + - **Stuck-agent detection.** `working` for more than N minutes with no output change is worth surfacing. `pane.output_matched` may serve. - **Preact swap** if first-load size disappoints (~45 KB → ~4 KB gzipped, @@ -197,17 +205,23 @@ surprise. here rather than half-done. **Until then, treat a herdr protocol bump as requiring a manual re-read of `actions.ts` against the live schema.** -- **`MAX_READ_LINES` (2000) is not a usable request against an idle agent.** +- ~~**`MAX_READ_LINES` (2000) is not a usable request against an idle + agent.**~~ *Resolved, by routing around the ceiling rather than raising it.* Scrollback lives on the alternate screen, so herdr recovers it by scrolling the pane: measured on herdr 0.8.0, `recent_unwrapped` costs ~35 ms per line past the viewport — 120 lines took 3.1 s, 300 lines 10.7 s (past `HERDR_TIMEOUT_MS`, so `POST /output` with `lines: 300` fails outright), and 500/1000/2000 lines each took ~15.8 s and returned *less* than `visible` - returns in 2 ms. The clamp bounds the response size, which was its purpose, - but not the wall time, and the ceiling is far above what herdr can actually - serve. Fixing it properly means either a much lower scrollback ceiling or a - transport timeout that scales with the request; both are policy decisions - the read-source fix deliberately did not make on its own. + returns in 2 ms. The clamp bounded the response size, which was its + purpose, but not the wall time, and the ceiling was far above what herdr + could actually serve — no transport timeout or lower ceiling was going to + make asking herdr for it a good idea, because the bytes past the viewport + were never retained at all (`docs/gotchas.md`'s alternate-screen entry). + So this was never fixed by tuning the clamp: `POST /api/agents/:id/history` + reads a journal-capable harness's own session log instead, and + `MAX_READ_LINES` still governs exactly what it always did — the + `visible`/`recent_unwrapped` ceiling for a plain shell pane, which has no + journal to fall back to and is unaffected by this work. - **Nothing guards `index.ts`'s call site for the delta fan-out.** `fanOut()` in `server/notify/notifier.ts` is covered diff --git a/src/shared/types.ts b/src/shared/types.ts index 9f7c270..acfa0d2 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -195,6 +195,27 @@ export interface KeyResult extends ActionResult { selected?: string | null; } +/** + * Earlier turns from the agent's own session log — the success body of + * `POST /api/agents/:id/history`. + * + * `source: "reconstruction"` is not an error: it is the server saying "I have + * no journal for this agent," and it always arrives with `lines: []` — the + * caller falls back to its existing client-side reconstruction silently, the + * same way it does today. + * + * `cursor` is OPAQUE. The client echoes it back as the next request's + * `before` and never constructs one; the server refuses anything that is not + * a run of digits with a 400. + */ +export interface HistoryResult { + lines: string[]; + source: "journal" | "reconstruction"; + hasMore: boolean; + cursor: string | null; + detail: string | null; +} + /** * A read response, which may say "nothing changed" instead of resending. * diff --git a/src/web/api.ts b/src/web/api.ts index 3f1ba90..9094253 100644 --- a/src/web/api.ts +++ b/src/web/api.ts @@ -1,4 +1,4 @@ -import type { ActionResult, KeyResult, NavKey, OutputResult, ParsedPrompt } from "@shared/types"; +import type { ActionResult, HistoryResult, KeyResult, NavKey, OutputResult, ParsedPrompt } from "@shared/types"; /** * Just the call signature these helpers use — not `typeof fetch`. @@ -68,6 +68,25 @@ export async function fetchPrompt(id: string, f: Fetch = fetch) { return readJson(url(id, "prompt"), {}, f); } +/** + * Earlier history from the agent's own session log. + * + * `before` is the OPAQUE cursor from a previous response's `cursor`, echoed + * back verbatim — never constructed here — or `null` for the newest page. + * `limit` counts TURNS, not lines; see `JOURNAL_PAGE_TURNS` in + * `AgentTerminal.tsx` for why that unit — and that page size — differs from + * the reconstructed-scrollback path's `HISTORY_PAGE`. + * + * A non-2xx response (unknown agent, malformed cursor) rejects, same as + * every other read — see `readJson`'s note on why a failure must not resolve + * with a value shaped like success. + */ +export async function fetchHistory( + id: string, before: string | null, limit: number, f: Fetch = fetch, +): Promise { + return readJson(url(id, "history"), { before, limit }, f); +} + /** * Every action funnels failures into an ActionResult rather than throwing. * A refused answer ("someone answered at the desk first") is information the diff --git a/src/web/components/AgentTerminal.tsx b/src/web/components/AgentTerminal.tsx index 99a5d03..a680603 100644 --- a/src/web/components/AgentTerminal.tsx +++ b/src/web/components/AgentTerminal.tsx @@ -1,6 +1,6 @@ import { Fragment, useCallback, useEffect, useRef, useState, type CSSProperties } from "react"; import type { ActionResult, Agent, NavKey, OutputResult, ParsedPrompt } from "@shared/types"; -import { answerWithKey, fetchOutput, fetchPrompt, sendKey, sendText } from "@web/api"; +import { answerWithKey, fetchHistory, fetchOutput, fetchPrompt, sendKey, sendText } from "@web/api"; import { parseAnsi, type AnsiSpan } from "@web/ansi"; import { groupLines } from "@web/lines"; import { StateDot } from "@web/components/AgentRow"; @@ -138,9 +138,24 @@ export function nextRefreshMs(current: number, changed: boolean, floor: number = return Math.min(MAX_REFRESH_MS, Math.round(current * REFRESH_BACKOFF)); } -/** Settled lines revealed per tap of "show earlier". */ +/** Settled lines revealed per tap of "show earlier" for a plain shell pane. */ const HISTORY_PAGE = 200; +/** + * Earlier turns fetched per tap of "show earlier" for an agent with a + * journal — `fetchHistory`'s `limit`. + * + * Counted in TURNS, not lines — deliberately its own constant rather than + * reusing `HISTORY_PAGE` above, which counts LINES for the reconstructed- + * scrollback path. A single assistant turn routinely flattens to several + * lines, so a page size chosen the way `HISTORY_PAGE` was (as a count of + * lines) would ask for far more prose than it looks like: 50 turns lands + * 250+ lines in one tap — a wall dumped on a phone screen, not the + * thumb-flick "show earlier" is meant to be. 20 keeps one tap's growth in + * the same ballpark as what the reconstructed path already reveals. + */ +const JOURNAL_PAGE_TURNS = 20; + export interface AgentTerminalProps { agent: Agent; onBack: () => void; @@ -173,6 +188,13 @@ export function AgentTerminal({ agent, onBack }: AgentTerminalProps) { // a pane stays open. const [fontPx] = useState(() => readPrefs().fontPx); const [shownHistory, setShownHistory] = useState(0); + // Journal-sourced lines, oldest first, and the cursor for the next page. + // Kept separate from `history.settled` because the two sources never mix + // for one agent (design decision 2) — this is WHICH ONE is in play for + // this agent, not something merged with the reconstructed path. + const [journalLines, setJournalLines] = useState([]); + const [journalCursor, setJournalCursor] = useState(null); + const [journalDone, setJournalDone] = useState(false); const [prompt, setPrompt] = useState(null); /** @@ -459,9 +481,15 @@ export function AgentTerminal({ agent, onBack }: AgentTerminalProps) { // before until "show earlier" is tapped, which is what keeps a 2000-line // history from becoming 36,000 DOM nodes nobody asked for. const history = historyFor(agent.agentId) ?? { settled: [], gaps: 0 }; - const revealed = shownHistory > 0 - ? history.settled.slice(Math.max(0, history.settled.length - shownHistory)) - : []; + // Design decision 2: the two sources never coexist for one agent. A + // journal-capable agent's revealed history is exactly the journal pages + // fetched so far; everything else keeps today's client-side reconstruction + // unchanged. + const revealed = agent.hasJournal + ? journalLines + : shownHistory > 0 + ? history.settled.slice(Math.max(0, history.settled.length - shownHistory)) + : []; // parseAnsi carries style ACROSS lines, so it must see history and the live // screen as one sequence — parsing them separately would drop any colour a @@ -562,21 +590,46 @@ export function AgentTerminal({ agent, onBack }: AgentTerminalProps) { content, which would otherwise shove the screen down and lose the operator's place, so the scroll position is pinned across the growth in the handler below. */} - {!error && history.settled.length > revealed.length && ( + {!error && (agent.hasJournal ? !journalDone : history.settled.length > revealed.length) && ( )} diff --git a/tests/journal-terminal.test.tsx b/tests/journal-terminal.test.tsx new file mode 100644 index 0000000..b575a42 --- /dev/null +++ b/tests/journal-terminal.test.tsx @@ -0,0 +1,94 @@ +// FIRST: React reads `document` at import time, so the DOM must exist before +// any component below is imported — see tests/terminal-render.test.tsx. +import "./support/dom"; + +import { afterEach, expect, test } from "bun:test"; +import { AgentTerminal } from "@web/components/AgentTerminal"; +import { digestOf } from "@shared/screen"; +import { agent, render, settle, stubFetch, unmount } from "./support/render"; + +const realFetch = globalThis.fetch; + +afterEach(async () => { + await unmount(); + // A stub left installed leaks into every test file that runs after this one. + globalThis.fetch = realFetch; +}); + +const screenOf = (lines: string[]) => ({ lines, source: "visible", digest: digestOf(lines) }); + +test("an agent with a journal fetches earlier lines instead of reading the cache", async () => { + const { fn, calls } = stubFetch({ + "/output": () => screenOf(["out"]), + "/history": () => ({ + ok: true, + lines: ["you · 13:04", "fix the flaky test", ""], + source: "journal", + hasMore: false, + cursor: null, + detail: null, + }), + }); + globalThis.fetch = fn as typeof fetch; + + const host = await render( + {}} />, + ); + await settle(); + + (host.querySelector(".term-earlier") as HTMLButtonElement).click(); + await settle(); + + expect(calls.some((c) => c.url.endsWith("/history"))).toBe(true); + expect(host.textContent).toContain("fix the flaky test"); +}); + +test("an agent with no journal never calls the route", async () => { + // Nothing regresses for a plain shell pane: it keeps the client-side + // reconstruction it has today. + const { fn, calls } = stubFetch({ + "/output": () => screenOf(["out"]), + }); + globalThis.fetch = fn as typeof fetch; + + const host = await render( + {}} />, + ); + await settle(); + + // There is no reconstructed history yet for a freshly-mounted agent, so + // "Show earlier" is not even offered — nothing to click, and nothing + // fetched either way. + const earlier = host.querySelector(".term-earlier"); + if (earlier) (earlier as HTMLButtonElement).click(); + await settle(); + + expect(calls.some((c) => c.url.endsWith("/history"))).toBe(false); +}); + +test("a journal line carrying a menu cannot render as a live option", async () => { + // Belt and braces over the server's stripMenu: the blend has no divider, so + // a stale "❯ 1. Yes" above the live screen would read as the live prompt. + const { fn } = stubFetch({ + "/output": () => screenOf(["out"]), + "/history": () => ({ + ok: true, + lines: ["agent · 13:06", "❯ 1. Yes", ""], + source: "journal", + hasMore: false, + cursor: null, + detail: null, + }), + }); + globalThis.fetch = fn as typeof fetch; + + const host = await render( + {}} />, + ); + await settle(); + + (host.querySelector(".term-earlier") as HTMLButtonElement).click(); + await settle(); + + expect(host.querySelectorAll("button.term-option")).toHaveLength(0); +}); From 60204a1918b63aaec1880c94f75b06f355a93d07 Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 23:10:34 +0700 Subject: [PATCH 16/22] fix: hasJournal is a hint, source is the answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the previous commit found the fallback it documented did not exist: `revealed` and the button's onClick both gated on the static `agent.hasJournal` prop, never on the per-request `source` the server actually returned. A hasJournal-true pane whose session ref went missing, or whose file was deleted or unreadable, got `source: "reconstruction", lines: []` back and latched `journalDone` — hiding "Show earlier" forever while `revealed` stayed pinned to empty `journalLines`, with `history.settled` never read. Silent and permanent data loss on the one feature this task exists to add. Fixed by deciding the render source per pane from what the server said (`journalFellBack`), not from the hint: on `source: "reconstruction"` the pane now falls back to the reconstructed path entirely and permanently, granting the first page of it immediately so the triggering tap is not wasted. Two more findings from the same review, fixed alongside since they touch the same handler: a rejected `/history` request no longer latches `journalDone` or hides the button — it surfaces through `feedback`, the same channel a failed key press or reply already uses, and leaves the cursor untouched so a retry asks for the same page. And a double-tap on the button now fires exactly one request, guarded by a ref checked synchronously in the handler rather than by state a re-render has not caught up with yet. docs/decisions.md decision 18 and docs/roadmap.md's addendum are corrected to describe the hint/answer split and the surfaced-failure case, rather than asserting a silence the code did not have. Co-Authored-By: Claude Opus 5 --- docs/decisions.md | 49 +++++++---- docs/roadmap.md | 12 ++- src/web/components/AgentTerminal.tsx | 106 +++++++++++++++++++----- tests/journal-terminal.test.tsx | 116 +++++++++++++++++++++++++++ 4 files changed, 244 insertions(+), 39 deletions(-) diff --git a/docs/decisions.md b/docs/decisions.md index 7f7d6de..6ce8aa8 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -431,20 +431,37 @@ session does not silently re-litigate them. **The session id never reaches the browser.** `adapter.ts` maps `agent_session` into a server-side map of `agentId → session ref`; the - wire type `Agent` gains exactly one field, `hasJournal: boolean`, which is - all the UI needs to choose a history source. A session id is a filesystem - key, and the browser has no use for one paddock could not itself resolve. - - **A missing journal is quiet in the UI and loud on the host.** The - operator sees the old behaviour, not an error: falling back to - reconstruction is a working dashboard, and a red banner for a pane that - never had a journal would be noise for the common case (a plain shell - pane has no journal by definition). The server does not get to be quiet — - `CLAUDE.md` forbids swallowing errors — so each cause (no adapter for - this harness, no session ref from herdr, file missing, permission denied) - logs once per agent on the host and travels in the response's `detail`; - an unparseable line skips that line, never the whole file. On the client, + wire type `Agent` gains exactly one field, `hasJournal: boolean`. That + field is a HINT that this pane is worth trying — a property of the + harness, decided once at reconcile time — not a guarantee any given + request will succeed: the session ref can be missing or the file can be + gone even when the harness itself has an adapter. The per-request + `source` on each `/history` response is the actual answer, and + `AgentTerminal` decides which history is "in play" for a pane from that + answer, not from the static hint — an earlier version of this code + rendered off `hasJournal` alone and stranded a pane whose every request + came back `source: "reconstruction"` on permanently empty journal lines, + never reading `history.settled` at all. A session id is a filesystem key + regardless, and the browser has no use for one paddock could not itself + resolve. + + **A missing journal is quiet in the UI and loud on the host — but only + for that specific answer, never for a failed request.** The operator + sees the old behaviour, not an error, when the server answers + `source: "reconstruction"`: falling back to reconstruction is a working + dashboard, and a red banner for a pane that never had a journal would be + noise for the common case (a plain shell pane has no journal by + definition). The server does not get to be quiet — `CLAUDE.md` forbids + swallowing errors — so each cause (no adapter for this harness, no + session ref from herdr, file missing, permission denied) logs once per + agent on the host and travels in the response's `detail`; an unparseable + line skips that line, never the whole file. On the client, `source: "reconstruction"` is read as this same signal, not a failure: it - means "the server has no journal for this agent," arrives with - `lines: []`, and the terminal falls back to its existing client-side - reconstruction without surfacing anything to the operator. + always arrives with `lines: []`, and the pane hands itself over to its + existing client-side reconstruction permanently, without surfacing + anything to the operator. A REJECTED request (a network blip, herdr + itself unreachable) is a different thing entirely and is not silent: it + is neither "no journal" nor "no more history", so it is surfaced the same + way a failed key press or reply already is, the affordance is left in + place for a retry, and the cursor is left exactly where it was so the + retry asks for the same page rather than skipping ahead. diff --git a/docs/roadmap.md b/docs/roadmap.md index aa8ce15..ff9a915 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -55,10 +55,14 @@ surprise. **Narrower since the journal route shipped:** for an agent whose `hasJournal` is true, "Show earlier" now IS an explicit second request — `POST /api/agents/:id/history`, reading the harness's own session log - rather than the reconstructed viewport buffer. The two sources never mix - for one agent (`docs/decisions.md` decision 18); this entry's description - stands unchanged for every agent without a journal, which remains most of - them in v2. + rather than the reconstructed viewport buffer. `hasJournal` is only a + hint, though: if the server's per-request answer for that pane ever comes + back `source: "reconstruction"` (no session ref, a missing or unreadable + file), the pane falls back to exactly the mechanism this entry describes, + permanently, for the rest of that pane's life — see + `docs/decisions.md` decision 18. This entry's description stands unchanged + for every agent without a journal, which remains most of them in v2, and + for any journal-hinted agent that has fallen back. - **Stuck-agent detection.** `working` for more than N minutes with no output change is worth surfacing. `pane.output_matched` may serve. diff --git a/src/web/components/AgentTerminal.tsx b/src/web/components/AgentTerminal.tsx index a680603..320f73a 100644 --- a/src/web/components/AgentTerminal.tsx +++ b/src/web/components/AgentTerminal.tsx @@ -190,11 +190,40 @@ export function AgentTerminal({ agent, onBack }: AgentTerminalProps) { const [shownHistory, setShownHistory] = useState(0); // Journal-sourced lines, oldest first, and the cursor for the next page. // Kept separate from `history.settled` because the two sources never mix - // for one agent (design decision 2) — this is WHICH ONE is in play for + // for one agent (design decision 18) — this is WHICH ONE is in play for // this agent, not something merged with the reconstructed path. const [journalLines, setJournalLines] = useState([]); const [journalCursor, setJournalCursor] = useState(null); + // "No more JOURNAL pages" — distinct from having fallen back to + // reconstruction below. Sets only on a genuine `hasMore: false` from a + // `source: "journal"` response. const [journalDone, setJournalDone] = useState(false); + /** + * `agent.hasJournal` is a HINT that this pane is worth trying — it is a + * property of the harness, decided once at reconcile time. `source` on + * each `/history` response is the ANSWER for this pane, decided per + * request: the session ref can be missing, the file can be gone or + * unreadable, even though the harness itself has a journal adapter (see + * decision 18's "quiet in the UI, loud on the host" cases). Rendering off + * the hint alone stranded the operator on a pane whose every response came + * back `source: "reconstruction", lines: []` — `journalDone` latched true + * and `revealed` stayed pinned to the empty `journalLines` forever, with + * `history.settled` never read. This flips permanently false→true the + * first time a response says so, and once it does this pane behaves + * EXACTLY like a journal-less one from then on — the two sources still + * never coexist, decided here instead of from the static prop. + */ + const [journalFellBack, setJournalFellBack] = useState(false); + // Guards the in-flight `/history` request against a double-tap on the + // button re-firing it with the same (not yet advanced) cursor. A REF, not + // just the `journalBusy` state below: two synchronous `click()`s land + // before React re-renders to reflect a state update, so only a ref read + // synchronously inside the handler can see the first click's effect + // before the second one runs. + const journalBusyRef = useRef(false); + // Mirrors the ref, so the button can be visually `disabled` while a + // request is in flight — the ref alone has no way to trigger a re-render. + const [journalBusy, setJournalBusy] = useState(false); const [prompt, setPrompt] = useState(null); /** @@ -481,11 +510,14 @@ export function AgentTerminal({ agent, onBack }: AgentTerminalProps) { // before until "show earlier" is tapped, which is what keeps a 2000-line // history from becoming 36,000 DOM nodes nobody asked for. const history = historyFor(agent.agentId) ?? { settled: [], gaps: 0 }; - // Design decision 2: the two sources never coexist for one agent. A - // journal-capable agent's revealed history is exactly the journal pages - // fetched so far; everything else keeps today's client-side reconstruction - // unchanged. - const revealed = agent.hasJournal + // Design decision 18: the two sources never coexist for one agent, but + // which one is "in play" is decided by what the SERVER has answered for + // this pane (`journalFellBack`), not by the static `hasJournal` hint — + // see the comment on `journalFellBack` above for why that distinction is + // load-bearing. Once a pane has fallen back, it reveals reconstructed + // history exactly like a journal-less agent always has. + const useJournal = agent.hasJournal && !journalFellBack; + const revealed = useJournal ? journalLines : shownHistory > 0 ? history.settled.slice(Math.max(0, history.settled.length - shownHistory)) @@ -590,44 +622,80 @@ export function AgentTerminal({ agent, onBack }: AgentTerminalProps) { content, which would otherwise shove the screen down and lose the operator's place, so the scroll position is pinned across the growth in the handler below. */} - {!error && (agent.hasJournal ? !journalDone : history.settled.length > revealed.length) && ( + {!error && (useJournal ? !journalDone : history.settled.length > revealed.length) && ( diff --git a/tests/journal-terminal.test.tsx b/tests/journal-terminal.test.tsx index b575a42..0691726 100644 --- a/tests/journal-terminal.test.tsx +++ b/tests/journal-terminal.test.tsx @@ -5,6 +5,7 @@ import "./support/dom"; import { afterEach, expect, test } from "bun:test"; import { AgentTerminal } from "@web/components/AgentTerminal"; import { digestOf } from "@shared/screen"; +import { rememberHistory } from "@web/pane-cache"; import { agent, render, settle, stubFetch, unmount } from "./support/render"; const realFetch = globalThis.fetch; @@ -92,3 +93,118 @@ test("a journal line carrying a menu cannot render as a live option", async () = expect(host.querySelectorAll("button.term-option")).toHaveLength(0); }); + + +test("a journal-hinted agent whose /history answers reconstruction falls back, not blank", async () => { + // Correction 4, and the review's Critical finding: `hasJournal: true` is a + // HINT that this pane is worth trying, not a guarantee the server can + // actually read it — the session ref can be missing, the file can be gone, + // even though the harness has an adapter. `source: "reconstruction"` is the + // server saying so, and the pane must hand itself over to the reconstructed + // path entirely rather than staying pinned to empty journal lines forever. + // + // Seeded well past HISTORY_PAGE (200) so the granted first page of + // reconstruction does NOT exhaust `history.settled` — the button must stay + // available afterward, showing the reconstruction's own remaining count. + const seeded = Array.from({ length: 205 }, (_, i) => `old line ${i}`); + rememberHistory("j4:p1", { settled: seeded, gaps: 0 }); + + const { fn } = stubFetch({ + "/output": () => screenOf(["out"]), + "/history": () => ({ + ok: true, + lines: [], + source: "reconstruction", + hasMore: false, + cursor: null, + detail: "no session ref for this pane", + }), + }); + globalThis.fetch = fn as typeof fetch; + + const host = await render( + {}} />, + ); + await settle(); + + (host.querySelector(".term-earlier") as HTMLButtonElement).click(); + await settle(); + + // The reconstructed lines rendered — the operator got SOMETHING for this + // tap, not a permanently empty pane. + expect(host.textContent).toContain("old line 204"); + // And the affordance survived: there is more reconstructed history behind + // it, reported the way the reconstructed path always has. + const earlier = host.querySelector(".term-earlier"); + expect(earlier).not.toBeNull(); + expect(earlier?.textContent).toContain("5 lines"); +}); + +test("a rejected /history request is surfaced, not swallowed, and the button survives", async () => { + // The Important finding: a transient failure is not "no more history". + // `journalDone` must not latch, and the pane's main output must not be + // replaced by the full-screen error banner either — this is a failed + // ACTION, the same category as a failed key press or reply, not a failed + // initial load. + const fn = async (input: string | URL | Request) => { + const url = String(input); + if (url.includes("/history")) { + return new Response(JSON.stringify({ ok: false, detail: "herdr unreachable" }), { + status: 502, headers: { "content-type": "application/json" }, + }); + } + return new Response(JSON.stringify(screenOf(["out"])), { + status: 200, headers: { "content-type": "application/json" }, + }); + }; + globalThis.fetch = fn as unknown as typeof fetch; + + const host = await render( + {}} />, + ); + await settle(); + + (host.querySelector(".term-earlier") as HTMLButtonElement).click(); + await settle(); + + expect(host.querySelector(".term-note.warn")?.textContent).toContain("herdr unreachable"); + // Not the full-load error path: the pane itself is still on screen. + expect(host.querySelector(".term-error")).toBeNull(); + // And the operator can still try again — the affordance was not hidden. + expect(host.querySelector(".term-earlier")).not.toBeNull(); +}); + +test("a double-tap on Show earlier fires exactly one request", async () => { + // The Minor finding: ordinary touch behaviour on a phone, and the naive + // handler would fire the fetch twice against the same, not-yet-advanced + // cursor — duplicating the page it prepends. + const { calls, fn } = stubFetch({ + "/output": () => screenOf(["out"]), + "/history": () => ({ + ok: true, + lines: ["you · 09:00", "one turn, fetched once", ""], + source: "journal", + hasMore: true, + cursor: "42", + detail: null, + }), + }); + globalThis.fetch = fn as typeof fetch; + + const host = await render( + {}} />, + ); + await settle(); + + const button = host.querySelector(".term-earlier") as HTMLButtonElement; + // Both fire before React has re-rendered to reflect `disabled` — the + // synchronous ref guard, not the DOM attribute, is what this test pins. + button.click(); + button.click(); + await settle(); + await settle(); + + expect(calls.filter((c) => c.url.endsWith("/history")).length).toBe(1); + const occurrences = (host.textContent?.match(/one turn, fetched once/g) ?? []).length; + expect(occurrences).toBe(1); +}); From 3f6e3136ab649fd2d7c69501d5653b0c65219339 Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 23:33:26 +0700 Subject: [PATCH 17/22] feat: --demo can demonstrate journal history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit README screenshots come from --demo, and until now every seeded demo agent had hasJournal: false, so "Show earlier" reading a real session log had nothing to show there. One agent (flaky-test-fix) now claims a journal and the demo backend's /history branch answers it with a short invented transcript, source: "journal" — matching the wire shape the real route produces and the field the client actually keys its routing on. Every other seeded agent is unchanged. --- src/web/demo/backend.ts | 48 +++++++++++++++++++++++++- tests/demo.test.ts | 75 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 121 insertions(+), 2 deletions(-) diff --git a/src/web/demo/backend.ts b/src/web/demo/backend.ts index e8385d7..6148719 100644 --- a/src/web/demo/backend.ts +++ b/src/web/demo/backend.ts @@ -22,6 +22,9 @@ import { const HOST_ID = "demo-box"; +/** The one seeded agent whose session log the demo can "read" — see `DEMO_HISTORY`. */ +const JOURNAL_AGENT_ID = "d6:p1"; + const SEED: Array<{ id: string; name: string; task: string; state: AgentState; ageMs: number }> = [ { id: "d1:p1", name: "schema-migration", task: "Apply migration to staging", state: "blocked", ageMs: 120_000 }, { id: "d2:p1", name: "lint-config", task: "Align eslint with the style guide", state: "done", ageMs: 300_000 }, @@ -43,9 +46,40 @@ const agents: Agent[] = SEED.map((s) => ({ stateSince: Date.now() - s.ageMs, updatedAt: Date.now(), acknowledgedAt: null, - hasJournal: false, + // Only ONE seeded agent claims a journal. The point of this fixture is to + // demonstrate both paths side by side — "Show earlier" reading a real log + // vs. falling back to client-side reconstruction — not to pretend every + // demo agent has one. + hasJournal: s.id === JOURNAL_AGENT_ID, })); +/** + * A short canned transcript for the one demo agent that has a journal. + * + * Invented content, per house rule 2 — never copied from a real session. It + * exists so `Show earlier` is demonstrable in the mode README screenshots come + * from, rather than being a feature only a live herdr can show. + */ +const DEMO_HISTORY: string[] = [ + "you · 13:04", + "the flaky-test-fix suite times out about one run in five — can you dig in?", + "", + "agent · 13:05", + "▸ Bash · run the suite three times", + "Reproduced it on the second run: the retry budget is exhausted before the " + + "first assertion fires, so the harness treats a slow fixture boot as a failure.", + "", + "you · 13:08", + "is it the fixture or the assertion timeout?", + "", + "agent · 13:09", + "▸ Read · tests/fixtures/upload.ts", + "The fixture waits on a fake clock that only advances on tick(); the suite's " + + "timeout is real wall time. Bumping the tick interval should fix it without " + + "touching the assertion.", + "", +]; + /** Cursor position on the blocked agent's menu, moved by the arrow keys. */ let cursor = 0; /** Screens keyed by agent, so a key press can change what the pane shows. */ @@ -143,6 +177,18 @@ function handle(url: string, body: Record): Response { return json({ ok: true }); } + if (route === "history") { + if (!agent.hasJournal) { + return json({ + ok: true, lines: [], source: "reconstruction", hasMore: false, cursor: null, + detail: "no journal for this demo agent", + }); + } + return json({ + ok: true, lines: DEMO_HISTORY, source: "journal", hasMore: false, cursor: null, detail: null, + }); + } + if (route === "ack") { if (agent.state !== "done" || agent.acknowledgedAt !== null) { return json({ ok: false, detail: "not a fresh done agent" }, 409); diff --git a/tests/demo.test.ts b/tests/demo.test.ts index 64ea86c..17dd7c2 100644 --- a/tests/demo.test.ts +++ b/tests/demo.test.ts @@ -1,7 +1,9 @@ import { expect, test } from "bun:test"; import { createDemoSource, DEMO_HOST_ID, DemoSource, demoAgents } from "@server/demo"; import { AgentStore } from "@server/state/store"; -import type { Agent } from "@shared/types"; +import type { Agent, HistoryResult } from "@shared/types"; +import { installDemoBackend } from "@web/demo/backend"; +import { fetchHistory } from "@web/api"; const NOW = 1_700_000_000_000; @@ -88,3 +90,74 @@ test("the delta browsers receive in demo mode is the store's own", () => { } expect(deltas[0]!.upserted.length).toBeGreaterThan(0); }); + +// ── the browser-only demo backend (GitHub Pages "live demo") ──────────────── +// +// This is a SEPARATE synthetic backend from `@server/demo` above: it replaces +// `fetch`/`WebSocket` in the browser so the static site has no server to talk +// to at all. `installDemoBackend` mutates globals Bun shares across every test +// file in this process, so each test here must restore them afterward — a +// leaked stub `fetch` would break an unrelated test that never asked for one. + +/** Reads the seeded agent list off the snapshot the demo socket sends. */ +async function demoSnapshotAgents(): Promise { + return new Promise((resolve) => { + const Ctor = (globalThis as unknown as { WebSocket: new () => { + onmessage: ((e: { data: string }) => void) | null; + close(): void; + } }).WebSocket; + const socket = new Ctor(); + socket.onmessage = (e) => { + const msg = JSON.parse(e.data) as { type: string; agents?: Agent[] }; + if (msg.type === "snapshot" && msg.agents) { + socket.close(); + resolve(msg.agents); + } + }; + }); +} + +test("one demo agent has a journal, so --demo can demonstrate Show earlier", async () => { + // README screenshots come from --demo. A feature invisible there cannot be + // screenshotted, and the roadmap already records one such gap (the approve + // path). Adding a second silently would be a choice, not an accident. + const savedFetch = globalThis.fetch; + const savedWebSocket = (globalThis as { WebSocket?: unknown }).WebSocket; + try { + installDemoBackend(); + const agents = await demoSnapshotAgents(); + const withJournal = agents.filter((a) => a.hasJournal); + expect(withJournal).toHaveLength(1); + } finally { + globalThis.fetch = savedFetch; + (globalThis as { WebSocket?: unknown }).WebSocket = savedWebSocket; + } +}); + +test("the demo journal uses invented content, and other demo agents are unaffected", async () => { + const savedFetch = globalThis.fetch; + const savedWebSocket = (globalThis as { WebSocket?: unknown }).WebSocket; + try { + installDemoBackend(); + const agents = await demoSnapshotAgents(); + const journalAgent = agents.find((a) => a.hasJournal); + if (!journalAgent) throw new Error("no seeded demo agent has a journal"); + const other = agents.find((a) => !a.hasJournal); + if (!other) throw new Error("expected at least one demo agent without a journal"); + + const withJournal: HistoryResult = await fetchHistory(journalAgent.agentId, null, 20); + expect(withJournal.source).toBe("journal"); + expect(withJournal.lines.length).toBeGreaterThan(3); + expect(withJournal.lines.join("\n")).toContain("flaky-test-fix"); + + // A different demo agent must keep behaving exactly as it did before this + // feature existed: no journal, so the client falls back to its own + // reconstruction rather than the server pretending to have one. + const without: HistoryResult = await fetchHistory(other.agentId, null, 20); + expect(without.source).toBe("reconstruction"); + expect(without.lines).toEqual([]); + } finally { + globalThis.fetch = savedFetch; + (globalThis as { WebSocket?: unknown }).WebSocket = savedWebSocket; + } +}); From acfd0a8dde185e53962b1088309925dc8706828c Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Thu, 20 Aug 2026 23:44:39 +0700 Subject: [PATCH 18/22] feat: the CLI --demo can demonstrate journal history too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLAUDE.md names `paddock --demo` as where README screenshots come from, but its /history route always answered source: "reconstruction" — demo mode never constructs a Supervisor, so sessionFor had nothing to ask and the real journal reader got session: null every time, regardless of any agent's hasJournal flag. The static-build demo (web/demo/backend.ts) fixed for its own host in the previous commit was a real gap on its own, but it did not close this one: they are two independent demo backends. server/demo.ts now exports demoJournalPage/demoSessionFor, the demo's own answer for /history, confined entirely to index.ts's DEMO branch so a demo run never touches a real session log. The invented transcript and the id of the one agent that gets it now live in shared/demo-history.ts, imported by both demo hosts, so they tell the same story instead of two that could drift. Verified live: `bun src/server/index.ts --demo` now answers source: "journal" with the shared transcript for flaky-test-fix and source: "reconstruction" for every other seeded agent. Also found and confirmed pre-existing (not introduced here, not fixed here): CLI --demo's /output, /prompt and /answer are only registered when HerdrActions is present, which demo mode never sets — already documented in docs/roadmap.md as the "approve path" gap — so the terminal pane itself, and therefore the Show earlier button, cannot render in a browser against CLI --demo regardless of this fix. The static-build demo has no such gap and is where the button is actually screenshotable today; the /history contract itself is confirmed correct on both hosts. Co-Authored-By: Claude Opus 5 --- src/server/demo.ts | 59 ++++++++++++++++++++++++++++++++++- src/server/index.ts | 29 +++++++++++++++--- src/shared/demo-history.ts | 44 ++++++++++++++++++++++++++ src/web/demo/backend.ts | 39 ++++------------------- tests/demo.test.ts | 63 +++++++++++++++++++++++++++++++++++++- 5 files changed, 195 insertions(+), 39 deletions(-) create mode 100644 src/shared/demo-history.ts diff --git a/src/server/demo.ts b/src/server/demo.ts index 5900c08..9a70307 100644 --- a/src/server/demo.ts +++ b/src/server/demo.ts @@ -1,4 +1,6 @@ import type { Agent, AgentState } from "@shared/types"; +import type { HerdrAgentSession } from "@shared/herdr-api"; +import { DEMO_JOURNAL_AGENT_ID, DEMO_JOURNAL_LINES } from "@shared/demo-history"; export const DEMO_HOST_ID = "demo-box"; @@ -28,10 +30,65 @@ export function demoAgents(now: number): Agent[] { stateSince: now - s.ageMs, updatedAt: now, acknowledgedAt: null, - hasJournal: false, + // Only ONE seeded agent claims a journal, matching `web/demo/backend.ts` + // (the static build's demo host) exactly — `DEMO_JOURNAL_AGENT_ID` is the + // single shared source of truth for which one, so both demo hosts + // demonstrate the same "Show earlier" story rather than two that could + // drift. `index.ts` wires this agent's `/history` answer in the DEMO + // branch; this flag is only the client-facing hint (decision 18). + hasJournal: s.id === DEMO_JOURNAL_AGENT_ID, })); } +/** + * The shape `server/journal/read.ts`'s real `JournalReader.read` returns — + * matched structurally rather than imported, so demo.ts (which stands in for + * herdr, per the note on `DemoStoreSink` below) does not take on a dependency + * on `journal/`, a separate leaf off the composition root. `index.ts` wraps + * `demoJournalPage` in an object satisfying the real `JournalReader` + * interface — the two shapes agreeing is what TypeScript checks for it. + */ +export interface DemoJournalPage { + lines: string[]; + source: "journal" | "reconstruction"; + hasMore: boolean; + cursor: string | null; + detail: string | null; +} + +/** + * The DEMO's whole answer for `/history`: one agent (`DEMO_JOURNAL_AGENT_ID`) + * gets the shared invented transcript with `source: "journal"` — the field + * the client actually keys its routing on, not the static `hasJournal` hint + * (decision 18) — and every other agent gets the same "no journal" shape the + * real reader sends for a harness with no adapter. Never reads a real file: + * `index.ts` confines this to the `DEMO` branch, the same way demo mode never + * opens a real herdr connection. + * + * Served whole in one page, so `hasMore: false` and `cursor: null` are the + * only self-consistent answer — there is no second page to point `cursor` at. + */ +export function demoJournalPage(session: HerdrAgentSession | null | undefined): DemoJournalPage { + if (!session || session.value !== DEMO_JOURNAL_AGENT_ID) { + return { + lines: [], source: "reconstruction", hasMore: false, cursor: null, + detail: "no journal for this demo agent", + }; + } + return { lines: DEMO_JOURNAL_LINES, source: "journal", hasMore: false, cursor: null, detail: null }; +} + +/** + * The DEMO's `sessionFor`: a synthetic session ref for the one journal agent, + * `null` for every other — mirroring what a real `Supervisor.sessionFor` + * would answer, without a real herdr connection to ask. + */ +export function demoSessionFor(id: string): HerdrAgentSession | null { + return id === DEMO_JOURNAL_AGENT_ID + ? { agent: "demo", kind: "id", source: "demo", value: id } + : null; +} + interface DemoDelta { upserted: Agent[]; removedIds: string[]; diff --git a/src/server/index.ts b/src/server/index.ts index f2cd263..5974478 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -2,7 +2,7 @@ import { readFileSync, statSync } from "node:fs"; import { homedir } from "node:os"; import { join } from "node:path"; import { createApp } from "@server/routes"; -import { createDemoSource, DemoSource, DEMO_HOST_ID } from "@server/demo"; +import { createDemoSource, DemoSource, DEMO_HOST_ID, demoJournalPage, demoSessionFor } from "@server/demo"; import { HerdrStream, ProtocolMismatchError, @@ -14,7 +14,7 @@ import { createActions, type HerdrActions } from "@server/herdr/actions"; import { StreamKeeper } from "@server/herdr/keeper"; import { AgentStore } from "@server/state/store"; import { Supervisor } from "@server/supervisor"; -import { createJournalReader, defaultRoots } from "@server/journal/read"; +import { createJournalReader, defaultRoots, type JournalReader } from "@server/journal/read"; import { shapeMessage, shapeSummary } from "@server/herdr/shape"; import { Hub } from "@server/ws/hub"; import { hubWebSocket, tryUpgradeWs, type WsData } from "@server/ws/serve"; @@ -511,14 +511,35 @@ if (DEMO) { */ const publicHosts = () => publicHostsFrom(settings.current().publicUrl, tunnelUrl); +/** + * The DEMO's own `JournalReader` — confined to the `DEMO` branch below, never + * touching a real session log. `--demo` is the mode README screenshots come + * from (CLAUDE.md), and until this existed every seeded demo agent answered + * `source: "reconstruction"` unconditionally: `sessionFor` had no supervisor + * to ask (demo mode never constructs one), so the real `createJournalReader` + * always got `session: null` — the exact gap this wiring closes. + * + * The decision itself (`demoJournalPage`) lives in `@server/demo`, alongside + * `demoAgents`'s matching `hasJournal` flag and the SAME shared transcript + * `web/demo/backend.ts` serves for the static build, so both demo hosts tell + * one story. This object only adapts that decision to the real + * `JournalReader` interface `routes.ts` expects. + */ +const demoJournal: JournalReader = { + read: (session) => Promise.resolve(demoJournalPage(session)), +}; + const appDeps = { store, publicHosts, hub, actions, settings, - journal: createJournalReader(defaultRoots(process.env, homedir())), - sessionFor: (id: string) => supervisor?.sessionFor(id) ?? null, + // Confined to the DEMO branch: a demo run must never read a real journal + // off the operator's own disk, the same reasoning that keeps demo mode + // from opening a real herdr connection. + journal: DEMO ? demoJournal : createJournalReader(defaultRoots(process.env, homedir())), + sessionFor: (id: string) => (DEMO ? demoSessionFor(id) : (supervisor?.sessionFor(id) ?? null)), health: () => ({ ok: true, hostId, diff --git a/src/shared/demo-history.ts b/src/shared/demo-history.ts new file mode 100644 index 0000000..e71f901 --- /dev/null +++ b/src/shared/demo-history.ts @@ -0,0 +1,44 @@ +/** + * The one invented "Show earlier" transcript both demo hosts show. + * + * paddock has two independent demo backends — the CLI's `paddock --demo` + * (`server/demo.ts`, a real server with synthetic agents) and the static + * GitHub Pages build (`web/demo/backend.ts`, a synthetic backend running + * entirely in the browser). Both need to demonstrate journal history for + * screenshots, and both must tell the SAME invented story rather than two + * that could drift apart one edit at a time — so the content lives here, + * imported by both, and neither file declares its own copy. + * + * Every line is invented, per house rule 2 (this repository is public). + * `flaky-test-fix` is the only proper noun, drawn from the approved fixture + * name set, and matches the seeded agent's task ("Stabilise the upload + * suite") in both demo hosts. + */ + +/** The one seeded demo agent, in both hosts, whose session log can be "read". */ +export const DEMO_JOURNAL_AGENT_ID = "d6:p1"; + +/** + * The transcript, already in the shape `toLines` produces + * (`server/journal/text.ts`): a speaker line, an optional tool-summary line, + * prose, then a blank line between turns. + */ +export const DEMO_JOURNAL_LINES: string[] = [ + "you · 13:04", + "the flaky-test-fix suite times out about one run in five — can you dig in?", + "", + "agent · 13:05", + "▸ Bash · run the suite three times", + "Reproduced it on the second run: the retry budget is exhausted before the " + + "first assertion fires, so the harness treats a slow fixture boot as a failure.", + "", + "you · 13:08", + "is it the fixture or the assertion timeout?", + "", + "agent · 13:09", + "▸ Read · tests/fixtures/upload.ts", + "The fixture waits on a fake clock that only advances on tick(); the suite's " + + "timeout is real wall time. Bumping the tick interval should fix it without " + + "touching the assertion.", + "", +]; diff --git a/src/web/demo/backend.ts b/src/web/demo/backend.ts index 6148719..71353b5 100644 --- a/src/web/demo/backend.ts +++ b/src/web/demo/backend.ts @@ -1,5 +1,6 @@ import type { Agent, AgentState, ServerMessage } from "@shared/types"; import { diffScreens, digestOf } from "@shared/screen"; +import { DEMO_JOURNAL_AGENT_ID, DEMO_JOURNAL_LINES } from "@shared/demo-history"; import { blockedScreen, DEMO_OPTIONS, DONE_SCREEN, IDLE_DOCS_SCREEN, SCREENS, WORKING_SCREEN, } from "@web/demo/screens"; @@ -22,9 +23,6 @@ import { const HOST_ID = "demo-box"; -/** The one seeded agent whose session log the demo can "read" — see `DEMO_HISTORY`. */ -const JOURNAL_AGENT_ID = "d6:p1"; - const SEED: Array<{ id: string; name: string; task: string; state: AgentState; ageMs: number }> = [ { id: "d1:p1", name: "schema-migration", task: "Apply migration to staging", state: "blocked", ageMs: 120_000 }, { id: "d2:p1", name: "lint-config", task: "Align eslint with the style guide", state: "done", ageMs: 300_000 }, @@ -49,37 +47,12 @@ const agents: Agent[] = SEED.map((s) => ({ // Only ONE seeded agent claims a journal. The point of this fixture is to // demonstrate both paths side by side — "Show earlier" reading a real log // vs. falling back to client-side reconstruction — not to pretend every - // demo agent has one. - hasJournal: s.id === JOURNAL_AGENT_ID, + // demo agent has one. `DEMO_JOURNAL_AGENT_ID` and the transcript below are + // shared with `server/demo.ts` (the CLI's `--demo` backend) so both hosts + // tell the same invented story rather than two that could drift. + hasJournal: s.id === DEMO_JOURNAL_AGENT_ID, })); -/** - * A short canned transcript for the one demo agent that has a journal. - * - * Invented content, per house rule 2 — never copied from a real session. It - * exists so `Show earlier` is demonstrable in the mode README screenshots come - * from, rather than being a feature only a live herdr can show. - */ -const DEMO_HISTORY: string[] = [ - "you · 13:04", - "the flaky-test-fix suite times out about one run in five — can you dig in?", - "", - "agent · 13:05", - "▸ Bash · run the suite three times", - "Reproduced it on the second run: the retry budget is exhausted before the " + - "first assertion fires, so the harness treats a slow fixture boot as a failure.", - "", - "you · 13:08", - "is it the fixture or the assertion timeout?", - "", - "agent · 13:09", - "▸ Read · tests/fixtures/upload.ts", - "The fixture waits on a fake clock that only advances on tick(); the suite's " + - "timeout is real wall time. Bumping the tick interval should fix it without " + - "touching the assertion.", - "", -]; - /** Cursor position on the blocked agent's menu, moved by the arrow keys. */ let cursor = 0; /** Screens keyed by agent, so a key press can change what the pane shows. */ @@ -185,7 +158,7 @@ function handle(url: string, body: Record): Response { }); } return json({ - ok: true, lines: DEMO_HISTORY, source: "journal", hasMore: false, cursor: null, detail: null, + ok: true, lines: DEMO_JOURNAL_LINES, source: "journal", hasMore: false, cursor: null, detail: null, }); } diff --git a/tests/demo.test.ts b/tests/demo.test.ts index 17dd7c2..f8ee27f 100644 --- a/tests/demo.test.ts +++ b/tests/demo.test.ts @@ -1,5 +1,7 @@ import { expect, test } from "bun:test"; -import { createDemoSource, DEMO_HOST_ID, DemoSource, demoAgents } from "@server/demo"; +import { + createDemoSource, DEMO_HOST_ID, DemoSource, demoAgents, demoJournalPage, demoSessionFor, +} from "@server/demo"; import { AgentStore } from "@server/state/store"; import type { Agent, HistoryResult } from "@shared/types"; import { installDemoBackend } from "@web/demo/backend"; @@ -37,6 +39,65 @@ test("demo agents all belong to the demo host", () => { for (const a of demoAgents(NOW)) expect(a.hostId).toBe(DEMO_HOST_ID); }); +// ── the CLI demo's /history answers (server/demo.ts, `paddock --demo`) ────── +// +// `paddock --demo` is the mode CLAUDE.md names for README screenshots. Its +// `/history` route (`routes.ts`) is registered unconditionally and always +// calls whatever `JournalReader` `index.ts` wires in; in demo mode that used +// to be the REAL reader fed a `sessionFor` with no supervisor to ask, so +// every seeded agent answered `source: "reconstruction"` no matter what +// `hasJournal` said. `demoJournalPage`/`demoSessionFor` are what `index.ts` +// wires in instead, confined to the `DEMO` branch — these tests exercise +// that decision directly, the same way the tests above exercise `demoAgents` +// directly rather than booting the whole server. + +test("exactly one CLI demo agent has a journal", () => { + const withJournal = demoAgents(NOW).filter((a) => a.hasJournal); + expect(withJournal).toHaveLength(1); +}); + +test("the CLI demo journal answers source: journal with the shared invented transcript", () => { + const journalAgent = demoAgents(NOW).find((a) => a.hasJournal); + if (!journalAgent) throw new Error("no seeded demo agent has a journal"); + + const page = demoJournalPage(demoSessionFor(journalAgent.agentId)); + expect(page.source).toBe("journal"); + expect(page.lines.length).toBeGreaterThan(3); + expect(page.lines.join("\n")).toContain("flaky-test-fix"); + // Served whole in one page: no further page for the client to ask for. + expect(page.hasMore).toBe(false); + expect(page.cursor).toBeNull(); +}); + +test("a different CLI demo agent still answers reconstruction, unaffected", () => { + const other = demoAgents(NOW).find((a) => !a.hasJournal); + if (!other) throw new Error("expected at least one demo agent without a journal"); + + expect(demoSessionFor(other.agentId)).toBeNull(); + const page = demoJournalPage(demoSessionFor(other.agentId)); + expect(page.source).toBe("reconstruction"); + expect(page.lines).toEqual([]); +}); + +test("the CLI demo and the static-build demo agree on which agent has the journal", async () => { + // Two independent demo hosts (server/demo.ts for `--demo`, web/demo/backend.ts + // for the static build) must tell the SAME invented story — this is the + // regression `@shared/demo-history` exists to prevent. + const savedFetch = globalThis.fetch; + const savedWebSocket = (globalThis as { WebSocket?: unknown }).WebSocket; + try { + installDemoBackend(); + const staticAgents = await demoSnapshotAgents(); + const staticJournalId = staticAgents.find((a) => a.hasJournal)?.agentId; + const cliJournalId = demoAgents(NOW).find((a) => a.hasJournal)?.agentId; + expect(staticJournalId).toBeDefined(); + expect(cliJournalId).toBe(staticJournalId); + } finally { + globalThis.fetch = savedFetch; + (globalThis as { WebSocket?: unknown }).WebSocket = savedWebSocket; + } +}); + test("tick emits a delta", () => { const seen: Agent[][] = []; const src = new DemoSource({ onDelta: (d) => seen.push(d.upserted), now: () => NOW }); From fd484d49fa1d5c8031b724f083ef5a16fe5dfd4e Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Fri, 21 Aug 2026 00:16:50 +0700 Subject: [PATCH 19/22] fix: a string in a user record is not proof a person typed it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the adapter served any `user` record whose content was a string, on the rule "a STRING is a person typing". the harness writes into that same field. measured against the three largest session logs on this machine: 733 such records would have been served, 176 carrying a `` body (subagent and tool result text), 180 carrying ``/`` blocks, and 453 carrying an absolute home path. 278 of them ran past MAX_TEXT_CHARS and were truncated to 4 KB of that and served anyway. design decision 4 promises prose only and tool results never. two mechanisms, because one is not enough. a NAMED LIST covers the shapes the harness is known to inject — result, task-notification, output-file, system-reminder, local-command-stdout, and the command-name/message/args triple a slash command expands to. a SHAPE RULE covers the rest: hooks, plugins and future harness versions write into the same field with vocabularies nobody has listed, and they all name their blocks in kebab- or snake-case, while the markup in a message a person wrote is html or jsx — a single lowercase word, or pascalcase. so `` and `` go and `
`, ``, `` stay. stripped, then dropped if stripping empties the record. a record that was only an injected block has nothing left worth showing and must not become a bare "you" row; a record that is a typed message with a block appended — the ordinary shape, since the harness appends to whatever was said — is worth keeping minus the block. and stripping happens before the text cap, not after, or a block wider than 4 KB is simply truncated and served. this subsumes the deferred isSidechain finding. that flag is top-level only, and a `` block is exactly how a subagent's output reaches a record the flag cannot see. after the fix, on the same logs: no served user record contains an injected element of any kind, and the 453 home paths fall to 12 — all of them in prose with no markup at all. those are not redacted, deliberately. that is content the operator typed and asked to see; a scrubber over it would mangle real messages while doing nothing about the secret a person can type directly, and it would move the bound from the KIND of content served to the content itself, which is what "bounded at the source" exists to avoid. three smaller corrections ride along, all in the same file: - `summariseTool` no longer reads `pattern`. a search pattern is operator-supplied text that routinely embeds the very secret being searched for, so it is dropped rather than bounded — truncating a secret still serves its prefix. - `summariseTools` collapses a run of the same tool to one `×N` token. `types.ts` and the design both promised `Bash ×3` while repeats rendered `Bash · x · Bash · y`; a run rather than a total, so the line still reads as a sequence of what happened. - journal stamps use the host's local clock. they sit inches above a live screen showing local time, and two clocks an inch apart differing by the utc offset is a reader mis-ordering their own session. docs/decisions.md 18 and the design's §2 said the reconstructed path "is switched off for that agent" while the client still merges every poll into it. the code is right and the sentences were wrong: `history.ts` can only commit a line it watched scroll off, so a buffer switched off at the source would be empty at the exact moment the fallback needs it. both now say the journal is the only source RENDERED, and that the buffer keeps accumulating for the fallback. the fixture is invented per house rule 2 and its paths are `/path/to/…` placeholders — its first draft used literal home paths and `make check-clean` caught it. Co-Authored-By: Claude Opus 5 --- docs/decisions.md | 45 ++++- .../2026-08-20-journal-history-design.md | 34 +++- src/server/journal/claude.ts | 43 ++++- src/server/journal/text.ts | 182 +++++++++++++++++- tests/fixtures/journal/claude-injected.jsonl | 9 + tests/journal-claude.test.ts | 73 +++++++ tests/journal-text.test.ts | 120 +++++++++++- 7 files changed, 484 insertions(+), 22 deletions(-) create mode 100644 tests/fixtures/journal/claude-injected.jsonl diff --git a/docs/decisions.md b/docs/decisions.md index 6ce8aa8..0f01549 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -397,9 +397,15 @@ session does not silently re-litigate them. the operator taps. **Journal history and reconstruction never coexist for one agent.** Where - a journal is readable it is the ONLY source above the live screen, and the - reconstructed path (`web/history.ts`) is switched off for that agent. - Where one is not, nothing changes from before this feature. Two sources + a journal is readable it is the ONLY source RENDERED above the live screen: + the reconstructed buffer is not drawn for that agent, and the two are never + concatenated or reconciled. It keeps ACCUMULATING in the background, and + that is deliberate rather than an oversight — `history.ts` can only commit + a line it watched scroll off the viewport, so a buffer switched off at the + source would be empty at the exact moment it is needed: when a journal read + answers `source: "reconstruction"` and the pane falls back. Merging costs a + diff per poll and buys the fallback its content. Where no journal is + readable, nothing changes from before this feature. Two sources DISPLAYED for one range means reconciling overlapping text produced by two different mechanisms — guesswork of exactly the kind this feature exists to remove. @@ -426,8 +432,37 @@ session does not silently re-litigate them. authentication of its own (decision 3), so what this route serves is bounded at the source rather than at the gate. Kept: assistant text, and user text the operator actually typed. Summarised: a `tool_use` becomes - one line (`▸ Bash · `). Dropped entirely: every `tool_result`, - subagent traffic, and thinking blocks. + one line (`▸ Bash ×3 · Read timer.ts`), a run of the same tool collapsing + to one `×N` token, and the hint drawn from a short allow-list of input + fields — never `pattern`, since a search pattern routinely embeds the very + secret being searched for. Dropped entirely: every `tool_result`, subagent + traffic, and thinking blocks. + + "User text the operator actually typed" is narrower than "a `user` record + whose content is a string", and the difference is measured rather than + theoretical. The harness injects its own blocks into that same field — + subagent `` bodies, ``/`` rows, + ``s, ``, and the + ``/``/`` triple a slash + command expands to. Across the three largest session logs on the + development machine, 733 string-content `user` records would have been + served and 176 of them carried a `` body. `` is also how a + SIDECHAIN's output reaches a record whose top-level `isSidechain` flag is + absent, so that flag alone never closed the hole. Those blocks are + stripped before anything is served, and a record stripping empties is + dropped rather than rendered as a bare speaker row (`stripInjected` in + `src/server/journal/text.ts`). Absolute paths remaining in genuinely typed + prose are NOT redacted: that is content the operator wrote and asked to + see, and a scrubber over it would mangle real messages while doing nothing + about the secret a person can type directly. The bound is on the KIND of + content served, which is what "bounded at the source" means. + + A failure `detail` carries a FIXED PHRASE, never a stringified error. Node + and Bun stringify a filesystem error with the path it failed on, and the + route returns `detail` to the browser verbatim, so an ordinary miss on a + rotated log would otherwise disclose the operator's home path — the same + filesystem key the next decision keeps off the wire. The raw error goes to + the host log, where the host can act on it. **The session id never reaches the browser.** `adapter.ts` maps `agent_session` into a server-side map of `agentId → session ref`; the diff --git a/docs/design/2026-08-20-journal-history-design.md b/docs/design/2026-08-20-journal-history-design.md index faa55b2..ca46f81 100644 --- a/docs/design/2026-08-20-journal-history-design.md +++ b/docs/design/2026-08-20-journal-history-design.md @@ -82,9 +82,13 @@ an affordance the operator taps. ### 2. Journal history and reconstruction never coexist for one agent -Where a journal is readable it is the **only** source above the live screen, and -`history.ts` is switched off for that agent. Where one is not, nothing changes -from today. +Where a journal is readable it is the **only** source RENDERED above the live +screen — the reconstructed buffer is not drawn for that agent, and the two are +never concatenated. It keeps accumulating in the background, deliberately: +`history.ts` can only commit a line it watched scroll off the viewport, so a +buffer switched off at the source would be empty at the exact moment the pane +needs it — when a journal read answers `source: "reconstruction"` and the pane +falls back. Where no journal is readable, nothing changes from today. Two sources for one range means reconciling overlapping text that was produced by two different mechanisms, which is guesswork of exactly the kind this feature @@ -116,7 +120,10 @@ at the source rather than at the gate. - **Kept:** assistant text, and user text the operator actually typed. - **Summarised:** a `tool_use` becomes one line — `▸ Bash ×3 · Read timer.ts` — - carrying the tool name and a short input hint. + carrying the tool name and a short input hint. A run of the same tool + collapses to one `×N` token. The hint comes from a short allow-list of input + fields and never from `pattern`: a search pattern routinely embeds the very + secret being searched for. - **Dropped:** every `tool_result`. That is where file contents and command output live. - **Dropped:** subagent (sidechain) traffic and thinking blocks. @@ -125,6 +132,25 @@ A `user` record whose content is a **list** is tool-result traffic, not somethin a person typed. Folding those into the call that produced them is what stops a session rendering hundreds of fabricated "you" turns. +A `user` record whose content is a **string** is not automatically something a +person typed either, and this is measured rather than assumed. The harness +injects its own blocks into that same field — `` (subagent and tool +result text), ``, ``, ``, +``, and the +``/``/`` triple a slash command +expands to. Across the three largest session logs on the development machine, +733 string-content `user` records would have been served, 176 of them carrying a +`` body. `` is also how a **sidechain's** output reaches a record +whose top-level `isSidechain` is absent, so that flag alone never closed the +hole. Those blocks are **stripped**, before any truncation, and a record left +empty by stripping is **dropped** rather than rendered as a bare speaker row. + +Absolute paths surviving in genuinely typed prose are deliberately **not** +redacted. That is content the operator wrote and asked to see; a scrubber over +it would mangle real messages while doing nothing about the secret a person can +type directly into a message. The bound is on the KIND of content served, which +is what "bounded at the source" means. + ### 5. The session id never reaches the browser `adapter.ts` maps `agent_session` into a **server-side** map of `agentId → diff --git a/src/server/journal/claude.ts b/src/server/journal/claude.ts index 2931c1a..b577e4e 100644 --- a/src/server/journal/claude.ts +++ b/src/server/journal/claude.ts @@ -1,7 +1,7 @@ import { readdir } from "node:fs/promises"; import { join } from "node:path"; import { containedRealpath, isSessionId } from "@server/journal/files"; -import { summariseTool } from "@server/journal/text"; +import { stripInjected, summariseTools, type ToolCall } from "@server/journal/text"; import type { JournalAdapter, JournalEntry } from "@server/journal/types"; /** @@ -25,7 +25,10 @@ import type { JournalAdapter, JournalEntry } from "@server/journal/types"; * {"type":"assistant", "message":{"role":"assistant","content":[ {type:"text"|"thinking"|"tool_use"} ]}} * * A `user` record whose content is a LIST is tool-result traffic, not something - * a person typed. `isSidechain` marks subagent traffic. + * a person typed. `isSidechain` marks subagent traffic — but only at the + * record's TOP LEVEL, so it is not sufficient on its own: a subagent's output + * also arrives as an injected `` block inside an unmarked record's + * string content. See `stripInjected` in `text.ts`. */ export const claudeAdapter: JournalAdapter = { name: "claude", @@ -94,15 +97,34 @@ function toEntry(rec: Record): JournalEntry | null { const content = message?.content; if (type === "user") { - // A STRING is a person typing. A LIST is tool-result traffic wearing the - // user role, and rendering those would fabricate hundreds of "you" turns. + // A LIST is tool-result traffic wearing the user role, and rendering those + // would fabricate hundreds of "you" turns. if (typeof content !== "string" || content.trim() === "") return null; - return { role: "user", at, text: content, tools: [] }; + /** + * A STRING is NOT, on its own, "a person typing" — that rule was the leak. + * The harness injects its own blocks into this same field: subagent + * `` bodies, ``/`` rows, + * ``s, ``, and the + * ``/``/`` triple a slash + * command expands to. All of that is tool output or harness bookkeeping, + * which design decision 4 says is never served — and `` in + * particular is how a SIDECHAIN's output reaches a record whose top-level + * `isSidechain` is absent, so the check above cannot see it. + * + * Stripped rather than dropped, and then dropped if nothing prose-shaped + * is left: see `stripInjected` for why both halves are needed. Note the + * order — stripping happens HERE, before `toLines` clamps to + * `MAX_TEXT_CHARS`, because clamping first would have served the first + * 4 KB of a block instead of none of it. + */ + const text = stripInjected(content); + if (text.trim() === "") return null; + return { role: "user", at, text, tools: [] }; } if (!Array.isArray(content)) return null; const texts: string[] = []; - const tools: string[] = []; + const calls: ToolCall[] = []; for (const part of content) { // A content element is allowed to be anything valid JSON permits — this // is a private, unversioned format. `null`, a bare string, a number: none @@ -112,10 +134,13 @@ function toEntry(rec: Record): JournalEntry | null { const p = part as Record; if (p.type === "text" && typeof p.text === "string") texts.push(p.text); else if (p.type === "tool_use" && typeof p.name === "string") { - tools.push(summariseTool(p.name, p.input)); + // Collected raw and summarised together at the end: a run of the same + // tool collapses to `Bash ×3`, which cannot be decided one call at a + // time. + calls.push({ name: p.name, input: p.input }); } // "thinking" and everything unknown falls through deliberately. } - if (texts.length === 0 && tools.length === 0) return null; - return { role: "assistant", at, text: texts.join("\n"), tools }; + if (texts.length === 0 && calls.length === 0) return null; + return { role: "assistant", at, text: texts.join("\n"), tools: summariseTools(calls) }; } diff --git a/src/server/journal/text.ts b/src/server/journal/text.ts index aacd744..1333976 100644 --- a/src/server/journal/text.ts +++ b/src/server/journal/text.ts @@ -62,6 +62,132 @@ export function stripMenu(text: string): string { .join("\n"); } +/** + * The block elements Claude Code INJECTS into a `user` record's string + * content. Not typed by anyone. + * + * WHY THIS EXISTS. `claude.ts` used to serve any `user` record whose content + * was a string, on the rule "a STRING is a person typing". That rule is + * insufficient, and the gap is measured rather than suspected: across the + * three largest session logs on the development machine, 733 such records + * would have been served, of which 176 carried a `…` body + * (subagent and tool result text) and 180 carried + * ``/`` blocks. Design decision 4 promises + * prose only and tool RESULTS never, so those bodies must not reach the wire. + * + * This also closes the `isSidechain` gap that was previously deferred: + * `isSidechain` is a TOP-LEVEL flag, and a `` block is precisely how a + * subagent's output reappears in a record whose top level is not marked at + * all. Dropping the block drops the sidechain traffic with it. + * + * STRIP, NOT DROP THE WHOLE RECORD, and the asymmetry is the point: a record + * that is ONLY an injected block has nothing left worth showing and is dropped + * by the caller once stripping empties it, while a record that is a real typed + * message with a block appended — the ordinary shape, since the harness + * appends its notifications to whatever the operator said — is still worth + * showing minus the block. Dropping wholesale would silently delete real + * messages; stripping alone would leave empty speaker rows. Doing both is what + * serves prose and only prose. + */ +const INJECTED_BLOCKS = [ + "result", + "task-notification", + "output-file", + "system-reminder", + "local-command-stdout", + "command-name", + "command-message", + "command-args", +] as const; + +/** + * Remove every injected block from a record's text, leaving only prose. + * + * Three passes, and each one is deliberately biased toward removing too much + * rather than too little — this is an EXPOSURE guard, so the failure it must + * never have is content escaping: + * + * 1. Balanced `…` pairs, non-greedy, so two blocks of the same + * name do not swallow the prose between them. + * 2. An OPENING tag with no close: everything from it to the end of the + * record goes. A truncated block is still block content, and keeping the + * tail because the harness did not close its own tag would leak exactly + * the bodies pass 1 exists to remove. + * 3. A CLOSING tag with no open: everything from the start of the record up + * to and including it goes, for the mirror reason — that text was inside + * the block. + * + * Passes 2 and 3 can, in principle, eat real prose from someone who typed a + * bare `` into a message. That is accepted: the cost is a message the + * operator can still read on the live screen, and the alternative cost is + * command output on a phone screen with no authentication in front of it. + */ +export function stripInjected(text: string): string { + let out = text; + for (const tag of INJECTED_BLOCKS) { + out = out.replace(new RegExp(`<${tag}(?:\\s[^>]*)?>[\\s\\S]*?`, "gi"), ""); + } + for (const tag of INJECTED_BLOCKS) { + out = out.replace(new RegExp(`<${tag}(?:\\s[^>]*)?>[\\s\\S]*$`, "i"), ""); + out = out.replace(new RegExp(`^[\\s\\S]*`, "i"), ""); + } + return stripMachineElements(out); +} + +/** + * Any element whose tag name is MACHINE-SHAPED, and everything under it. + * + * The named list above is a list, and a list only ever covers injectors + * somebody has already seen. Hooks, plugins and future harness versions all + * write into this same `user` string field with tag vocabularies of their own, + * so a rule is needed as well as a list. + * + * The rule is the tag NAME. Harness and hook injections name their blocks in + * `kebab-case` or `snake_case` — ``, ``, + * `` — while the markup that turns up in a message + * a PERSON wrote is HTML or JSX, whose element names are a single lowercase word + * or PascalCase: `
`, ``, `