Repository navigation
🤖 refactor: the durable journal and the removal tombstones read and write through SessionFiles and HostFiles - #7426
Merged
Merged
Conversation
…rite through SessionFiles and HostFiles
…eck use the files they were given
…out awaiting a non-promise
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
🟢 e2e stories (Haiku): 14 passed27 agent steps · 7 replayed from cache · 30 model calls · 207.3k tokens (69% cached) All 14 tests in 9 files
e2e 0.20.0 · 3m 4s · web · run artifacts 🟢 e2e Electron stories (Haiku): 3 passed3 agent steps · 3 model calls · 8.1k tokens (32% cached) All 3 tests in 3 files
e2e 0.20.0 · 19.7s · electron · run artifacts 🟢 e2e agent repros (Haiku): no tests selectedSpend: $0.00 of the $1.00 cap (proxy ledger) e2e 0.20.0 · 5.3s · 2 targets (web, phone) · run artifacts |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 24, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 24, 2026
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
…ugh the HistoryService's SessionFiles (coder#7428) ## Summary 5b PR 5 of phase 5 (Loop host I/O) of the agent-loop refactor. `SessionUsageService` reads and writes its files through the `SessionFiles` of its HistoryService (phase 2) instead of `fs/promises` and `writeFileAtomic`: - the settled ledger `session-usage.json`, - the in-flight files of running streams (`session-usage.in-flight/<key>.json`), - the headless sidecar `headless-usage.jsonl`. All three live in the session dir, so they are session files. The service already takes its locks from the same HistoryService (phase 4). Now a memory `SessionFiles` layer keeps these files off the disk too. Behavior does not change: the default is the Node layer (`nodeSessionFiles`), which calls the same `fs/promises` functions and `writeFileAtomic`. Its members read their bindings at call time, so the existing tests that spy on `fs/promises` still intercept these calls. ## Order Plan order: stack 5b is PR 1 to PR 9. This PR waits for phase 4 PR 13 (coder#7423, merged), which also changed `sessionUsageService.ts`. It uses no code of 5b PRs 3 and 4, so it opens on its own, with base main. ## Implementation - `services/sessionUsageService.ts`: a private `files` getter returns `historyService.files`. Like the `fileLocks` getter beside it, it reads at each use, not at construction. The 11 `fs/promises` calls and the 2 `writeFileAtomic` calls go through it, and the module-level `readInFlightUsageFile` takes the files as its first argument. The file has no `fs` import now. - `recordHeadlessUsage` checks the removal tombstone through the same files (`isWorkspaceRemovalTombstoned(rootDir, workspaceId, this.files)`), like HistoryService's own check. The files argument of that function came with 5b PR 4 (coder#7426, merged), so a later commit of this PR adds it after a merge of main. The plan estimated about 420 lines. This PR adds 115 and removes 18, because the service reaches the files through the HistoryService it already has, so no caller changes. ## Validation - `sessionUsageService.test.ts`: a second new case puts a tombstone only in the memory volume, and `recordHeadlessUsage` refuses the write (undefined) and creates no session dir. Without the files argument of the check, the write succeeds and the case fails. - `sessionUsageService.test.ts`: a new case builds HistoryService on memory `SessionFiles`. It records a ledger entry, an in-flight step and a headless sidecar line. The memory session dir holds all three, and a restarted service folds the in-flight file into the ledger from memory (120 input tokens) and deletes it there. The session dir on disk does not exist. With main's `sessionUsageService.ts`, the case fails (ENOENT on the memory readdir). - The plan's tests (`sessionUsageService.test.ts`, `sessionUsageService.inFlight.test.ts`, `streamManager*.test.ts`) plus `agentStatusService.test.ts` and `tokenizerService.test.ts`: 513 pass, 0 fail. The 8 other test files that use `SessionUsageService`: 520 pass, 0 fail. The on-disk tests read `session-usage.json` and the in-flight files from disk, and they pass unchanged. - `make static-check`, `make crap-check` and `make dry-check` pass. `make lint-eslint` first flagged an any-typed matcher (`expect.arrayContaining`) in the new case, and a test-only commit replaced it with `toContain` checks. - `make test` (host suite lock) on head 6ce1a31: `bun test src` had 27905 pass, 7 skip, 0 fail. The shared scripts/storybook/vscode run had 729 pass, 1 skip, 0 fail, and the isolated files had 0 fail. Loop-test tripwire, 3 runs against main: 0 trips. - After the lint fix and a merge of main be78b8f: `make static-check` and `make lint-eslint` pass, and the plan's tests have 416 pass, 0 fail. - After the tombstone commit and a merge of main 9c841fa (coder#7426): typecheck, `make static-check`, `make lint-eslint`, `make crap-check` and `make dry-check` pass, and the plan's tests plus `workspaceRemoval.test.ts` have 428 pass, 0 fail. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$195.58`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=195.58 -->
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
…anscript index go through SessionFiles (coder#7430) ## Summary 5b PR 3 of phase 5 (Loop host I/O) of the agent-loop refactor. Four modules that an agent turn reaches read and write their files through injected file services instead of `fs/promises`, `os` and `writeFileAtomic`: - `additionalSystemContext.ts`: the scratchpad (`additional-system-context.md`) and its disabled marker. - `memoryContextPersistence.ts`: the memory-context file of the context window. - `runtimeScratchDir.ts`: the host scratch dir, and the Docker client config read of the devcontainer mount check. - `subagentTranscriptArtifacts.ts`: the sub-agent transcript index. The session-dir files go through `SessionFiles` (phase 2). The turn passes the `SessionFiles` of its HistoryService, so a memory layer keeps these files off the disk too. The Docker client config (`~/.docker`) is no session file, so that read goes through `HostFiles` and `HostPaths` (5a PR 5). Behavior does not change: every default is the Node layer (`nodeSessionFiles`, `nodeHostFiles`, `nodeHostPaths`), which calls the same `fs/promises` functions, `os.homedir()`, `ensurePrivateDir` and `writeFileAtomic`. ## Order Plan order: stack 5b is PR 1 to PR 9. This PR waits for phase 2 PR 7 and phase 4 PR 14 (coder#7425, merged), which changed `updateSubagentTranscriptArtifactsFile`. It uses no code of 5b PRs 4 and 5, so it opens on its own, with base main. ## Implementation - `additionalSystemContext.ts`: `readAdditionalSystemContext` and `writeAdditionalSystemContext` take an optional last `files` argument (default `nodeSessionFiles`). `ensurePrivateDir` is `files.ensurePrivateDir`. - `memoryContextPersistence.ts`: the `MemoryContextStore` constructor takes `files` (same default). `AgentSession` passes `historyService.files`. - `runtimeScratchDir.ts`: `ensureScratchDirForSpec` takes `files` after `abortSignal`, and `RemoteScratchDirCache.ensure` forwards an optional `files`. Both create the host and devcontainer-mount dirs, which are in the session dir. `canBindMountHostPathsIntoContainers` takes the home dir from `nodeHostPaths.userHome()` and gets a `HostFiles` argument. - `subagentTranscriptArtifacts.ts`: `readSubagentTranscriptArtifactsFile` takes `files`, and `updateSubagentTranscriptArtifactsFile` and `upsertSubagentTranscriptArtifactIndexEntry` take `files?` beside `locks`. `HistoryService` passes `this.files` to its 2 index reads. - `turnRequestBuilder.ts`: the scratchpad read and both scratch-dir calls pass `historyService.files`. - Callers outside the turn keep the Node default: `InstructionsService` (the scratchpad panel), the child-artifact roll-up of `WorkspaceService` (its copies still call `fs/promises`), `artifactsOperations.ts` and `terminalService.ts`. The plan estimated about 360 lines. This PR adds 266 and removes 47. ## Validation - Done check of the plan: `pairs.mjs --phase5` lists no host import in the 4 files (it listed 2 in each on main). - 5 new cases, each on memory file layers. With main's production files, all 5 fail: - `additionalSystemContext.test.ts`: the scratchpad and its marker are in the memory volume, the Node default reads none, and the temp dir on disk stays empty. - `memoryContextPersistence.test.ts`: save, load and discard work in memory, and the session dir on disk does not exist. - `runtimeScratchDir.test.ts`: `ensureScratchDirForSpec` creates the host dir in memory only. `canBindMountHostPathsIntoContainers` reads a client config that exists only in memory HostFiles and names a remote context (false), while the disk without a config gives true. - `historyService.test.ts`: a transcript index written through memory `SessionFiles` is in memory and not on disk, and `getSubagentTranscript` finds it by owner and by session scan. - The plan's tests (`additionalSystemContext.test.ts`, `agentSession*.test.ts`, `runtimeScratchDir.test.ts`, `historyService*.test.ts`) plus the golden and contract tests of `sessionFiles/`, `memoryContextPersistence`, `subagentTranscript*`, `instructionsService`, `turnRequestBuilder*`, `terminalService*`, `artifactsOperations*`, `DevcontainerRuntime*` and `sessionLocks/`: 2476 pass, 0 fail in 94 files. - `make static-check`, `make lint-eslint`, `make crap-check` and `make dry-check` pass. crap-check warns only that the `AgentSession` constructor and `prepareOrThrow` are touched hotspots, with unchanged complexity. - `make test` (host suite lock) on head 0c9c084: `bun test src` had 27908 pass, 7 skip, 1 fail. The failure was `BackgroundProcessManager > terminate > should deliver SIGTERM to the bash process` (a 3 s timeout under a host load of 13). Its file passed twice on a rerun (166 pass, 0 fail). The shared scripts/storybook/vscode run had 729 pass, 1 skip, 0 fail, and the 18 isolated files had 0 fail. Loop-test tripwire, 3 runs against main: 0 trips. - After a merge of main e89cf28 (coder#7426, coder#7428): a test-only commit drops a duplicate import, typecheck and the 4 static gates pass, and the related tests (the list above plus `workspaceRemoval`, `sessionUsageService*` and `utils/journal/`) have 2582 pass, 0 fail in 101 files. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$197.19`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=197.19 -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
5b PR 4 of phase 5 (Loop host I/O) of the agent-loop refactor. The durable event journal and the workspace removal tombstones read and write through the injected file services instead of
fs/promises:Journal,BlobStoreandDurableEventJournaluse theSessionFilesof their journal (phase 2).<rootDir>/locks/workspace-removed-<id>.json) is not a session file, soworkspaceRemoval.tspublishes and checks it throughHostFiles(5a PR 5). The removal also deletes the session dir through it.HistoryServicepasses its ownSessionFilestoisWorkspaceRemovalTombstoned(claim 3.5). The check needs onlyaccess, which both services have.Behavior does not change: every default is the Node layer (
nodeSessionFiles,nodeHostFiles), which calls the samefs/promisesfunctions andwriteFileAtomic.Order
Plan order: stack 5b is PR 1 to PR 9. This PR waits for phase 4 PR 12 (#7404, merged), which moved the journal and removal lock calls onto
SessionLocks. It uses no code of 5b PRs 3 and 5, so it opens on its own, with base main.Implementation
utils/journal/journal.ts:JournalOptions.files(absent:nodeSessionFiles, likelocks).mkdir,appendFile,readFile,statandopengo through it.utils/journal/blobStore.ts: the constructor takesfiles(defaultnodeSessionFiles). All 8 file calls go through it.utils/journal/durableEventJournal.ts: the constructor takesfilesafterlocks, with the same default, and gives it to itsJournalandBlobStore. The two callers that have a history service (cli/debug/cache-audit.ts,replay/replayFixture.ts) passhistoryService.files, as they pass its locks. The process-wide registry (sharedDurableEventJournal) keeps the Node layer, as phase 4 did for its locks.services/workspaceRemoval.ts:isWorkspaceRemovalTombstonedtakes an optional files argument (defaultnodeHostFiles).removeSessionDirUnderMemoryLocksandsealSubAgentForRemovalUnderMemoryLockstakefiles?: HostFilesbesidelocks, andpublishRemovalTombstonewrites through it. The rollback, lease and startup-heal reads keepfs/promises: the plan names only the access, the rm and the publish.services/historyService.ts: its 9 tombstone checks passthis.files.The plan estimated about 420 lines. This PR adds 163 and removes 39, because only the callers that already hold a history service pass their own files.
Validation
durableEventJournal.test.ts: a new case builds the journal on memorySessionFiles. The rows, the blob and the mention index (itsstat) live in the memory volume, and the temp dir on disk holds no journal file and no blob folder. With main's journal files, the case fails (ENOENT on the memory read).workspaceRemoval.test.ts: a new case removes a session dir through a memoryHostFiles. The tombstone exists in memory and not on disk, the memory session dir is gone, and a same-named dir on disk stays. With main'sworkspaceRemoval.ts, the case fails.historyService.test.ts: a new case gives HistoryService memorySessionFileswith a tombstone in the memory volume only.writePartialrefuses with "was removed". With main'shistoryService.tsandworkspaceRemoval.ts, the write succeeds and the case fails.utils/journal/,workspaceRemoval.test.ts,historyService.test.ts) plusreplay/,refinement/andsessionLocks.callOptions.test.ts: 577 pass, 0 fail.make static-check,make lint-eslint,make crap-checkandmake dry-checkpass.make lint-eslintfirst flagged an await of a non-promise in the newworkspaceRemoval.test.tscase, and a test-only commit fixed it.make test(host suite lock) on head 5e59214:bun test srchad 27900 pass, 7 skip, 0 fail. The shared scripts/storybook/vscode run had 729 pass, 1 skip, 0 fail, and the isolated files had 0 fail. Loop-test tripwire, 3 runs against main: 0 trips.make static-checkpasses, andutils/journal/,workspaceRemoval.test.tsandhistoryService.test.tshave 424 pass, 0 fail.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$189.95