From 08d99da5ae06e6b844166289deb4cf7d4530cad2 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Fri, 6 Nov 2026 03:17:14 +0100 Subject: [PATCH] test(edit-history): the ssh kill test no longer races the fake's pid file The test waited for a real 1000 ms timeout and read the pid file afterwards. On a loaded machine the kill could land before the fake ssh wrote it. It now waits for the pid and sends the timeout's SIGTERM itself, which is all Node's timeout does. Without exec the fake still survives and the test goes red. The invariants said "the reads" go through buildRemoteEditHistoryCommand; the image probes go through remoteProbePaths, on the same limiter. Co-Authored-By: Claude Opus 5.5 --- docs/architecture-invariants.md | 2 +- test/remote-edit-sources.test.ts | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index c3fa886a..b5c3d450 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -768,7 +768,7 @@ Matching semantics: a query containing `/` matches the relative path, otherwise Paths, agent descriptions and diff lines render via `textContent` only (browser test with a hostile path). The header button uses the marker-class pattern (`btn-edit-history--hidden`) and stays off phones (`mobile.css`, locked in `test/mobile-header-buttons-policy.test.ts`). Docker and non-claude sessions answer `unsupported` and are never read. -⚠️ **Remote (SSH) sessions: read over ssh, never woken.** The sync stays path-based; its deps get the session, and `server.ts` routes them to the local reader or to `remote-files.ts`. `transcriptPath` is one ssh round trip (`buildRemoteClaudeTranscriptLocateCommand`): first the conversation the claude process in the session's remote pane writes now (its `/sessions/.json`, found through the pane's process tree, since a remote host sends Codeman no hooks), else the most recently written `/projects/*/.jsonl` of the stored history's conversation, `` and `` (`remoteTranscriptIds`; nothing moves `claudeSessionId` after a remote `/clear`, and no fixed order is right both after a `/clear` and after a respawn that resumed the launch conversation); the result is an absolute path on the host. A read pass is one more round trip (`buildRemoteClaudeEditSourcesCommand`), or several with more than 300 subagent requests (below), with the local reader's semantics: offset clamped to the size, a block with `len` ends at `offset + len`, a new file reads its tail, one budget shared main-first, `onlyRequested` for "Load older", at most 8 KiB of `meta.json` per subagent (within a 4 MiB meta budget per read, which the local reader does not have: a meta comes only with a subagent's first read, so one past the budget is never sent and that subagent shows its id instead of its description). `test/remote-edit-sources.test.ts` runs both scripts on a real `sh` against a temp `$HOME` and compares the read byte for byte with `readClaudeEditSources` on seeded random transcripts, so `/clear` catch-up, "Load older", cursors and the store LRU behave remotely as locally. Every request value reaches the script only after validation (id `[A-Za-z0-9._-]`, relPath `main` or `subagents/agent-.jsonl`, integer offsets) and shellescaped; the scripts run under `sh -c` with `LC_ALL=C`. The config dir is an absolute `CLAUDE_CONFIG_DIR` of the ssh command's environment, else `$HOME/.claude` (claude's own rule); the remote claude runs under `$SHELL -i -l -c`, the lookup under a plain `sh`, so a value set only in a login or interactive profile (`.profile`, `.bash_profile`, `.zshrc`) is not seen. A round trip's timeout grows with its byte cap (`remoteEditSourcesTimeoutMs`: 30 s plus the transfer at 256 KiB/s), since a forward pass may carry 13 MiB and "Load older" 29 MiB. ⚠️ **Never wakes:** the reads go through `buildRemoteEditHistoryCommand` (`buildRemoteFileCommand` with `exec`, so a timeout reaches ssh and not only the local `sh -c`, which on dash stays its parent) + `runWithRemoteSshLimit` (`ConnectTimeout=10`), so a sleeping host is a connect timeout and the panel shows `unavailable` with the stored entries; the edit-history read path reaches neither `remote-wake.ts` nor `session-routes.ts`, even transitively (guard in `test/remote-edit-sources.test.ts`). Without hooks there is no `transcript:tool_end`, so an open panel re-reads on `session:idle` and the refresh button only. +⚠️ **Remote (SSH) sessions: read over ssh, never woken.** The sync stays path-based; its deps get the session, and `server.ts` routes them to the local reader or to `remote-files.ts`. `transcriptPath` is one ssh round trip (`buildRemoteClaudeTranscriptLocateCommand`): first the conversation the claude process in the session's remote pane writes now (its `/sessions/.json`, found through the pane's process tree, since a remote host sends Codeman no hooks), else the most recently written `/projects/*/.jsonl` of the stored history's conversation, `` and `` (`remoteTranscriptIds`; nothing moves `claudeSessionId` after a remote `/clear`, and no fixed order is right both after a `/clear` and after a respawn that resumed the launch conversation); the result is an absolute path on the host. A read pass is one more round trip (`buildRemoteClaudeEditSourcesCommand`), or several with more than 300 subagent requests (below), with the local reader's semantics: offset clamped to the size, a block with `len` ends at `offset + len`, a new file reads its tail, one budget shared main-first, `onlyRequested` for "Load older", at most 8 KiB of `meta.json` per subagent (within a 4 MiB meta budget per read, which the local reader does not have: a meta comes only with a subagent's first read, so one past the budget is never sent and that subagent shows its id instead of its description). `test/remote-edit-sources.test.ts` runs both scripts on a real `sh` against a temp `$HOME` and compares the read byte for byte with `readClaudeEditSources` on seeded random transcripts, so `/clear` catch-up, "Load older", cursors and the store LRU behave remotely as locally. Every request value reaches the script only after validation (id `[A-Za-z0-9._-]`, relPath `main` or `subagents/agent-.jsonl`, integer offsets) and shellescaped; the scripts run under `sh -c` with `LC_ALL=C`. The config dir is an absolute `CLAUDE_CONFIG_DIR` of the ssh command's environment, else `$HOME/.claude` (claude's own rule); the remote claude runs under `$SHELL -i -l -c`, the lookup under a plain `sh`, so a value set only in a login or interactive profile (`.profile`, `.bash_profile`, `.zshrc`) is not seen. A round trip's timeout grows with its byte cap (`remoteEditSourcesTimeoutMs`: 30 s plus the transfer at 256 KiB/s), since a forward pass may carry 13 MiB and "Load older" 29 MiB. ⚠️ **Never wakes:** the lookup and the reads go through `buildRemoteEditHistoryCommand` (`buildRemoteFileCommand` with `exec`, so a timeout reaches ssh and not only the local `sh -c`, which on dash stays its parent), the image probes through `remoteProbePaths`, and all of them through `runWithRemoteSshLimit` (`ConnectTimeout=10`), so a sleeping host is a connect timeout and the panel shows `unavailable` with the stored entries; the edit-history read path reaches neither `remote-wake.ts` nor `session-routes.ts`, even transitively (guard in `test/remote-edit-sources.test.ts`). Without hooks there is no `transcript:tool_end`, so an open panel re-reads on `session:idle` and the refresh button only. ⚠️ **A read with more than 300 subagent requests spans several round trips** (`planRemoteEditSourceRounds`, `EDIT_SOURCE_REQUESTS_PER_ROUND_TRIP`). The script is one argument of the ssh line, Linux caps a single argv string at 128 KiB (`E2BIG` at about 1000 subagents), the cursors have no cap and the stored conversation is read first, so one script for all of them left a long history `unavailable`, and not even a `/clear` got past it. The cut is by NAME RANGE, not only by count: the script takes a file no `case` line names for a new one (its tail, its meta), so plain groups would read each other's files as new. Each round trip walks only the names between its cuts (one `awk` per round trip, never a fork per file), so a new file lands in exactly one range. Main comes with the first round trip, budget and meta budget carry over, and a later round trip still lists its files, because a requested file missing from the result loses its cursor. A failed round trip fails the whole read and nothing of it reaches the store; each one takes its own limiter slot. The merged result equals the local reader's, byte for byte (the fuzz test cuts every read at a random size). A read of up to 300 runs no `awk` at all. ⚠️ No `case` inside a `$(…)`: bash 3.2, macOS `sh`, cannot parse it, which is why the names come from a shell function. diff --git a/test/remote-edit-sources.test.ts b/test/remote-edit-sources.test.ts index f0f37101..11795a77 100644 --- a/test/remote-edit-sources.test.ts +++ b/test/remote-edit-sources.test.ts @@ -700,8 +700,10 @@ describe('a read too large for one ssh line', () => { }); describe('a timeout ends ssh itself', () => { - // Node signals the local `sh -c`. On dash that shell stays the parent of a plain command, so - // only `exec` makes the signal reach ssh. The fake ssh records its pid and then just waits. + // Node's timeout signals the local `sh -c`. On dash that shell stays the parent of a plain + // command, so only `exec` makes the signal reach ssh. The fake ssh records its pid and then + // waits; the test sends the timeout's SIGTERM itself, once that pid is there, so a slow + // machine cannot kill the fake before it wrote it. it('kills the ssh process, not only the shell around it', async () => { const bin = mkdtempSync(join(tmpdir(), 'codeman-fake-ssh-')); const pidFile = join(bin, 'pid'); @@ -713,10 +715,10 @@ describe('a timeout ends ssh itself', () => { chmodSync(join(bin, 'ssh'), 0o755); const command = buildRemoteEditHistoryCommand(REMOTE, buildRemoteClaudeTranscriptLocateCommand(['conv-1'])); expect(command).toMatch(/^exec ssh /); - await new Promise((resolve) => { - exec(command, { timeout: 1000, env: { ...process.env, PATH: `${bin}:${process.env.PATH}` } }, () => resolve()); - }); + const child = exec(command, { env: { ...process.env, PATH: `${bin}:${process.env.PATH}` } }); + await expect.poll(() => existsSync(pidFile), { timeout: 10_000 }).toBe(true); const pid = Number(readFileSync(pidFile, 'utf8')); + child.kill(); const alive = (): boolean => { try { process.kill(pid, 0);