Repository navigation
🤖 refactor: HostFiles and HostPaths services with Node and memory layers - #7319
Merged
Merged
Conversation
Member
Author
|
@codex review |
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. |
🛡️ 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. |
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
Contributor
🟢 e2e stories (Haiku): 14 passed27 agent steps · 7 replayed from cache · 30 model calls · 206.8k tokens (73% cached) All 14 tests in 9 files
e2e 0.20.0 · 2m 57s · 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 · 20.3s · 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 23, 2026
This was referenced Oct 23, 2026
Merged
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
…Files and HostPaths (coder#7322) ## Summary 5b PR 6 of phase 5 (Loop host I/O) of the agent-loop refactor. Plugin discovery, its journals and its registry now read host files through `HostFiles` and the user home through `HostPaths` (coder#7319). The three files (`agentPlugins/discovery.ts`, `journals.ts`, `registry.ts`) import no `node:fs`, `node:fs/promises` or `node:os` any more. Behavior does not change: each call goes to the Node layers, which read the `node:fs/promises` and `node:os` namespaces at call time, so the spies of the existing tests still apply. The frozen pairs of `local/no-host-imports` stay at 44 in 26 files. ## Order The plan order of stack 5b is PR 1 to PR 9. 5b PRs 2 to 5 wait for phase 4 and phase 2 PRs that have not merged. This PR waits only for 5a PR 5 (coder#7319, merged), and it uses no code of 5b PRs 2 to 5. So it lands now, with base main, as the bottom of the new stack 5b. ## Implementation - `hostFiles/hostFiles.ts`: `HostFiles.promises` also gives `open` and `constants`. `readPluginFileWithinRootCapped` reads a plugin file through one handle opened with `O_NONBLOCK`, so a FIFO cannot hang it and the bytes come from the inode it checked. Both layers already have the two members (the Node layer is the namespace itself, the memory layer is the memfs `promises`). - `hostFiles.contract.testHarness.ts`: one new case that both layers pass. `open` with `O_RDONLY | O_NONBLOCK` reads through the handle, its bigint `stat` names the inode of `lstat`, and `open` of a missing file fails with ENOENT. - `discovery.ts`: `nodePluginFs` and `readPluginFileWithinRootCapped` use `nodeHostFiles.promises`, and `computeAgentPluginContainers` takes `~/.agents/plugins` from `nodeHostPaths.userHome()`. - `journals.ts`, `registry.ts`: `nodeHostFiles.promises` in place of the `node:fs/promises` import. The plan estimated about 450 lines for this PR. It adds 64 lines and removes 28, because no caller passes its own HostFiles or HostPaths yet, so this PR adds no parameter for one. A whole discovery run on a memory host also needs `ensurePathContained` (`tools/skillFileUtils.ts`) on HostFiles. That file belongs to 5a PR 6, and the services that would pass a memory host belong to 5b PR 8. ## Validation - The plan's "no host import" check, an import scan with the module list of `local/no-host-imports`, run on the three files: before, `discovery.ts` had `fs/promises`, `fs` and `os`, and `journals.ts` and `registry.ts` had `fs/promises`. After, the three files have no pair. - `bun test ./src/node/services/agentPlugins/`: 826 pass, 0 fail. - `bun test ./src/node/services/hostFiles/`: 30 pass (the new case on both layers). - `make static-check`, `make lint-eslint`. - `make test` (host suite lock): 27683 pass, 7 skip, 3 fail. The fails were timeouts under the suite's load, and each file passes alone: `backup/gitRepo.test.ts` (2 BackupRepoCache cases) passes 61 of 61, and `bash_output.test.ts` (a known flake) passes 14 of 14. Neither file imports the agentPlugins or hostFiles code. - The ui line (`scripts/` tests and the others of CI's line): 725 pass, 1 fail. The fail was a timeout in `scripts/perf/desktopColdStart.test.ts` while another test process ran on the host. Alone it passes 2 of 2. - Loop-test tripwire, 3 runs against origin/main 9c1eb89: 0 trips. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$71.07`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=71.07 -->
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
## Summary 5a PR 6 of phase 5 (Loop host I/O) of the agent-loop refactor. The skill read tools now use `HostFiles` and `HostPaths` (coder#7319) for their host calls. `tools/skillFileUtils.ts` and `tools/agent_skill_list.ts` import no `fs/promises` or `node:os` any more, so their 3 frozen pairs of `local/no-host-imports` leave the allow map. On the base (5afa16c) the map holds 40 pairs in 25 files. On this branch it holds 37 pairs in 23 files. Behavior does not change: each call goes to the Node layers, which read the `node:fs/promises` and `node:os` namespaces at call time. ## Order The plan order of stack 5a is PR 1 to PR 14. 5a PRs 3 and 4 wait for phase 3 PR 8 (coder#7277). This PR waits only for 5a PR 5 (coder#7319, merged), and it changes no file of 5a PRs 3 and 4. So it lands now, with base main, ahead of them. ## Implementation - `tools/skillFileUtils.ts`: `lstat` and `realpath` go through `nodeHostFiles.promises`. That covers `ensurePathContained`, `isSameHostDirectory` and the skill path checks. - `tools/agent_skill_list.ts`: `readdir`, `stat`, `readFile` and `realpath` go through `nodeHostFiles.promises`, and the user home comes from `nodeHostPaths.userHome()`. - `agentSkills/agentSkillsService.ts:2` and `tools/agent_skill_list.test.ts:2` import `node:os` as a namespace (`import * as os`), not as the default export. `nodeHostPaths` reads the namespace, and Bun applies a `spyOn(os, "homedir")` only to the import form the test uses. So the test's spy now reaches both the tool and `agentSkillsService.ts:282`, as the plan's claim 1.4 says. - `eslint.config.mjs`, `.oxlintrc.json` and `scripts/noHostImports.test.ts`: the 3 pairs leave the allow map and the frozen list. ## Validation - `bun test` of the plan's list (`agent_skill_list`, `agent_skill_read`, `agent_skill_read_file`, `agentSkills/`, `scripts/noHostImports.test.ts`): 267 pass, 0 fail. - Scratch probe: with the test's old default `os` import, 6 `agent_skill_list` tests fail, because the spy misses `nodeHostPaths`. With the namespace import they pass. I reverted the probe. - An import scan with the module list of `local/no-host-imports` (phases 2 and 4 included): 42 pairs in 25 files before, 39 in 23 after. - `make static-check`, `make lint-eslint`. - `make test` (host suite lock): 27702 pass, 7 skip, 1 fail. The fail is the known `backgroundProcessManager.test.ts` flake (SIGTERM delivery timing). Alone, under the lock, the file passes 163 of 163. - The ui line (`scripts/` tests and the others of CI's line): 726 pass, 0 fail. - Loop-test tripwire, 3 runs against origin/main 42c551e: 0 trips. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$74.48`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=74.48 -->
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
… HostPaths (coder#7338) ## Summary 5a PR 7 of phase 5 (Loop host I/O) of the agent-loop refactor. The skill write tools and the skills catalog clone now use `HostFiles` and `HostPaths` (coder#7319) for their file calls. `tools/agent_skill_delete.ts`, `tools/agent_skill_write.ts` and `tools/skillsCatalogFetch.ts` import no `fs/promises` or `os` any more, so their 4 frozen pairs of `local/no-host-imports` leave the allow map. The map goes from 37 pairs in 23 files (5a PR 6, coder#7329) to 33 pairs in 20 files. Behavior does not change: each call goes to the Node layers, which read the `node:fs/promises` and `node:os` namespaces at call time. ## Order This PR is stacked on 5a PR 6 (coder#7329), as in the plan. It does not use code from coder#7329, but both PRs remove neighboring lines of the same allow map. 5a PRs 3 and 4 still wait for coder#7277, and this PR changes none of their files. ## Implementation - `tools/agent_skill_delete.ts`, `tools/agent_skill_write.ts`: `nodeHostFiles.promises` in place of the `fs/promises` namespace import. - `tools/skillsCatalogFetch.ts`: the named `fs/promises` imports (`lstat`, `mkdtemp`, `readFile`, `readdir`, `realpath`, `rm`, `stat`) become `nodeHostFiles.promises` calls, and `tmpdir()` becomes `nodeHostPaths.tmpDir()`. The git and fetch calls stay. 5a PR 9 moves them onto HostProcesses. - `tools/agent_skill_write.test.ts:2` imports `node:os` as a namespace, like `agentSkillsService.ts` since coder#7329. The test's `spyOn(os, "homedir")` then reaches the home that `agentSkillsService.ts` reads, so the test no longer reads the real `~/.mux/skills`. A scratch probe showed that the test also passes with the old default import, because the real home holds no skill of that name. So no assertion detects this change. - `eslint.config.mjs`, `.oxlintrc.json` and `scripts/noHostImports.test.ts`: the 4 pairs leave the allow map and the frozen list. ## Validation - `bun test` of the plan's list (`agent_skill_delete`, `agent_skill_write`, `skillsCatalogFetch`, plus `scripts/noHostImports.test.ts`): 124 pass, 0 fail. The 16 `spyOn(fsPromises, ...)` spies of `skillsCatalogFetch.test.ts` still control the clone directory and the reads, so the namespace calls reach them. - An import scan with the module list of `local/no-host-imports` (phases 2 and 4 included): 39 pairs in 23 files before, 35 in 20 after. - `make typecheck`, `make static-check`, `make lint-eslint`. - `make test` (host suite lock, host load average near 66): 27699 pass, 7 skip, 4 fail. All 4 were 5-second timeouts. Alone, under the lock, `compactionPreparationLifecycle.test.ts` and `agentPlugins/installService.installMethods.test.ts` pass 104 of 104. - The ui line (`scripts/` tests and the others of CI's line): 726 pass, 0 fail. - Loop-test tripwire, 3 runs against 5a PR 6 (b03fa1a): 0 trips. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$74.48`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=74.48 -->
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
…eof (coder#7351) ## Summary 5a PR 3 of phase 5 (Loop host I/O) of the agent-loop refactor. Agent-loop tools no longer use `instanceof LocalBaseRuntime` or `instanceof RemoteRuntime`. They read a new `Runtime.location` field instead. So `tools/bash.ts`, `tools/skillFileHint.ts` and `tools/agent_skill_read_file.ts` no longer import a runtime class, and their 3 frozen pairs of `local/no-host-imports` leave the allow map. On main (7265ac0) the map holds 22 pairs in 13 files. On this branch it holds 19 pairs in 11 files. Every runtime gives the same answer as `instanceof` did, so behavior does not change. ## Order The plan order of stack 5a is PR 1 to PR 14. This PR waited for phase 3 PR 8 (coder#7277, merged at 11:41 UTC), because `bash.ts` changed there. 5a PRs 5, 6 and 7 (coder#7319, coder#7329, coder#7338) merged before this PR. They change no source file of this PR. coder#7329 and coder#7338 removed neighboring allow-map lines, so merge commit d156ebc keeps the deletions of both sides. 5a PR 4 is stacked on this PR. ## Implementation - `runtime/Runtime.ts`: optional `readonly location?: "local" | "devcontainer" | "remote"`, and two helpers. `isLocalBaseRuntime(runtime)` is true for "local" and "devcontainer". `isRemoteRuntime(runtime)` is true for "remote". - `LocalBaseRuntime.ts`, `DevcontainerRuntime.ts`, `RemoteRuntime.ts`: `location` is a prototype getter. Subclasses and the `Object.create(...prototype)` doubles of tests inherit it. `MultiProjectRuntime` and plain object doubles leave it absent, and `instanceof` is false for them too. - `tools/bash.ts:1041` and `:1064`, `tools/skillFileHint.ts`, `tools/agent_skill_read_file.ts`: the helpers in place of `instanceof`. - `utils/ipAddress.ts`: one more change (commit 4927205). The check of the impossible `net.isIP` answer now uses the shared `assert` helper instead of a hand-written `throw`, as the repo's assertion style asks. It changes no reachable path. - `eslint.config.mjs`, `.oxlintrc.json` and `scripts/noHostImports.test.ts`: the 3 pairs leave the allow map and the frozen list. ## Validation - `src/node/runtime/runtimeLocation.test.ts` (new): a table of 12 runtime classes (the 8 runtime classes, `MultiProjectRuntime` and the 3 test doubles of `tools/testHelpers.ts`). For each class, `isLocalBaseRuntime` and `isRemoteRuntime` give the `instanceof` answers on an `Object.create` instance. A runtime without `location` takes neither branch. - `bun test` of the plan's list (`src/node/runtime/`, `bash`, `skillFileHint`, `agent_skill_read_file`) plus `ipAddress.test.ts` and `scripts/noHostImports.test.ts`: 760 pass, 0 fail. - `make typecheck`, `make static-check`, `make lint-eslint`. - `make test` (host suite lock), on head b2593a1 (main b3911c8 merged in, before the merge of the current main): 27765 pass, 7 skip, 0 fail, and every isolated file passes. - The ui line on that head: 732 pass, 0 fail. - Loop-test tripwire on that head, 3 runs against main: 0 trips. - After merging main (d156ebc): `make typecheck`, `make static-check`, `make lint-eslint` and the targeted list (760 pass) again. 5a PR 4 contains this tree, and its full gates ran on its final head. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$85.60`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=85.60 -->
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
…ifications (coder#7352) ## Summary 5a PR 4 of phase 5 (Loop host I/O) of the agent-loop refactor. The notify tool shows OS notifications through a new `DesktopNotifications` service, not through its own `import("electron")`. So `tools/notify.ts` loses its frozen `electron` pair of `local/no-host-imports`. The allow map goes from 19 pairs in 11 files (5a PR 3) to 18 pairs in 10 files. Outside Electron the tool returns the same result text as before. ## Order This PR is stacked on 5a PR 3, as in the plan. It waited for phase 3 PR 8 (coder#7277, merged), because `ToolConfiguration` and `turnRequestBuilder.ts` changed there. 5a PR 5 (coder#7319, merged) also added tags to `StoreTags` in `di/tags.ts`. Merge commit f542862 joins both tag lists. 5a PR 3 (coder#7351) merged at 15:40 UTC, so this PR now targets main. Merge commit c525854 brings in main 0856ca4. It keeps both imports and both layer entries in `di/layers/stores.ts`, where phase 4 PR 4 (coder#7345) added `SessionLocksNode`. In the lint configs it keeps main's lines and removes only the notify.ts pair. ## Implementation - `desktopNotifications/desktopNotifications.ts`: the `DesktopNotifications` interface with one method, `show({ title, body?, workspaceId? })`, which resolves `{ success, error? }`. - `desktopNotifications/electronDesktopNotifications.ts`: the Node layer. `createElectronDesktopNotifications(host)` takes the Electron check and the electron import, so tests can pass a fake electron module. It holds the code that `notify.ts` had before (the `import("electron")`, its eslint-disable and today's error text outside Electron). It is not in `AGENT_LOOP_FILES`. - `recordingDesktopNotifications.testHarness.ts`: a test layer that records each request and returns a set result. - `di/tags.ts`, `di/layers/stores.ts`, `di/layers/core.ts`: `DesktopNotificationsTag` joins `StoreTags`, `StoresLive` gives the Electron layer, and `AILive` passes it to `AIService`. - `aiService.ts`, `turnRequestBuilder.ts`, `common/utils/tools/tools.ts`: `AIService` takes optional loop host services and `TurnRequestBuilder` puts `desktopNotifications` on each `ToolConfiguration`. A config without it (tests that build one by hand) uses the Electron layer. - `tools/notify.ts`: calls `config.desktopNotifications.show(...)`, and falls back to the browser notification as before when nothing shows. ## Validation - `tools/notify.test.ts`: 2 new cases. The tool sends the trimmed title, body and workspace ID to `config.desktopNotifications` and reports `notifiedVia: "electron"`. When the service shows nothing, the tool reports `notifiedVia: "browser"`. - `electronDesktopNotifications.test.ts` (new): outside Electron the result is today's text. In Electron (a fake electron host, commit c27edf3), the layer cuts the title and body to the OS limits, shows nothing on a platform without notifications, reports a failed import, and on a click restores and focuses the first window and sends the workspace ID. Quality / CRAP had flagged the moved function as new and untested (CRAP 34). With these tests a local `crap-check` with coverage no longer reports it. - `bun test` of `notify`, `desktopNotifications/`, `turnRequestBuilder` and `scripts/noHostImports.test.ts`: 50 pass, 0 fail. - After merge commit c525854: `make static-check`, and the `notify`, `desktopNotifications/`, `turnRequestBuilder`, `sessionLocks/`, `di/` and `scripts/noHostImports.test.ts` tests (89 pass, 0 fail). - `make typecheck`, `make static-check`, `make lint-eslint`. After c27edf3: `make typecheck`, oxlint and eslint on the folder, and the `notify` and `desktopNotifications/` tests (13 pass). - `make test` (host suite lock) on head b882a24 (before c27edf3, which changes only the Electron layer and its test): the `bun test src` run had 27771 pass, 7 skip and 1 fail. The fail is the known flake `BackgroundProcessManager > spawn > gives concurrent same-name spawns from two backends distinct directories on a non-host runtime`. That file alone then passed (163 pass, 0 fail). `make test` stops at the first failing run, so I ran its other steps by hand: the shared scripts/storybook/vscode run (729 pass, 0 fail) and each of the 18 isolated files (0 fail). - The ui line on that head: 732 pass, 0 fail. - Loop-test tripwire on that head, 3 runs against main: 0 trips. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$85.60`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=85.60 -->
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
…er#7360) ## Summary 5a PR 8 of phase 5 (Loop host I/O) of the agent-loop refactor. The AGENTS.md tools (`xum_agents_*`), the config file helper of the config tools and the repo-dir check of `bash.ts` make their host file calls through `HostFiles` (5a PR 5, coder#7319). So 6 frozen pairs of `local/no-host-imports` leave the allow map. The map goes from 18 pairs in 10 files (5a PR 4) to 12 pairs in 5 files. Behavior does not change: the Node layer calls the same `fs/promises` and `writeFileAtomic` functions. ## Order 5a PR 3 (coder#7351) merged at 15:40 UTC and 5a PR 4 (coder#7352) at 17:05 UTC, so this PR now targets main. The merge queue removed it once at 17:05 UTC: `Test / Stories` failed because its Sonnet probe got HTTP 502 twice, before any story ran. Merge commit 9c3218d brings in main 32b80b2. Its only conflicts were the allow-map lines next to the notify.ts pair, and it keeps this PR's removals. Plan order is PR 1 to PR 14, so this PR keeps its place. It touches `bash.ts` and the allow-map lines next to the lines that PRs 3 and 4 change, so it builds on them and does not conflict with them. ## Implementation - `tools/xum_agents_path.ts`, `tools/xum_agents_read.ts`, `tools/xum_agents_write.ts`: `nodeHostFiles.promises` in place of the `fs/promises` namespace import, as 5a PRs 6 and 7 did for the skill tools. - `tools/bash.ts:969`: `assertLocalRepoDirectoryExists` calls `nodeHostFiles.promises.stat`. It was the only use of the `fs` import, so the import goes. - `tools/shared/configReadWrite.ts`: `readConfigDocumentUnvalidated` and `writeConfigDocument` take a last `files: HostFiles` argument that defaults to `nodeHostFiles`. The read, the `lstat` symlink check, the `mkdir` and both atomic writes go through it. The callers (`xum_config_read.ts`, `xum_config_write.ts`) do not change. The async write passes `{ encoding: "utf-8" }` in place of the string `"utf-8"`, because the `HostFiles` member takes only the options object. `writeFileAtomic` resolves both forms to the same options. - `eslint.config.mjs`, `.oxlintrc.json` and `scripts/noHostImports.test.ts`: the 6 pairs leave the allow map and the frozen list. ## Validation - `tools/shared/configReadWrite.test.ts` (new): on the Node layer (a temp dir) and on the memory layer (`createMemoryHost`), a missing file reads as `{}`, a written `config.json` reads back with a new `writeId`, a written `providers.jsonc` keeps its comment header and reads back, and a write to a symlinked `config.json` throws and leaves the link's target unchanged. 8 pass. - `bun test` of the plan's list (`xum_agents`, `bash`, `shared/configReadWrite`) plus `xum_config_read`, `xum_config_write` and `scripts/noHostImports.test.ts`: 158 pass, 0 fail. - `make static-check`, `make lint-eslint`. - `make test` (host suite lock) on head 1ae3fe1 (before the merge of 5a PR 4): 27780 pass, 7 skip, 0 fail, then 729 pass, 0 fail, and each of the 18 isolated files 0 fail. - The ui line on that head: 732 pass, 0 fail. - Loop-test tripwire on that head, 3 runs against main: 0 trips. - After merge commit 9c3218d: `make static-check` and the targeted list (158 pass, 0 fail). `git diff origin/main` of the three lint files shows only the 6 pairs. - After merge commit 7a1c206 (5a PR 4 at c525854, with main 0856ca4): `make static-check`, `make lint-eslint`, and the targeted list plus `notify` and `desktopNotifications/` (171 pass, 0 fail). The merge had one conflict: the `hostClock` import line of `bash.ts`, where main added `nodeClock`. The merge keeps both imports. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$90.60`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=90.60 -->
azaj01
pushed a commit
to azaj01/mux
that referenced
this pull request
Oct 25, 2026
…through HostFiles, HostPaths and HostClock (coder#7371) ## Summary 5b PR 7 of phase 5 (Loop host I/O) of the agent-loop refactor. The instruction readers and the home-clutter reminder reach the host through the phase 5 and phase 3 services: - `instructionFiles.ts` and `homeClutterWatch.ts` read files through `HostFiles` (coder#7319). - `systemMessage.ts` and `tools.ts` take the user home and the Xum home from `HostPaths` (coder#7319). - `HomeClutterReminderSource` arms its scan timeout on the session's `HostClock` (phase 3). The five files import no `fs`, `fs/promises` or `os` any more, and `systemMessage.ts` no longer imports `getXumHome`. Behavior does not change: the Node layers read the same `fs/promises` and `os` namespaces and call `getXumHome` at each call, so the `os.homedir` spies of `systemMessage.test.ts` and `instructionsService.test.ts` still apply. None of the five files is in the `local/no-host-imports` file set, so the frozen pairs do not change. ## Order Plan order: stack 5b is PR 1 to PR 9, and this PR comes after 5b PR 6 (coder#7322, merged). Landing order: this PR lands ahead of 5b PRs 2 to 5, by rule 2 of plan note 1. Those PRs wait for phase 4 and phase 2 PRs that have not merged. This PR waits only for 5a PR 5 (coder#7319, merged) and phase 3 PR 1 (coder#7270, merged), and it uses no code of 5b PRs 2 to 5. So it opens on its own, with base main. No open phase 2 to 5 PR changes these five files. ## Implementation - `utils/main/instructionFiles.ts`: the local file reader calls `nodeHostFiles.promises.readFile`. - `runtime/homeClutterWatch.ts`: the default `readDir` calls `nodeHostFiles.promises.readdir`. - `services/systemMessage.ts`: the Claude-compat global dir comes from `nodeHostPaths.userHome()`. The global instruction set, the settings `config.json` path and its directory come from `nodeHostPaths.xumHome()`. - `agentNotifications/sources/HomeClutterReminderSource.ts`: the constructor takes an optional `clock` (absent: `nodeClock`, as for tool configs that tests build by hand). The scan timeout calls `clock.setTimer`, and the scan start times come from `clock.nowMs()`. - `common/utils/tools/tools.ts`: the reminder gets `nodeHostPaths.userHome()` and `config.clock`. The plan estimated about 320 lines for this PR. It adds 71 lines and removes 21, because no caller passes its own `HostFiles` or `HostPaths` yet (the same reason as in 5b PR 6). ## Validation - `HomeClutterReminderSource.test.ts`: a new case runs a poll on a stuck scan with the TestClock. The poll is still open at 249 ms of TestClock time and ends at 250 ms, with no real wait. With the timer pointed back at `nodeClock`, the case fails. - The plan's tests (`instructionFiles`, `systemMessage`, `instructionsService`, `homeClutterWatch`, `HomeClutterReminderSource`) plus `tools.test.ts`: 135 pass, 0 fail. - `make static-check`, `make lint-eslint`, `make dry-check`, `make crap-check`. - `make test` (host suite lock) on head c9af576: `bun test src` had 27679 pass, 7 skip and 1 fail. The fail is `renderer first load > the entry's static imports do not reach the tool definitions module`, which fails the same way on main 2e2a2b7 (I ran it there alone). `make test` stops at the first failing run, so I ran its other steps by hand: the shared scripts/storybook/vscode run (729 pass, 0 fail) and each of the 18 isolated files (0 fail). - The ui line on that head: 732 pass, 0 fail. - Loop-test tripwire, 3 runs against origin/main 2e2a2b7: 0 trips. - Round 2 (77a0664) merges main 2633118 (coder#7369, the fix for the renderer first load test). On it: `make static-check`, and the plan's tests plus `tools.test.ts` and `rendererFirstLoad.test.ts`: 138 pass, 0 fail. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$112.49`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=112.49 -->
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
5a PR 5 of phase 5 (Loop host I/O) of the agent-loop refactor. It adds two services,
HostFiles(the fs/promises calls andwriteFileAtomicthat agent-loop files make outside the session dirs) andHostPaths(the Xum home, the user home and the temp folder), each with a Node layer and a memfs test layer, and one contract suite that both layers pass. No caller changes: later 5a PRs move the skill, agent-file and config tools onto them. The frozen pairs oflocal/no-host-importsstay at 44 in 26 files.Order
The plan order is 5a PR 1, 2, 3, 4, 5. 5a PRs 3 and 4 wait for phase 3 PR 8 (#7277), so this PR lands first and becomes the bottom of the new stack 5a. None of 5a PRs 3, 4 and 5 uses code from another. 5a PR 4 also appends to
di/tags.ts(StoreTags). A merge commit resolves that overlap with a merge commit when PR 4 joins the stack.Implementation
src/node/services/hostFiles/hostFiles.ts: the two interfaces, type-only imports, sodi/tags.tsimports them without a cycle.nodeHostFiles.ts: the Node layers. Each member reads its binding at call time:promisesis thenode:fs/promisesnamespace itself, andHostPathscallsos.homedir(),os.tmpdir()andgetXumHome()on each call. Tests spy on those namespaces (skillsCatalogFetch.test.ts on fsPromises, systemMessage.test.ts and agent_skill_read.test.ts onos.homedir), so the moved callers keep those spies working.memoryHostFiles.testHarness.ts:createMemoryHost()gives both services on one fresh memfs volume, with the three host-like fixes of SessionFilesMemory (phase 2 PR 6, 🤖 tests: add the SessionFilesMemory layer on memfs #7262). That file now exportscreateFixedMemfs()for this. SessionFilesMemory itself is unchanged in behavior.di/tags.ts:HostFilesTagandHostPathsTagjoinStoreTags, andStoresLivegives the Node layers, next toSessionFilesNode.Validation
bun test ./src/node/services/hostFiles/: 28 pass. The contract (12 cases per layer) covers each member and each error code a caller branches on (ENOENT, EEXIST, ENOTDIR, EISDIR, ENOTEMPTY), symlinks,writeFileAtomicandwriteFileAtomicSync(no temp file left, mode kept). With a plain memfs volume (no fixes) the memory layer fails 2 cases (unlink of a directory, rename onto a full directory), so the suite tells the fixes apart. Two memory hosts share no file. Spies onnode:fs/promises,os.homedirandos.tmpdir, and a change ofXUM_ROOT, reach the Node layers at call time.bun test ./src/node/services/sessionFiles/: passes (SessionFilesMemory oncreateFixedMemfs()).make static-check,make lint-eslint.make test(host suite lock): 28324 pass, 7 skip, 3 fail.WorkspaceMenuBar.test.tsx:1263fails on main too (🤖 tests: pageTimingCore and WorkspaceMenuBar tests fail in their make test-unit half and pass alone #7297). The 2bash_outputfails are known flakes: alone,bash_output.test.tspasses 14 of 14.scripts/tests): 725 pass, 1 fail. The fail is thescripts/perfpageTimingCore timeout that main shows too (🤖 tests: pageTimingCore and WorkspaceMenuBar tests fail in their make test-unit half and pass alone #7297).Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$67.24