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..2ad424f 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -377,3 +377,144 @@ 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 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. + + **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 ×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`). + + A NAMED LIST is not enough on its own — hooks, plugins and future harness + versions inject blocks with vocabularies nobody has listed — so a second, + weaker rule runs alongside it: an element whose tag NAME is kebab- or + snake-cased is machine output, because harness injections name their blocks + that way while the markup in a typed message is HTML or JSX (`
`, + ``). This rule is an inference, not a contract, and it is + deliberately the weaker of the two. It fires only on a BALANCED pair or a + self-closing element, never on an unmatched bracket: `` in + "replace `` with ``" is a placeholder, and truncating + at it deleted the operator's instruction. What it still costs is a message + quoting a real custom element or framework tag with both halves present — + `` — which loses that element and everything + between the tags. The named list, by contrast, may take an opener's whole + remainder, because a truncated `` really does mean the rest of the + record is machine output. + + 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 + 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 + 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/design/2026-08-20-journal-history-design.md b/docs/design/2026-08-20-journal-history-design.md new file mode 100644 index 0000000..ca46f81 --- /dev/null +++ b/docs/design/2026-08-20-journal-history-design.md @@ -0,0 +1,290 @@ +# 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 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 +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. 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. + +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. + +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 → +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. diff --git a/docs/gotchas.md b/docs/gotchas.md index 253dd3e..5e8dc75 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 | @@ -222,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/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. diff --git a/docs/roadmap.md b/docs/roadmap.md index efb4e0c..ff9a915 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -52,6 +52,18 @@ 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. `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. - **Preact swap** if first-load size disappoints (~45 KB → ~4 KB gzipped, @@ -197,17 +209,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/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/server/demo.ts b/src/server/demo.ts index 87e3430..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,9 +30,65 @@ export function demoAgents(now: number): Agent[] { stateSince: now - s.ageMs, updatedAt: now, acknowledgedAt: null, + // 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/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/server/index.ts b/src/server/index.ts index 447e465..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,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, 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"; @@ -510,12 +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, + // 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/server/journal/claude.ts b/src/server/journal/claude.ts new file mode 100644 index 0000000..b577e4e --- /dev/null +++ b/src/server/journal/claude.ts @@ -0,0 +1,146 @@ +import { readdir } from "node:fs/promises"; +import { join } from "node:path"; +import { containedRealpath, isSessionId } from "@server/journal/files"; +import { stripInjected, summariseTools, type ToolCall } 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 — 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", + 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 (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`)); + 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; + } + // 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; + }, +}; + +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 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; + /** + * 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 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 + // 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") { + // 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 && calls.length === 0) return null; + return { role: "assistant", at, text: texts.join("\n"), tools: summariseTools(calls) }; +} diff --git a/src/server/journal/files.ts b/src/server/journal/files.ts new file mode 100644 index 0000000..c4d5a3e --- /dev/null +++ b/src/server/journal/files.ts @@ -0,0 +1,99 @@ +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. + * + * 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 realRoot: string; + try { + 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; +} + +/** + * 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/src/server/journal/read.ts b/src/server/journal/read.ts new file mode 100644 index 0000000..26b8f50 --- /dev/null +++ b/src/server/journal/read.ts @@ -0,0 +1,199 @@ +import { adapterFor } from "@server/journal/registry"; +import { claudeRoots, MAX_TAIL_BYTES, tailChunk } from "@server/journal/files"; +import { toLines } from "@server/journal/text"; +import type { JournalEntry, 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"); + + /** + * FIXED PHRASES, never `String(err)`, and this is an exposure fix rather + * than tidying. A Bun/Node filesystem error stringifies with the path it + * failed on — `ENOENT: no such file or directory, open '/.claude/ + * projects/…'` — and `routes.ts` returns `{ ok: true, ...page }` + * verbatim, so a rotated or deleted log turned an ordinary miss into the + * operator's home path, username and project layout going over the wire + * to the browser. Design decision 5 says a filesystem key never reaches a + * client that cannot need one, and a path in an error message is the same + * key by another route. + * + * The RAW error is not lost: `reportJournalMiss` in `routes.ts` logs the + * detail host-side, where `CLAUDE.md` requires it to be loud, and the + * host is the side that can act on a path. + */ + let size: number; + try { + size = Bun.file(path).size; + } catch (err) { + console.error("journal: could not read the session log", err); + return none("could not read the session log"); + } + + /** + * CLAMPED to the file, not trusted as given. `before` is format-validated + * in the route (digits only) but that says nothing about its range, and a + * cursor from a log that has since been compacted or rotated can sit far + * past the current end. Unclamped, the reader then spends one whole + * round trip per `MAX_TAIL_BYTES` walking back through bytes that do not + * exist before it reaches any record — 512 KB of nothing per tap. + * Clamping costs the operator no history: everything at or before `size` + * is still reachable, because `size` IS the end of the file. + */ + const end = Math.min(before ?? size, size); + if (end <= 0) return { lines: [], source: "journal", hasMore: false, cursor: null, detail: null }; + + 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". Fixed phrase for the same reason as + // that one — the raw error, path and all, goes to the host log. + console.error("journal: could not read a page of the session log", err); + return none("could not read a page of the session log"); + } + + /** + * 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: oldestOffset > 0, + cursor: oldestOffset > 0 ? String(oldestOffset) : 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/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/text.ts b/src/server/journal/text.ts new file mode 100644 index 0000000..6c2cdc1 --- /dev/null +++ b/src/server/journal/text.ts @@ -0,0 +1,318 @@ +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 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. + * + * 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 DIGIT_OPTION_RE = /^\s*(?:[❯>]\s*)?\d+[.)]\s+\S.*$/; + +/** + * 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 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 { + return text + .split("\n") + .filter((line) => !isOptionRow(line)) + .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: `
`, ``, ` )} diff --git a/src/web/demo/backend.ts b/src/web/demo/backend.ts index e61371e..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"; @@ -43,6 +44,13 @@ const agents: Agent[] = SEED.map((s) => ({ stateSince: Date.now() - s.ageMs, updatedAt: Date.now(), acknowledgedAt: null, + // 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. `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, })); /** Cursor position on the blocked agent's menu, moved by the arrow keys. */ @@ -142,6 +150,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_JOURNAL_LINES, 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/src/web/pane-cache.ts b/src/web/pane-cache.ts index fdd79a6..052f9c5 100644 --- a/src/web/pane-cache.ts +++ b/src/web/pane-cache.ts @@ -26,11 +26,46 @@ export interface Screen { digest: string | null; } +/** + * Everything "Show earlier" has learned about an agent's JOURNAL history. + * + * Held here for exactly the reason the two caches above are: this used to be + * four `useState`s inside `AgentTerminal`, which is remounted per agent AND on + * every navigation. Six taps of history — six round trips, and pages the + * operator scrolled — were therefore thrown away the moment they went back to + * the list and reopened the pane, and the only way to see them again was six + * more taps and six more POSTs. The reconstructed path this replaces does not + * lose its scrollback on that journey, so losing it here was a REGRESSION for + * the agents the feature was built for. + * + * `cursor` and `done` matter as much as `lines`: without them the reopened + * pane would re-fetch page one and prepend it to nothing, and `fellBack` is + * the pane's permanent answer to "does this agent have a journal at all" — + * re-asking the server on every navigation is the retry that decision 18 says + * must happen once. + */ +export interface JournalState { + /** Journal-sourced lines, oldest first. */ + lines: string[]; + /** Opaque cursor for the NEXT (older) page, or null at the beginning. */ + cursor: string | null; + /** No more journal pages — a genuine `hasMore: false`. */ + done: boolean; + /** This pane has permanently handed over to the reconstructed path. */ + fellBack: boolean; +} + +export const emptyJournal = (): JournalState => ({ + lines: [], cursor: null, done: false, fellBack: false, +}); + const screens = new Map(); const histories = new Map(); +const journals = new Map(); export const screenFor = (agentId: string): Screen | undefined => screens.get(agentId); export const historyFor = (agentId: string): History | undefined => histories.get(agentId); +export const journalFor = (agentId: string): JournalState | undefined => journals.get(agentId); export function rememberScreen(agentId: string, screen: Screen): void { screens.set(agentId, screen); @@ -40,6 +75,24 @@ export function rememberHistory(agentId: string, history: History): void { histories.set(agentId, history); } +/** + * Read-modify-write in one call, returning the new value. + * + * The CACHE is the single source of truth and the component's state is only a + * mirror that makes React re-render. Patching through a function of the + * previous CACHED value — rather than of the previous rendered value — is what + * keeps those two from drifting when an update is applied from a promise + * callback that closed over an older render. + */ +export function updateJournal( + agentId: string, + patch: (prev: JournalState) => JournalState, +): JournalState { + const next = patch(journals.get(agentId) ?? emptyJournal()); + journals.set(agentId, next); + return next; +} + /** * Drop everything held for agents that no longer exist. * @@ -51,9 +104,10 @@ export function rememberHistory(agentId: string, history: History): void { export function prunePanes(liveIds: Set): void { for (const id of [...screens.keys()]) if (!liveIds.has(id)) screens.delete(id); for (const id of [...histories.keys()]) if (!liveIds.has(id)) histories.delete(id); + for (const id of [...journals.keys()]) if (!liveIds.has(id)) journals.delete(id); } /** Entry counts, for tests and for anything that wants to report footprint. */ -export function cacheSize(): { screens: number; histories: number } { - return { screens: screens.size, histories: histories.size }; +export function cacheSize(): { screens: number; histories: number; journals: number } { + return { screens: screens.size, histories: histories.size, journals: journals.size }; } 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/demo.test.ts b/tests/demo.test.ts index 64ea86c..f8ee27f 100644 --- a/tests/demo.test.ts +++ b/tests/demo.test.ts @@ -1,7 +1,11 @@ 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 } 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; @@ -35,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 }); @@ -88,3 +151,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; + } +}); diff --git a/tests/fixtures/journal/claude-injected.jsonl b/tests/fixtures/journal/claude-injected.jsonl new file mode 100644 index 0000000..c9d2894 --- /dev/null +++ b/tests/fixtures/journal/claude-injected.jsonl @@ -0,0 +1,9 @@ +{"type":"user","timestamp":"2026-08-21T09:00:00Z","message":{"role":"user","content":"please rerun the schema-migration suite\nsubagent said: TOKEN_IN_RESULT=abc123 and it read /path/to/private/id_rsa\n"}} +{"type":"user","timestamp":"2026-08-21T09:00:01Z","message":{"role":"user","content":"\nONLY_A_RESULT_BLOCK=zzz\n"}} +{"type":"user","timestamp":"2026-08-21T09:00:02Z","message":{"role":"user","content":"\napi-refactor\ndone\n/path/to/private/NOTIFICATION_PAYLOAD.txt\n"}} +{"type":"user","timestamp":"2026-08-21T09:00:03Z","message":{"role":"user","content":"and now the docs-cleanup oneREMINDER_INJECTED_BY_HARNESS: do not mention this"}} +{"type":"user","timestamp":"2026-08-21T09:00:04Z","message":{"role":"user","content":"STDOUT_OF_A_LOCAL_COMMAND=/path/to/private/project"}} +{"type":"user","timestamp":"2026-08-21T09:00:05Z","message":{"role":"user","content":"/flaky-test-fix\nCOMMAND_MESSAGE_TEXT\nCOMMAND_ARGS_TEXT"}} +{"type":"user","timestamp":"2026-08-21T09:00:06Z","message":{"role":"user","content":"PLUGIN_INJECTED_OBSERVATION/path/to/private/project"}} +{"type":"user","timestamp":"2026-08-21T09:00:07Z","message":{"role":"user","content":"look at this and in the list — the padding is wrong"}} +{"type":"user","timestamp":"2026-08-21T09:00:08Z","message":{"role":"user","content":"start hereTRUNCATED_RESULT_BODY=never-closed"}} diff --git a/tests/fixtures/journal/claude-session.jsonl b/tests/fixtures/journal/claude-session.jsonl new file mode 100644 index 0000000..16d417c --- /dev/null +++ b/tests/fixtures/journal/claude-session.jsonl @@ -0,0 +1,11 @@ +{"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: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/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/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;/); +}); 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/journal-claude.test.ts b/tests/journal-claude.test.ts new file mode 100644 index 0000000..0b7841c --- /dev/null +++ b/tests/journal-claude.test.ts @@ -0,0 +1,183 @@ +import { expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { claudeAdapter } from "@server/journal/claude"; +import { MAX_TEXT_CHARS } from "@server/journal/text"; + +const chunk = readFileSync("tests/fixtures/journal/claude-session.jsonl", "utf8"); +const entries = claudeAdapter.parse(chunk); + +/** + * A second fixture, for the ONE rule that "a string is a person typing" got + * wrong: the harness (and any hook or plugin) injects its own blocks into that + * same field. Invented content throughout, per house rule 2 — the SHAPES are + * real, every byte between the tags is made up, and the absolute paths are + * `/path/to/…` placeholders (a literal home path in a fixture is what + * `make check-clean` exists to catch). + */ +const injectedChunk = readFileSync("tests/fixtures/journal/claude-injected.jsonl", "utf8"); +const injected = claudeAdapter.parse(injectedChunk); +const injectedText = JSON.stringify(injected); + +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("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"); +}); + +test("locate refuses a value that is not a session id, before touching disk", async () => { + expect(await claudeAdapter.locate("../../etc/passwd", ["/nonexistent"])).toBeNull(); +}); + +/** + * Every injected block shape, asserted on the whole parse — one leak anywhere + * is the whole failure, exactly like the `SECRET_TOKEN` assertion above. + * + * The BODIES are what matter, so each is given a distinctive invented marker + * in the fixture rather than being checked by tag name: a test that only + * asserted the tags were gone would pass on an implementation that stripped + * the angle brackets and served the text between them. + */ +test("no injected block body reaches the output, whatever shape it arrived in", () => { + for (const leak of [ + "TOKEN_IN_RESULT", // — subagent / tool result text + "/path/to/private", // an absolute path carried inside a block + "ONLY_A_RESULT_BLOCK", // a record that is nothing but a result + "NOTIFICATION_PAYLOAD", // inside + "REMINDER_INJECTED_BY_HARNESS", // + "STDOUT_OF_A_LOCAL_COMMAND", // + "COMMAND_MESSAGE_TEXT", // + "COMMAND_ARGS_TEXT", // + "flaky-test-fix", // + "PLUGIN_INJECTED_OBSERVATION", // a plugin's own block, not on the list + "TRUNCATED_RESULT_BODY", // an OPENED block the harness never closed + ]) { + expect(injectedText).not.toContain(leak); + } +}); + +test("the typed message a block was appended to still arrives, minus the block", () => { + // The whole reason stripping beats dropping the record: the operator really + // did type this, and it is the only thing in the record worth showing. + const kept = injected.filter((e) => e.role === "user").map((e) => e.text.trim()); + expect(kept).toContain("please rerun the schema-migration suite"); + expect(kept).toContain("and now the docs-cleanup one"); + expect(kept).toContain("start here"); +}); + +test("a record that was ONLY an injected block is dropped, not served as a blank turn", () => { + // Five of the fixture's nine records are pure injection. None may survive + // as an empty "you" row above the live screen, so four turns remain. + expect(injected.every((e) => e.text.trim() !== "")).toBe(true); + expect(injected).toHaveLength(4); +}); + +test("markup a PERSON wrote survives — the strip is not a blanket tag filter", () => { + // `` and `` are HTML and JSX names: single lowercase word, + // or PascalCase. Only kebab/snake-cased names are treated as injected, so a + // message quoting real markup is not silently eaten. + const kept = injected.find((e) => e.text.includes("the padding is wrong")); + expect(kept).toBeDefined(); + expect(kept!.text).toContain("this"); + expect(kept!.text).toContain(""); +}); + +test("a subagent result reaching top level without isSidechain is still dropped", () => { + // `isSidechain` is a TOP-LEVEL flag; none of these records carry it. A + // `` block is exactly how a subagent's output arrives in a record + // the flag cannot see, which is why the flag alone was never sufficient. + expect(injectedChunk).not.toContain("isSidechain"); + expect(injectedText).not.toContain("TOKEN_IN_RESULT"); +}); + +test("the adapter strips BEFORE the text cap, not after — pinned at the call site", () => { + /** + * The ordering assertion has to live here, where `toEntry` chooses when to + * call `stripInjected`. Asserted in journal-text.test.ts it proves nothing + * about the adapter: that test picks the call order itself, so an adapter + * clamping first sails through it. + * + * The shape is chosen so the two orders diverge VISIBLY. A block wider than + * the cap sits FIRST, with the typed message after it: + * + * strip → clamp (correct): the whole block goes, the message remains. + * clamp → strip (broken): the clamp cuts inside the block, taking the + * closing tag and the message with it; the + * truncated opener then strips to nothing and + * the record is dropped — the operator's own + * words gone, and the record silently absent. + */ + const huge = "y".repeat(MAX_TEXT_CHARS * 2); + const line = JSON.stringify({ + type: "user", + timestamp: "2026-08-21T10:00:00Z", + message: { role: "user", content: `OVERSIZE_RESULT_BODY${huge}the typed message` }, + }); + const parsed = claudeAdapter.parse(line); + expect(parsed).toHaveLength(1); + expect(parsed[0]!.text).toBe("the typed message"); + expect(parsed[0]!.text).not.toContain("OVERSIZE_RESULT_BODY"); +}); diff --git a/tests/journal-files.test.ts b/tests/journal-files.test.ts new file mode 100644 index 0000000..9ceaa02 --- /dev/null +++ b/tests/journal-files.test.ts @@ -0,0 +1,91 @@ +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 () => { + // 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-")); + 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 () => { + // 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); +}); diff --git a/tests/journal-read.test.ts b/tests/journal-read.test.ts new file mode 100644 index 0000000..61bc125 --- /dev/null +++ b/tests/journal-read.test.ts @@ -0,0 +1,292 @@ +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([]); + // EXACTLY the fixed phrase, not merely containing it. A Bun/Node filesystem + // error stringifies with the path it failed on, and `routes.ts` returns + // `detail` to the browser verbatim, so `${String(err)}` turned an ordinary + // miss into the operator's home path going over the wire — the filesystem + // key decision 5 keeps off it. `toContain` is not enough to catch that: + // interpolation APPENDS, so a leaking detail still contains the phrase. + // Equality is what makes "nothing else travels" the assertion. + expect(page.detail).toBe("could not read a page of the session log"); + expect(page.detail).not.toContain(root); + expect(page.detail).not.toContain(UUID); +}); + +test("a cursor past the end of the file is clamped, not walked back through", async () => { + // `before` is format-validated in the route (digits only), which says + // nothing about its RANGE. A cursor from a log that has since been + // compacted or rotated can sit far past the current end; unclamped, the + // reader spends one whole round trip per MAX_TAIL_BYTES crossing bytes that + // do not exist before it reaches any record. + const { roots, file } = await journal(6); + const size = Bun.file(file).size; + const reader = createJournalReader(roots); + + const far = await reader.read(SESSION, size + MAX_TAIL_BYTES * 4, 50); + const tail = await reader.read(SESSION, null, 50); + + // One request, and the SAME page a request with no cursor would have given: + // clamping to the end of the file costs no history, because the end of the + // file is where a tail read starts anyway. + expect(far.source).toBe("journal"); + expect(far.lines).toEqual(tail.lines); + expect(far.detail).toBeNull(); +}); + +/** 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")))); +}); 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); +}); diff --git a/tests/journal-route.test.ts b/tests/journal-route.test.ts new file mode 100644 index 0000000..701dd95 --- /dev/null +++ b/tests/journal-route.test.ts @@ -0,0 +1,219 @@ +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); +}); + +/** Runs `body` with `console.error` captured — the sink `warn` writes to. */ +async function captureWarnings(body: () => Promise): Promise { + const lines: string[] = []; + const real = console.error; + console.error = (...args: unknown[]) => { lines.push(args.map(String).join(" ")); }; + try { + await body(); + } finally { + console.error = real; + } + return lines; +} + +test("a detail never carries a filesystem path to the browser", async () => { + // `routes.ts` returns `{ ok: true, ...page }` verbatim, so whatever the + // reader puts in `detail` reaches the phone. Decision 5 keeps filesystem + // keys off the wire, and a path inside an error message is the same key by + // another route. + const { app } = harness({ + lines: [], source: "reconstruction", hasMore: false, cursor: null, + detail: "could not read the session log", + }); + const body = await (await post(app, {})).json() as { detail: string }; + expect(body.detail).not.toMatch(/\/home\/|\/Users\/|ENOENT|EACCES|\.jsonl/); +}); + +test("a miss is reported once per agent, and again after the journal recovers", async () => { + // The de-duplicating set used to be write-only: an agent whose journal came + // back could never be reported again if it later broke a second time, and + // "history silently stopped going deeper" is invisible without that line. + let page: JournalPage = { + lines: [], source: "reconstruction", hasMore: false, cursor: null, + detail: "session log not found — compacted, rotated or removed", + }; + const store = new AgentStore("dev-box"); + store.replaceAll([agent({ agentId: "w9:p9" })], NOW); + 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() { return page; } }, + }); + const ask = () => app.request("/api/agents/w9:p9/history", { + method: "POST", + headers: { "content-type": "application/json", origin: "http://localhost" }, + body: "{}", + }); + + const first = await captureWarnings(async () => { await ask(); await ask(); await ask(); }); + expect(first.filter((l) => l.includes("no journal history"))).toHaveLength(1); + + // The journal reads again... + page = { lines: ["you", "hi", ""], source: "journal", hasMore: false, cursor: null, detail: null }; + await captureWarnings(async () => { await ask(); }); + + // ...so the NEXT failure is heard rather than suppressed by a stale entry. + page = { + lines: [], source: "reconstruction", hasMore: false, cursor: null, + detail: "session log not found — compacted, rotated or removed", + }; + const second = await captureWarnings(async () => { await ask(); }); + expect(second.filter((l) => l.includes("no journal history"))).toHaveLength(1); +}); + +test("the miss set cannot grow without bound", async () => { + // Agent ids do not repeat across harness restarts, so a set that only ever + // grew held one string per agent id ever seen, forever. Reporting has to + // still work at the far end of that: the 400th distinct agent is warned + // about exactly like the first. + const store = new AgentStore("dev-box"); + const many = Array.from({ length: 400 }, (_, i) => agent({ agentId: `wb:p${i}` })); + store.replaceAll(many, NOW); + 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() { + return { + lines: [], source: "reconstruction" as const, hasMore: false, cursor: null, + detail: "no journal adapter for this harness", + }; + }, + }, + }); + const lines = await captureWarnings(async () => { + for (const a of many) { + await app.request(`/api/agents/${a.agentId}/history`, { + method: "POST", + headers: { "content-type": "application/json", origin: "http://localhost" }, + body: "{}", + }); + } + }); + expect(lines.filter((l) => l.includes("no journal history"))).toHaveLength(400); + // And the early entries have been evicted, so re-asking about the FIRST + // agent warns again rather than being silenced by a set that never forgets. + const again = await captureWarnings(async () => { + await app.request("/api/agents/wb:p0/history", { + method: "POST", + headers: { "content-type": "application/json", origin: "http://localhost" }, + body: "{}", + }); + }); + expect(again.filter((l) => l.includes("no journal history"))).toHaveLength(1); +}); diff --git a/tests/journal-terminal.test.tsx b/tests/journal-terminal.test.tsx new file mode 100644 index 0000000..ca1f6c2 --- /dev/null +++ b/tests/journal-terminal.test.tsx @@ -0,0 +1,304 @@ +// 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 { journalFor, prunePanes, rememberHistory } from "@web/pane-cache"; +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); +}); + + +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); +}); + +test("journal history survives leaving the pane and coming back", async () => { + // `AgentTerminal` is remounted per agent and on every navigation. Held as + // component state, six taps of history — six round trips, and pages the + // operator had already scrolled — vanished the moment they went back to the + // list and reopened the pane, and only six more taps brought them back. The + // reconstructed path this replaces never loses its scrollback on that same + // journey, so losing it here was a regression for exactly the agents this + // feature was built for. + prunePanes(new Set()); + let served = 0; + const { fn, calls } = stubFetch({ + "/output": () => screenOf(["out"]), + "/history": () => { + served++; + return { + ok: true, + lines: [`page ${served}`], + source: "journal", + // Still more to fetch, so the button stays offered and a second mount + // cannot be mistaken for "the affordance ended". + hasMore: true, + cursor: String(1000 - served * 100), + detail: null, + }; + }, + }); + globalThis.fetch = fn as typeof fetch; + + const first = await render( + {}} />, + ); + await settle(); + (first.querySelector(".term-earlier") as HTMLButtonElement).click(); + await settle(); + expect(first.textContent).toContain("page 1"); + + // Back to the list, then into the pane again — a fresh mount. + await unmount(); + const second = await render( + {}} />, + ); + await settle(); + + // The page is on screen with NO new request: it came from the cache. + const before = calls.filter((c) => c.url.endsWith("/history")).length; + expect(second.textContent).toContain("page 1"); + expect(before).toBe(1); + + // And the cursor came back with it, so the next tap asks for the page AFTER + // the one already held rather than re-fetching page one onto itself. + (second.querySelector(".term-earlier") as HTMLButtonElement).click(); + await settle(); + const asked = calls.filter((c) => c.url.endsWith("/history")).at(-1); + expect((asked!.body as { before?: string }).before).toBe("900"); + expect(second.textContent).toContain("page 2"); + expect(second.textContent).toContain("page 1"); +}); + +test("a pane that fell back does not re-ask the route on every reopen", async () => { + // `fellBack` is the pane's permanent answer to "is there a journal here". + // Held in the component, every navigation asked the server again — and got + // the same "no" — before showing the reconstruction it already had. + prunePanes(new Set()); + const { fn, calls } = stubFetch({ + "/output": () => screenOf(["out"]), + "/history": () => ({ + ok: true, lines: [], source: "reconstruction", hasMore: false, cursor: null, + detail: "no journal adapter for this harness", + }), + }); + globalThis.fetch = fn as typeof fetch; + rememberHistory("j8:p1", { settled: ["reconstructed line"], gaps: 0 }); + + const first = await render( + {}} />, + ); + await settle(); + (first.querySelector(".term-earlier") as HTMLButtonElement).click(); + await settle(); + expect(journalFor("j8:p1")!.fellBack).toBe(true); + + await unmount(); + const second = await render( + {}} />, + ); + await settle(); + (second.querySelector(".term-earlier") as HTMLButtonElement).click(); + await settle(); + + // One request, ever: the reopened pane behaves like a journal-less one. + expect(calls.filter((c) => c.url.endsWith("/history"))).toHaveLength(1); + expect(second.textContent).toContain("reconstructed line"); +}); diff --git a/tests/journal-text.test.ts b/tests/journal-text.test.ts new file mode 100644 index 0000000..17796eb --- /dev/null +++ b/tests/journal-text.test.ts @@ -0,0 +1,262 @@ +import { expect, test } from "bun:test"; +import { + clamp, MAX_TEXT_CHARS, stripAnsi, stripInjected, stripMenu, summariseTool, + summariseTools, 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("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. + 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); +}); + +test("a search pattern is never used as a hint", () => { + // A pattern is operator-supplied text that routinely embeds the very thing + // being searched for. Design decision 4 bounds this route at the SOURCE, so + // the field is not on the hint allow-list at all — a bare tool name is the + // whole orientation this line owes anyone. + expect(summariseTool("Grep", { pattern: "AKIA_SECRET_KEY_SHAPE" })).toBe("Grep"); + expect(summariseTool("Grep", { pattern: "x" })).not.toContain("x"); +}); + +test("a run of the same tool collapses to one ×N token", () => { + // `src/server/journal/types.ts` and the design's §4 both promise `Bash ×3`. + // Un-aggregated, three calls rendered as `Bash · x · Bash · y · Bash · z` — + // the promise false, and the longest possible line on the narrowest screen. + expect(summariseTools([ + { name: "Bash", input: { description: "one" } }, + { name: "Bash", input: { description: "two" } }, + { name: "Bash", input: { description: "three" } }, + { name: "Read", input: { file_path: "/srv/project/src/timer.ts" } }, + ])).toEqual(["Bash ×3", "Read · timer.ts"]); +}); + +test("aggregation keeps call order rather than totalling", () => { + // A sequence of what happened, not a frequency table: two separate runs of + // the same tool stay two tokens, in the order they were called. + expect(summariseTools([ + { name: "Read", input: {} }, + { name: "Bash", input: {} }, + { name: "Bash", input: {} }, + { name: "Read", input: {} }, + ])).toEqual(["Read", "Bash ×2", "Read"]); +}); + +test("a single call still carries its hint", () => { + expect(summariseTools([{ name: "Bash", input: { description: "run tests" } }])) + .toEqual(["Bash · run tests"]); +}); + +test("journal times are the host's local clock, not UTC", () => { + // These lines sit inches above the live screen, which shows whatever clock + // the agent's terminal printed — local. Two clocks an inch apart, differing + // by the machine's UTC offset, is a reader mis-ordering their own session. + // + // The timezone is SET here rather than inherited: `bun test` runs with TZ + // pinned to UTC, so a test that merely read the ambient zone would pass + // identically against the `getUTCHours` this replaced — a guard that cannot + // fail is not a guard. + const saved = process.env.TZ; + try { + process.env.TZ = "Asia/Tokyo"; // UTC+9, no DST, so the arithmetic is fixed + expect(toLines([ + { role: "user", at: "2026-08-20T13:04:00Z", text: "hello", tools: [] }, + ])[0]).toBe("you · 22:04"); + } finally { + process.env.TZ = saved; + } +}); + +test("every named injected block is removed, body and all", () => { + for (const [tag, body] of [ + ["result", "SUBAGENT_RESULT_BODY"], + ["task-notification", "NOTIFICATION_BODY"], + ["output-file", "OUTPUT_FILE_BODY"], + ["system-reminder", "REMINDER_BODY"], + ["local-command-stdout", "STDOUT_BODY"], + ["command-name", "COMMAND_NAME_BODY"], + ["command-message", "COMMAND_MESSAGE_BODY"], + ["command-args", "COMMAND_ARGS_BODY"], + ] as const) { + const out = stripInjected(`typed prose <${tag}>${body} more prose`); + expect(out).not.toContain(body); + expect(out).toContain("typed prose"); + expect(out).toContain("more prose"); + } +}); + +test("an injected block the harness never closed takes the rest of the record", () => { + // A truncated block is still block content. Keeping the tail because the + // writer did not close its own tag would leak exactly what pass 1 removes. + expect(stripInjected("typed prose UNCLOSED_BODY and on and on")) + .not.toContain("UNCLOSED_BODY"); +}); + +test("a closing tag with no opener takes everything before it", () => { + // The mirror case: that text was inside the block. + expect(stripInjected("ORPHANED_BODY tail")).not.toContain("ORPHANED_BODY"); +}); + +test("a block shape nobody has listed yet is still removed, by its NAME's shape", () => { + // A list only covers injectors somebody has already seen; hooks and plugins + // write into the same field with vocabularies of their own. Kebab- and + // snake-cased names are machine-written; HTML and JSX names are not. + expect(stripInjected("HOOK_BODY")) + .not.toContain("HOOK_BODY"); + expect(stripInjected("PLUGIN_BODY")) + .not.toContain("PLUGIN_BODY"); +}); + +test("markup a person wrote is left alone", () => { + // The false positive worth avoiding: over-stripping here deletes something + // an operator typed. `
`, ``, `` are never touched. + const typed = "the
row
under is misaligned"; + expect(stripInjected(typed)).toBe(typed); +}); + +test("stripping removes a block wider than the text cap rather than truncating it", () => { + // The unit half of the ordering rule. The half that actually pins the ORDER + // is in tests/journal-claude.test.ts, at the call site — asserting it here, + // where the test itself chooses when to call `stripInjected`, cannot fail on + // an adapter that clamps first. + const huge = "x".repeat(MAX_TEXT_CHARS * 2); + const out = stripInjected(`hello${huge}`); + expect(out).toBe("hello"); +}); + +test("an angle-bracket placeholder in typed prose is not a block, and survives whole", () => { + // Introduced by the shape rule and caught in review: an unbalanced kebab- or + // snake-cased bracket in a typed message is overwhelmingly a PLACEHOLDER, + // which is ordinary developer prose. Truncating at the opener deleted the + // operator's actual instruction — "replace ", "run `git push origin ", + // "if a" — and the design says over-stripping real prose is the worse of the + // two failures. + for (const typed of [ + "replace with everywhere", + "run `git push origin ` and then open the PR", + "if ad then continue with the next step", + ]) { + expect(stripInjected(typed)).toBe(typed); + } +}); + +test("a shape-matched element is still removed when it is BALANCED", () => { + // The concession above is scoped to unbalanced brackets only. A matched pair + // is still machine output by its name's shape, and still goes — along with + // everything between the tags. + expect(stripInjected("keep me HOOK_BODY and me")) + .toBe("keep me and me"); + expect(stripInjected("PLUGIN_BODYtail")) + .toBe("tail"); +}); + +test("a NAMED block the harness truncated still takes the remainder", () => { + // The asymmetry, stated as a test: the named list may take an opener's whole + // remainder, because a truncated `` really does mean the rest of the + // record is machine output. The shape rule may not. + expect(stripInjected("typed prose TRUNCATED_BODY and on and on")) + .toBe("typed prose "); +}); 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/pane-cache.test.ts b/tests/pane-cache.test.ts index fc2f811..5ecaf60 100644 --- a/tests/pane-cache.test.ts +++ b/tests/pane-cache.test.ts @@ -1,6 +1,7 @@ import { expect, test } from "bun:test"; import { - historyFor, prunePanes, rememberHistory, rememberScreen, screenFor, cacheSize, + historyFor, journalFor, prunePanes, rememberHistory, rememberScreen, screenFor, + cacheSize, updateJournal, } from "@web/pane-cache"; function seed(ids: string[]) { @@ -27,7 +28,7 @@ test("pruning to nothing empties both caches", () => { prunePanes(new Set()); seed(["a", "b"]); prunePanes(new Set()); - expect(cacheSize()).toEqual({ screens: 0, histories: 0 }); + expect(cacheSize()).toEqual({ screens: 0, histories: 0, journals: 0 }); }); test("pruning is idempotent", () => { @@ -54,5 +55,43 @@ test("caches only ever hold agents that were seeded", () => { seed(["a"]); // A live id with no cache entry must not create one. prunePanes(new Set(["a", "never-opened"])); - expect(cacheSize()).toEqual({ screens: 1, histories: 1 }); + expect(cacheSize()).toEqual({ screens: 1, histories: 1, journals: 0 }); +}); + +test("journal history is held per agent, not per mount", () => { + // The whole point of this module: `AgentTerminal` is remounted per agent and + // on every navigation, so journal state living inside it was thrown away the + // moment the operator went back to the list — six taps of history and six + // POSTs, gone. The reconstructed path never lost its scrollback on that same + // journey. + prunePanes(new Set()); + updateJournal("a", (p) => ({ ...p, lines: ["older"], cursor: "120" })); + updateJournal("a", (p) => ({ ...p, lines: ["oldest", ...p.lines], cursor: "60" })); + expect(journalFor("a")).toEqual({ + lines: ["oldest", "older"], cursor: "60", done: false, fellBack: false, + }); + // And it is per AGENT: one pane's pages never appear in another's. + expect(journalFor("b")).toBeUndefined(); +}); + +test("a pane that fell back stays fallen back", () => { + // `fellBack` is the pane's permanent answer to "does this agent have a + // readable journal". Losing it on navigation means re-asking the server on + // every reopen, which decision 18 says happens once. + prunePanes(new Set()); + updateJournal("a", (p) => ({ ...p, fellBack: true })); + expect(journalFor("a")!.fellBack).toBe(true); +}); + +test("a closed pane's journal history does not linger", () => { + // Evicted by the same signal as the screen and the scrollback: the agent is + // gone. Otherwise this grows by one entry per agent ever opened. + prunePanes(new Set()); + updateJournal("gone", (p) => ({ ...p, lines: ["x"] })); + updateJournal("stays", (p) => ({ ...p, lines: ["y"] })); + expect(cacheSize().journals).toBe(2); + prunePanes(new Set(["stays"])); + expect(journalFor("gone")).toBeUndefined(); + expect(journalFor("stays")).toBeDefined(); + expect(cacheSize().journals).toBe(1); }); 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/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"); +}); 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, }; }