Repository navigation
🤖 tests: put the session lock modules under local/no-host-imports with their 1 pair - #7355
Merged
Merged
Conversation
…ends take their locks through SessionLocks
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 · 12 replayed from cache · 15 model calls · 46.4k tokens (27% cached) All 14 tests in 9 files
e2e 0.20.0 · 2m 1s · 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 · 26.9s · 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 |
# Conflicts: # .oxlintrc.json # eslint.config.mjs # scripts/noHostImports.test.ts
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
This was referenced Oct 23, 2026
# Conflicts: # .oxlintrc.json # eslint.config.mjs # scripts/noHostImports.test.ts
ThomasK33
changed the title
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
… contract (coder#7367) ## Summary Adds two test layers for `SessionLocks` and runs the lock contract on four layers: node, memory, node + foreign and memory + foreign. It is PR 6 of phase 4. It changes only test harnesses and one test file. ## Order - Plan order: PR 6 comes after PR 5 (coder#7355), on top of PR 8 (coder#7354). - Landing order: PR 6 lands on main as its own PR, before PR 8 and PR 5. - Why: PR 6 waits only for PR 4 (coder#7345), which has merged. It uses no code from PR 8 or PR 5. PR 5 is held until coder#7360 merges, because both change the lint configs. Basing PR 6 on main keeps it out of that wait. ## Background Phase 4 moves the loop's lock calls behind `SessionLocks`. Later PRs (9 to 16) test the moved code with locks that need no real files and no real waits, and that play other backends. One contract runs on every layer, so the fakes cannot drift from the Node locks. ## Implementation - `memorySessionLocks.testHarness.ts`: `SessionLocksMemory` builds a fresh `MemoryLockTable` at each build of the layer. An acquire takes a free or dead lock at once. Otherwise it waits in FIFO order until a release hands the lock over, or until the `HostClock` passes the timeout. Under a TestClock the test moves time with `adjust`. Refusals use today's text (`fileLockTimeoutMessage`, `crossProcessLockTimeoutMessage`) and today's argument asserts. An aborted signal throws today's abort error. A record lock creates its missing parent, as Node does. Three parts stay on the Node locks: - Init-record locks (`foundByReaddir`: the path has the segment `/locks/init-record/`). `InitStateManager` lists their folder, so a holder in memory would leave no file and a live init would look abandoned. - Record locks with `createParentDirectory: false`. WorkflowRunStore needs their acquire to fail with ENOENT once the run directory is gone, with no window between a check and the take. Only the Node lock's own write gives that. - The use leases. The mutation gate, `isHeld` and `taskService` read other backends' lease files. Phase 5 moves them. - `foreignHolders.testHarness.ts`: `makeForeignHolders()` gives the test a handle (`hold`, `release`, `reclaimed`). `withForeignHolders(inner, holders)` wraps a layer. A live foreign holder refuses at once with today's text, and an aborted signal wins over it. A dead one is reclaimed by the next acquire, and its path goes to `reclaimed`. A probe reclaims nothing. `hold()` publishes the lease first and probes the gate second, as `WorkspaceUseLeases.hold` does. A live holder on `workspaceMutationLockPath` makes it release the lease and throw `WorkspaceMutationInProgressError` with today's text. `isHeld` counts a live fake lease in the workspace's lease folder. Every fake holder records an identity (`foreignIdentity()`), so its text equals the Node text for a current record. `liveBlocker(pid)` gives that blocker. Fake pids are `foreignPid(n)` = `process.pid + n`, so a fake holder is never this process. - `sessionLocks.contract.test.ts`: the contract runs on the three new layers. The Node text test now compares against `liveBlocker`, so the fake text equals the Node text for the child's pid. Four new contract cases run on all 4 layers: an aborted signal wins over a live holder, `createParentDirectory: false` on a missing parent rejects with ENOENT, an aborted signal wins over a missing parent, and a record lock creates its missing parent by default. Eight more cases: - memory: a 60 s timeout passes on `TestClock.adjust`, and it has not settled 1 ms before the timeout. - memory: a release hands the file lock to the waiters in FIFO order, and the old handle's `assertStillOwned` then throws. - memory: an init-record lock makes a real file, and inspect sees it. - memory: each build of `SessionLocksMemory` has its own table. - foreign: dead holders are reclaimed in order, and a probe reclaims none. - foreign: a live foreign mutator makes `hold()` refuse with today's text. - foreign: `hold()` publishes its lease before it probes the gate. - foreign: a live fake lease of another backend makes `isHeld` true. ## Validation - `bun test ./src/node/services/sessionLocks/`: pass. The contract file has 53 pass, 0 fail. - Scratch checks, each reverted: - Drop the live check in `wrapWithForeignHolders`: the 4 live cases of node + foreign and memory + foreign fail. - Drop `foundByReaddir`: the init-record case fails. - Share one table across builds of `SessionLocksMemory`: the fresh-table case fails. - Drop the abort check in the foreign layer: the abort case of both foreign layers fails. - Send `createParentDirectory: false` to the memory table instead of the Node locks: the ENOENT case of both memory layers fails. - Drop the memory layer's mkdir: the create-parent case of both memory layers fails. - Drop the fake-lease check in the foreign `isHeld`: the isHeld case fails. - Probe the fake gate before the lease: the gate-order case fails. - `make static-check`: pass. - `make crap-check`, `make dry-check`, `make mutate-check` (base = the parent commit): PASS with 0 functions in scope. The PR changes only test harnesses and a test file. - `make test` on 3e31a0aa6a, which is this diff on coder#7355's head 2f34c99: 27689 pass, 0 fail in the main run, and every other run passes. The second half (`tests/ui/storybook/`, `tests/ui/domIsolation.test.ts`, `vscode/src/`, `scripts/`): 732 pass, 0 fail. - Tripwire (`scripts/perf/loopTestBaseline.ts --runs 3`, base = coder#7355's head 2f34c99, on 3e31a0aa6a): 0 trips. - On the head 423f813: `bun test ./src/node/services/sessionLocks/`, `make static-check` and `make lint-eslint` pass. The tripwire and `make test` were not rerun after the move to main and the Codex fixes, because the PR changes only test files under `sessionLocks/`. ## Follow-ups - Codex round 4 (deferred, not a blocker): the memory layer's `acquireProcessFileLock` does not create the missing parent of `lockPath`, while `fileLock.ts` does. No test uses the memory layer yet. PR 9 adds the mkdir and a contract case before it moves any caller. Tracked in coder#7326. (PR 9 fixes this one.) - Codex round 5 (deferred, not a blocker): in the foreign-holder layer, a fake live holder on a record lock with `createParentDirectory: false` refuses with a timeout even when the parent is gone, where Node gives ENOENT. No test combines the two yet. The first PR that runs such a caller on `withForeignHolders` fixes it. Tracked in coder#7326. ## Risks Test-only. No production file imports the new harnesses. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$103.72`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=103.72 -->
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
…oder#7372) ## Summary `HistoryService` takes its session locks from `SessionLocks`, and exposes them as `readonly locks` for the owners that later PRs move. It is PR 9 of phase 4. The app keeps today's lock functions: `HistoryLive` yields `SessionLocksTag`, which `StoresLive` provides as `SessionLocksNode`. ## Order - Plan order: PR 9 comes after PR 8 (coder#7354) and PR 5 (coder#7355). - Landing order: PR 9 lands on main as its own PR, before coder#7354 and coder#7355 (phase 4 plan note 9). - Why: PR 9 waits for phase 2 PR 11 and phase 3 PR 7, which have merged. It uses no code from PR 8, PR 5 or the later phase 4 PRs. PR 15 and PR 16 need its `historyService.locks`, and phase 5 waits for those, so PR 9 is on the critical path. The lint pairs for `historyService.ts` that coder#7355 adds are not code. coder#7355 handles them when it next merges main. ## Implementation - `historyService.ts`: the constructor takes `locks: SessionLocks = nodeSessionLocks` as its third argument. The 9 `withLock` calls use `locks.workspaceFiles`, and the history write lock uses `this.locks.acquireProcessFileLock` with today's path, timeout and label. The file makes no value import of a lock module now. - `di/layers/core.ts`: `HistoryLive` passes `yield* SessionLocksTag`. - `testHistoryService.ts`: a `locks` option (default `SessionLocksNode`), built by the new `buildSessionLocks.testHarness.ts`. That builder lives apart from the contract harness, because the Jest integration tests (`tests/ipc`) load `testHistoryService.ts`, and `bun:test` does not resolve there. - `memorySessionLocks.testHarness.ts`: the memory file lock creates the folder of its lock path, as `fileLock.ts` does, but no lock file. This closes the round 4 follow-up of coder#7367 (tracked in coder#7326). Both folder writes are a synchronous `mkdirSync`, so waiters join the FIFO in call order. With an awaited mkdir, the contract case "a release hands the file lock to the waiters in FIFO order" failed 14 of 1500 reruns. - Tests: - `historyService.foreignHolder.test.ts`: a live foreign writer refuses at once with today's text (the text the Node lock gives after its 10 s timeout), and the history stays empty. A dead writer is reclaimed, and the next write goes on. - `historyFiles.golden.test.ts`: the phase 2 leak test also passes `SessionLocksMemory`, and it finds no file under `<root>/locks`. - Contract: a file lock creates its missing parent (4 layers), and a held memory file lock leaves no file at its path. ## Phase 5 The memory layer's folder side effects stay on `node:fs` in phase 4: the cross-process mkdir from coder#7367 and the file-lock mkdir from this PR. Phase 5's proof turn allows no such disk write. So phase 5's 5b PR 2 moves them onto an injected `HostFiles`, and it keeps the FIFO order by queueing each waiter before its await (phase 5 plan note 5). ## Validation All on the head (`6c5629c648`, this diff on main `eeddd295d9`): - `bun test ./src/node/services/sessionFiles/ ./src/node/services/sessionLocks/ ./src/node/services/historyService*.test.ts`: 944 pass, 0 fail. This includes the phase 2 golden and trace, the call record and the kill test. - The FIFO case with `--rerun-each 300`: 1500 pass, 0 fail. Before the synchronous mkdir: 14 of 1500 failed. - `make static-check`, `make lint-eslint`: pass. - `make test`: 27753 pass, 0 fail in the main run, and every other run passes. - Tripwire (`scripts/perf/loopTestBaseline.ts --runs 3 --base origin/main`): 0 trips. - `make crap-check`: PASS, 0 violations. `make mutate-check`: PASS, 0 mutants in scope (only the nearest test files of `historyService.ts` ran). - Scratch check, reverted: put the direct `acquireProcessFileLock` call back in `withHistoryWriteFileLock`, and the live case of `historyService.foreignHolder.test.ts` fails. - `rg -n "acquireProcessFileLock|workspaceFileLocks" src/node/services/historyService.ts` prints only the `this.locks.acquireProcessFileLock` call and one doc comment. ## Risks - One behavior is the same by construction: the Node layer is today's functions, so lock paths, timeouts, labels and texts do not change. The call record fixture (`sessionLocks/__fixtures__/callOptions.json`) is unchanged and its test passes. - `HistoryService` now holds a `locks` field. Callers that built it with one or two arguments keep the Node default. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$115.93`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=115.93 -->
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
…rom SessionLocks (coder#7379) ## Summary The session usage, goal and timing services take the workspace FIFO from `SessionLocks`. It is PR 7 of phase 4. Lock keys and order do not change: the Node layer gives today's `workspaceFileLocks`. ## Order - Plan order: PR 7 comes after PR 6, with PR 8 (coder#7354), and before PR 9. - Landing order: PR 7 lands on main as its own PR, after PR 9 (coder#7372), before PR 8 (coder#7354) and PR 5 (coder#7355) (recorded as a phase 4 plan note). - Why: PR 7 needs PR 9's `historyService.locks`, which has merged. It uses no code from PR 8 or PR 5. ## Implementation - `sessionUsageService.ts` (8 calls) and `workspaceGoalService.ts` (29 calls): `fileLocks` is a getter that returns `historyService.locks.workspaceFiles`. A memory or foreign-holder layer reaches these writes too. The getter reads at each use, not at construction, so tests that pass a partial `HistoryService` need no edit. - `sessionTimingService.ts` (4 calls): `new SessionTimingService(config, telemetry, locks = nodeSessionLocks)`. `desktop.ts` keeps its call. - `desktop.ts` and `core.ts` do not change. ## Validation All on the head (`c7cacdb5db`, this diff on main `4770fa63e0`): - `bun test` of `sessionUsageService*.test.ts`, `workspaceGoalService*.test.ts`, `sessionTimingService.test.ts` and `sessionLocks/`: 324 pass, 0 fail. The test files pass with no edit. The call record fixture is unchanged. - `make static-check`, `make lint-eslint`: pass. - `make test`: 27753 pass, 0 fail in the main run, and every other run passes. - Tripwire (`scripts/perf/loopTestBaseline.ts --runs 3 --base origin/main`): 0 trips. - `make crap-check`: PASS, 0 violations. `make mutate-check`: PASS, 0 mutants in scope. ## Risks - None for behavior: in the app, `historyService.locks.workspaceFiles` and the default are the same `workspaceFileLocks` map. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$122.03`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=122.03 -->
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
…om SessionLocks (coder#7385) Replaces coder#7380 (Codex security review hung there). ## Summary The todo and review pane tools take the workspace FIFO from `SessionLocks`. The turn request builder passes its history service's locks in the tool config. It is PR 15 of phase 4. Lock keys and order do not change: the Node layer gives today's `workspaceFileLocks`. ## Order - Plan order: PR 15 comes after PRs 10 to 14. - Landing order: PR 15 lands on main as its own PR, before PR 5 (coder#7355), PR 8 (coder#7354), PR 7 and PRs 10 to 14 (recorded as a phase 4 plan note). - Why: PR 15 waits for phase 3 PR 8 (coder#7277) and for PR 9's `historyService.locks` (coder#7372). Both have merged. It uses no code from the PRs it jumps. Phase 5's 5a PR 12 waits for it. ## Implementation - `common/utils/tools/tools.ts`: `ToolConfiguration` gains an optional `sessionLocks`. When it is absent, the tools use the Node layer, as for `clock` and `desktopNotifications` in configs that tests build by hand. - `turnRequestBuilder.ts`: the tool config gets `sessionLocks: historyService.locks`. - `tools/todo.ts`: `writeTodos` takes the locks. `todo_write` passes `config.sessionLocks`. `setTodosForSessionDir(ws, dir, todos, locks = nodeSessionLocks)` takes locks, and both app callers pass one: - `propose_plan.ts` passes `config.sessionLocks` as is. The default parameter covers undefined, so `createProposePlanTool` gains no branch. - `mock/mockAiStreamPlayer.ts` passes `deps.historyService.locks`. Tests and the call record scenarios keep the default. - `tools/review_pane.ts`: the read, apply and write of `review_pane_update` run under `locks.workspaceFiles`, from `config.sessionLocks`. - `todo.ts` and `review_pane.ts` import no lock module now. The lint pairs for these two files arrive with coder#7355. coder#7355 drops them when it next merges main. ## Validation - On the head `16124bf4be`: `bun test` of `tools/todo.test.ts`, `tools/review_pane.test.ts`, `tools/propose_plan*.test.ts`, `mock/`, `scripts/noHostImports.test.ts` and `sessionLocks/`: 128 pass, 0 fail. `make static-check`: pass. `make crap-check`: PASS, 0 violations (a warning on the touched hotspot `prepareOrThrow`, CC 308 unchanged). - On `3b158d7e39` (the first commit; the second one drops a `??` in `propose_plan.ts` and adds two comments): `make lint-eslint` passes. `make test` gives 27753 pass, 0 fail in the main run, and every other run passes. The tripwire (`scripts/perf/loopTestBaseline.ts --runs 3 --base origin/main`) reports 0 trips. - In `todo.ts` and `review_pane.ts`, `rg -nP '(?<![.\w])(acquireProcessFileLock\(|acquireCrossProcessLock\(|inspectCrossProcessLock\(|workspaceUseLeasesFor\(|workspaceFileLocks\b)'` prints nothing. ## Mutation check `make mutate-check` on `16124bf4be`: 148 killed, 2 equivalent, 150 survived, 94 not run. - Equivalent (2): the `config.sessionLocks ?? nodeSessionLocks` lines in `todo.ts` and `review_pane.ts`. Each has a `mutation-equivalent:` marker: `sessionLocks` is an object or undefined, so `??` and `||` agree. - Survived (150): every survivor is on a line that this PR does not change. Each is a pre-existing gap for PR 17, listed in coder#7326. - `TurnRequestBuilder.prepareOrThrow` (CC 308): 141. The PR adds one property line, `sessionLocks: this.dependencies.historyService.locks`, with no operator to mutate. - `createProposePlanTool`: 6, on lines 35, 47, 60, 69, 91 and 115. - `createReviewPaneUpdateTool`: 3, on lines 182, 184 and 206. - Not run (94): past the default budget of 300 mutants per run (`mutate_report.py --max-mutants`). The scope is large because `prepareOrThrow` is a CC 308 hotspot. ## Risks - None for behavior. In the app, `historyService.locks` is the Node layer, so every tool uses today's `workspaceFileLocks` FIFO. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$122.03`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=122.03 -->
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
…heir locks from SessionLocks (coder#7378) ## Summary The init-record locks, the session file FIFO and the plan migration lock go through `SessionLocks`. It is PR 16 of phase 4. Every new `locks` argument is optional, and its default is the Node layer, so the app's lock paths, timeouts, labels and texts do not change. ## Order - Plan order: PR 16 comes last in phase 4, after PR 15. - Landing order: PR 16 lands on main as its own PR, before PR 5 (coder#7355), PR 8 (coder#7354), PR 7 and PRs 10 to 14 (recorded as a phase 4 plan note). - Why: PR 16 has no wait of its own except PR 9's `historyService.locks` (coder#7372), which has merged. It uses no code from the PRs it jumps. Phase 5's 5a PR 13 waits for it. - Phase end: the phase 4 end checks run on the last phase 4 merge commit, not on this one. ## Implementation - `initStateManager.ts`: `new InitStateManager(config, locks = nodeSessionLocks)`. These calls go through `locks`: - the init lease probe (`useLeasesFor(config).isHeld(ws, "init")`) - the init-record lock (`acquireCrossProcessLock`) and its probe (`inspectCrossProcessLock`) - the write barrier (`workspaceFiles.withLock`) `core.ts` keeps `new InitStateManager(config)`. - `utils/sessionFile.ts`: `new SessionFileManager(config, fileName, locks = nodeSessionLocks)`. `eventStore.ts` and the other callers keep the default. - `utils/runtime/planLocation.ts`: the migration lock is `locks.acquireProcessFileLock`. `resolvePlanFileLocation`, `markRemotePlanMigratedForDeletion` and `importLegacyRemotePlan` take an optional `locks` and pass it through `migrateRemotePlan` to `withMigrationLock`. The private `withMigrationLock(config, id, locks, fn)` takes `locks` before `fn`, so the migration body keeps its indentation. - Callers that pass `this.historyService.locks` (the plan's sites): - `agentSession.ts`: `resolvePlanFileLocation` for the post-compaction attachments. - `workspaceService.ts`: `importLegacyRemotePlan`, both `markRemotePlanMigratedForDeletion` calls, and `getPlanFileLocation`'s `resolvePlanFileLocation`. - All other callers keep the default. They are `findImportableLegacyPlanPath`, which the plan does not list, and the calls in `taskService.ts`, `workspaceOperations.ts`, `planReviewService.ts` and `turnRequestBuilder.ts`. In the app, `historyService.locks` is the same Node layer, so this changes no behavior. ## Validation All on the head (`dc17e3374e`, this diff on main `4770fa63e0`): - `bun test` of `initStateManager.test.ts`, `workspaceLeasesRepro.test.ts`, `workspaceService.remotePlanNamespace.test.ts`, `workspaceService.planStorageRepro.test.ts`, `planReviewService.ensureSnapshot.test.ts` and `sessionLocks/`: 173 pass, 0 fail. The call record fixture is unchanged, and its test passes. - `make static-check`, `make lint-eslint`: pass. - `make test`: 27753 pass, 0 fail in the main run, and every other run passes. - Tripwire (`scripts/perf/loopTestBaseline.ts --runs 3 --base origin/main`): 0 trips. - `make crap-check`: PASS, 0 violations. - `core.ts` and `eventStore.ts` do not change. In the 3 moved files, `rg -nP '(?<![.\w])(acquireProcessFileLock\(|acquireCrossProcessLock\(|inspectCrossProcessLock\(|workspaceUseLeasesFor\(|workspaceFileLocks\b)'` prints nothing. ## Mutation check `make mutate-check`: 49 mutants, 21 killed, 28 survived. Every survivor is on a line that this PR does not change. Each one is a pre-existing gap for PR 17, listed in coder#7326. | Function | Lines (head) | Survivors | | --- | --- | --- | | `AgentSession.buildAttachmentsFromContext` | 13546, 13598-13600 | 9 | | `InitStateManager` constructor, `readUnownedRunningInit`, `acquireRecordLock`, `releaseRecordClaim`, `isRecordLockHeld` | 140, 511, 524, 557, 568, 573 | 6 | | `WorkspaceService.importLegacyPlan`, `markRemotePlanMigratedForRemoval`, `deletePlanFilesOfMetadata` | 6076-6077, 9002-9004, 18771 | 11 | | `markRemotePlanMigratedForDeletion`, `migrateRemotePlan` (planLocation.ts) | 141, 263 | 2 | ## Risks - None for behavior: every default and every `historyService.locks` in the app is the Node layer. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$122.03`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=122.03 -->
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
…n SessionFiles (coder#7391) ## Summary 5a PR 12 of phase 5 (Loop host I/O) of the agent-loop refactor. The todo tools and the review pane tools read and write their session files (`todos.json`, `assistedReview.json`) through the session's `SessionFiles`, the history layer's file service of phase 2. So 4 frozen pairs of `local/no-host-imports` leave the allow map. The map goes from 10 pairs in 5 files to 6 pairs in 3 files. Behavior does not change: the Node layer calls the same `fs/promises` functions and the same `writeFileAtomic`. ## Order This PR waited for phase 4 PR 15 (coder#7385, merged as dab0392), which gave these tools the workspace FIFO from `SessionLocks`. Phase 2 PR 7 (coder#7312) is merged. Plan order is PR 1 to PR 14, so it keeps its place. ## Implementation - `tools.ts`: `ToolConfiguration` gains an optional `sessionFiles`. `turnRequestBuilder.ts` fills it from the `HistoryService`. A config without it gets the Node layer, as configs that tests build by hand do today. - `todo.ts` and `review_pane.ts`: every read, `mkdir`, atomic write and remove of the session file goes through `config.sessionFiles`, with the Node layer as the default. - `todos/todoStorage.ts` and `reviewPane/assistedReviewStorage.ts`: the readers take the files as a last argument with no default, and so does `setTodosForSessionDir`. So the type checker lists every caller. `propose_plan.ts`, `agentSession.ts` and the mock stream player pass the session's files. The callers outside the agent loop (`workspaceService.ts`, `goalStatusBoard.ts`) pass `nodeSessionFiles` explicitly. - The todo reminder (`TodoListReminderSource`) and the turn-end todo nudge (`needsTodoTurnEndNudge`) read the list through the session's files, as `todo_write` writes it. - `eslint.config.mjs`, `.oxlintrc.json` and `scripts/noHostImports.test.ts`: the 4 pairs (`todo.ts` and `review_pane.ts`, each with `fs/promises` and `@/node/utils/writeFileAtomic`) leave the allow map and the frozen list. ## Validation - 2 new cases, one in `todo.test.ts` and one in `review_pane.test.ts`, run the tools on the memory `SessionFiles`. Each reads the state back from the memory volume and checks that the temp folder on disk stays empty. With main's `todo.ts` and `review_pane.ts`, both new cases fail. With this PR, both pass. - The plan's list (`todo.test.ts`, `review_pane.test.ts`, `propose_plan.test.ts`, `services/mock/`, `scripts/noHostImports.test.ts`): 70 pass, 0 fail. - `make static-check` and `make lint-eslint`. - `make test` (host suite lock) on head 7c75dc7: `bun test src` had 27807 pass, 7 skip, 0 fail. The ui line had 729 pass, 1 skip, 0 fail. The shared scripts/storybook/vscode run and all isolated files passed with 0 fail. Loop-test tripwire, 3 runs against main: 0 trips. - The branch first merged main dab0392 with the merge base 16124bf (the coder#7385 head that merged). The diff against main had the same changes as the diff against 16124bf. The one exception was `.oxlintrc.json`, where main already had the formatter layout, so only the 4 allow entries changed. - Round 1 (d9e4781, e860bc2): the todo reminder and the turn-end nudge read through the session's files, and the 3 helpers lose their Node default. 2 new cases run on memory `SessionFiles` (the reminder through `getToolsForModel`, the nudge of a session with memory history files). With the readers of the previous head, both fail. With the fix, both pass. The plan's list plus `agentNotifications/`, `agentSession.turnStopHooks`, `sessionLocks/`, `goalStatusBoard`, `workspaceService.metadata`, `workspaceService.sendMessage`, `tools.test` and `tools.modelOnlyNotifications`: 383 pass, 0 fail. `make static-check`, `make lint-eslint`, `make crap-check`, `make dry-check`. - Round 4 (ac9d932, 8a92b12) merges main a59ebb3 (coder#7378 and coder#7355). coder#7378 added a `nodeSessionLocks` import to the lock scenarios harness, so the merge declared it twice, and CI's Static Checks failed. The harness now imports it once. The allow map keeps coder#7355's new pair. On the merge: `make static-check`, `make lint-eslint`, `make crap-check`, and the round 1 test list: 383 pass, 0 fail. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$148.96`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=148.96 -->
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
…rvice (coder#7392) ## Summary 5a PR 13 of phase 5 (Loop host I/O) of the agent-loop refactor. Agent-loop code builds its runtimes through a new `RuntimeFactory` service, so the in-memory loop can give its `FakeRuntime` in place of a concrete runtime. So 5 frozen pairs of `local/no-host-imports` leave the allow map. On main (with 5a PR 12, coder#7391) the map has 6 pairs in 3 files. With this PR, 1 pair is left: `agentSession.ts @/node/services/workspaceUseLeases`, which coder#7355 added. Behavior does not change: the Node layer calls today's functions. ## Order This PR waited for phase 4 PR 16 (coder#7378, merged as 6004b7c) and phase 3 PR 5 (coder#7274, merged), which change the same functions of `agentSession.ts`. coder#7391 landed first, and this PR merges it. They conflicted only in the allow map (`eslint.config.mjs`, `.oxlintrc.json`, `scripts/noHostImports.test.ts`). ## Implementation - `runtimeFactory/runtimeFactoryService.ts`: the `RuntimeFactory` interface. Its members are `createRuntime`, `createRuntimeForWorkspace`, `createRuntimeContextForWorkspace`, `createMultiProjectRuntime` (the multi-project branch of `AIService.createWorkspaceRuntimeContext`) and `createLocalRuntime`. Each has the signature of today's function. The module has only type imports, so `di/tags.ts` can import it with no cycle. `createRuntime` and `createLocalRuntime` have no caller yet. 5a PR 14 moves the reached runtime callers (`agentResolution.ts`, `continuousCompactionSummary.ts`, `planReviewService.ts`, `workspaceProjectRepos.ts`) onto them. - `runtimeFactory/nodeRuntimeFactory.ts`: the Node layer, which calls today's functions. `StoresLive` provides it under `RuntimeFactoryTag`. - `di/layers/core.ts` and `turnRequestBuilder.ts`: `AILive` passes the factory to `AIService` through `LoopHostServices`. - `aiService.ts`: every runtime that `AIService` builds goes through `this.runtimeFactory`. A hand-built `AIService` without one gets the Node layer. - `agentSession.ts` and `contextManagement/strategies/tokenBudget.ts`: they build their runtimes through `aiService.runtimeFactory`, with the Node layer as the default for test doubles of `AIService`. - `runtimeFactory/fakeRuntimeFactory.testHarness.ts`: `fakeRuntimeFactory` and `RuntimeFactoryFake` give the memory host's `FakeRuntime` for every member. - `eslint.config.mjs`, `.oxlintrc.json` and `scripts/noHostImports.test.ts`: the 5 pairs (`agentSession.ts`, `aiService.ts` and `tokenBudget.ts` with `@/node/runtime/runtimeHelpers`, and `aiService.ts` with `@/node/runtime/runtimeFactory` and `@/node/runtime/multiProjectRuntime`) leave the allow map and the frozen list. ## Validation - `aiService.test.ts`: a new case gives `AIService` a `fakeRuntimeFactory` and checks that the workspace runtime context holds the memory host's runtime. With main's `aiService.ts`, the case fails. With this PR, it passes. - The plan's list (`aiService.test.ts`, `agentSession.pinnedBudget.test.ts`, `contextManagement/`, `runtime/runtimeHelpers.test.ts`, `scripts/noHostImports.test.ts`): 255 pass, 0 fail. - `make static-check` and `make lint-eslint`. - `make test` (host suite lock) on head f72b3e9: `bun test src` had 27807 pass, 7 skip, 1 fail. The failure was `sshAskpass` "handles two sequential prompts without ignoring the second", a timing flake in code that this PR does not touch: the file passed 10 of 10 reruns. The rest of `make test` (the shared scripts/storybook/vscode run: 729 pass, 1 skip, 0 fail, and the 18 isolated files: 0 fail) passed. Loop-test tripwire, 3 runs against main: 0 trips. - The branch merged main 6004b7c with a merge base that holds coder#7378's merged head afe126d and the main that the branch merged before. Each file's diff against main equaled its diff against that base. - Then it merges main a59ebb3 (coder#7355). The allow map conflicted, and the resolution removes exactly the 5 pairs of this PR and keeps coder#7355's new pair. The other 11 files keep the same diff against main. On that head (d7c3c41): `make static-check`, `make lint-eslint`, `make crap-check`, the plan's list (255 pass, 0 fail) and the proof above. - Round 2 (3639619) merges main 646ce43 (coder#7391). The allow map conflicted, and the resolution again removes exactly the 5 pairs of this PR, so 1 pair is left. On that head: `make static-check`, `make lint-eslint`, `make crap-check`, and the plan's list with the tests of coder#7391 (todo, review_pane, propose_plan, mock/, turnStopHooks, tools.modelOnlyNotifications): 360 pass, 0 fail. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$151.37`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=151.37 -->
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
…oder#7394) ## Summary AgentSession takes its turn use lease from the history service's `SessionLocks`. It is PR 10 of phase 4. The lease path, the 0 s wait and the refusal text do not change: the Node layer gives today's `workspaceUseLeasesFor`. ## Order - PR 10 of phase 4. It is the bottom of a native stack: PRs 11 to 14 follow on top of it. - coder#7355 (PR 5) has merged. It put the lock modules under `local/no-host-imports` with this file's pair, and this PR removes the pair. - It uses `historyService.locks` from PR 9 (coder#7372), which has merged. ## Implementation - `agentSession.ts`: `beginTurnUseLease` calls `this.historyService.locks.useLeasesFor(this.config).hold(...)`. `WorkspaceMutationInProgressError` comes from `sessionLocks.ts` (the same class). A type-only import of `WorkspaceUseLease` stays, and the rule allows it. - `agentSession.useLease.test.ts`: a live foreign gate case. The foreign-holder layer plays another backend's mutation gate with no file on disk, and the turn refuses at once with today's text and never reaches the provider. - `.oxlintrc.json`, `eslint.config.mjs`, `scripts/noHostImports.test.ts`: the `agentSession.ts -> workspaceUseLeases` pair goes. It was the last pair: the allow map and the frozen pairs are now empty. ## Validation On the head `89a8f2d2f4` (main merged in after coder#7392 and coder#7393): - `make static-check` passes, with the oxlint and ESLint configs in parity. `make lint-eslint` passes. - `bun test` of `agentSession*.test.ts`, `workspaceUseLeases.test.ts` and `scripts/noHostImports.test.ts`: 1340 pass, 0 fail. - `make crap-check`, the tripwire, `make test` and `make mutate-check` run on this head now. I will update this list when they finish. On the previous head `fcda462f51`, every gate passed: - `make crap-check`: PASS, 0 violations. - The tripwire (`scripts/perf/loopTestBaseline.ts --runs 3 --base origin/main`): 0 trips. - `make test`: 27832 pass, 0 fail in the main run, and every other run passes. - `make mutate-check`: PASS. The changed lines have no mutable operator (0 mutants). Other checks: - Revert check: I restored the direct `workspaceUseLeasesFor` call in `agentSession.ts`, and the new case "a live foreign mutation gate refuses B's turn..." failed. - `rg -nP '(?<![.\w])workspaceUseLeasesFor\(' src/node/services/agentSession.ts` prints nothing. - The merges of main: coder#7355 landed as a squash, so the first merge brought back the pair that this PR drops. The commit `fcda462f51` drops it again. coder#7392 then removed the other pairs from the allow map. The second merge keeps both removals, so the map is empty. The diff against main has the 5 files above and nothing else. ## Risks - None for behavior: in the app, `historyService.locks` is `nodeSessionLocks`, which calls the same `workspaceUseLeasesFor`. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$146.84`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=146.84 -->
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
Puts the five session lock modules under
local/no-host-importsfor the agent-loop files, with the 1 (file, module) pair that those files still import. It is PR 5 of phase 4.Background
Phase 4 moves the loop's lock calls behind
SessionLocks. With the lock modules inHOST_MODULES, a loop file that adds a new direct lock import fails lint. The allow map holds today's imports, and the later move PRs remove them:agentSession.ts->workspaceUseLeases(PR 10)The PR first listed 5 pairs. 4 of them are gone, because the PRs that moved those imports merged first:
historyService.ts->workspaceFileLocks,fileLock: 🤖 refactor: HistoryService takes its session locks from SessionLocks #7372 (PR 9).tools/todo.ts->workspaceFileLocksandtools/review_pane.ts->workspaceFileLocks: 🤖 refactor: the todo and review pane tools take the workspace FIFO from SessionLocks #7385 (PR 15, which replaced 🤖 refactor: the todo and review pane tools take the workspace FIFO from SessionLocks #7380).Implementation
eslint.config.mjs:HOST_MODULESgains@/node/utils/concurrency/workspaceFileLocks,@/node/utils/concurrency/fileLock,@/node/utils/concurrency/processLiveness,@/node/utils/main/crossProcessLockand@/node/services/workspaceUseLeases.mutexMapstays off the list:tools/file_edit_operation.tskeeps a pure in-memory map.HOST_IMPORT_ALLOWgains the 1 pair..oxlintrc.json: the same modules and pair.scripts/noHostImports.test.ts: the frozen pairs gain the same 1.#7381 (PR 8) merged, so this PR now has base main. The branch has one merge of main, which resolves the 3 lint files.
Validation
bun test ./scripts/noHostImports.test.ts: 6 pass, 0 fail.make static-checkandmake lint-eslint: pass. The rule fails a stale allow entry, so the 1 pair is in use.make check-oxlint-config:.oxlintrc.jsonmatcheseslint.config.mjsfor every lint file.make teston 1afc6e2: 27804 pass, 1 fail under host load (load average about 13):backgroundProcessManager.test.ts("gives concurrent same-name spawns from two backends distinct directories on a non-host runtime"). The file passes alone on this head (166 of 166). The PR changes no runtime code.--runs 3 --base origin/main): 0 trips.make crap-check: PASS, CC 0 -> 0.make mutate-check: PASS with 0 mutants. The PR changes no production function.Risks
Lint-only. A loop file that later adds a direct lock import fails lint until its PR lists the pair.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$135.57