Repository navigation
🤖 refactor: skill write tools and the catalog clone use HostFiles and HostPaths - #7338
Merged
Merged
Conversation
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. |
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. 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. |
Contributor
🟢 e2e stories (Haiku): 14 passed27 agent steps · 7 replayed from cache · 32 model calls · 229.8k tokens (71% cached) All 14 tests in 9 files
e2e 0.20.0 · 3m 5s · web · run artifacts 🟢 e2e Electron stories (Haiku): 3 passed3 agent steps · 3 model calls · 8.1k tokens (21% cached) All 3 tests in 3 files
e2e 0.20.0 · 19.8s · 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 |
This was referenced Oct 23, 2026
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 -->
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 7 of phase 5 (Loop host I/O) of the agent-loop refactor. The skill write tools and the skills catalog clone now use
HostFilesandHostPaths(#7319) for their file calls.tools/agent_skill_delete.ts,tools/agent_skill_write.tsandtools/skillsCatalogFetch.tsimport nofs/promisesorosany more, so their 4 frozen pairs oflocal/no-host-importsleave the allow map. The map goes from 37 pairs in 23 files (5a PR 6, #7329) to 33 pairs in 20 files. Behavior does not change: each call goes to the Node layers, which read thenode:fs/promisesandnode:osnamespaces at call time.Order
This PR is stacked on 5a PR 6 (#7329), as in the plan. It does not use code from #7329, but both PRs remove neighboring lines of the same allow map. 5a PRs 3 and 4 still wait for #7277, and this PR changes none of their files.
Implementation
tools/agent_skill_delete.ts,tools/agent_skill_write.ts:nodeHostFiles.promisesin place of thefs/promisesnamespace import.tools/skillsCatalogFetch.ts: the namedfs/promisesimports (lstat,mkdtemp,readFile,readdir,realpath,rm,stat) becomenodeHostFiles.promisescalls, andtmpdir()becomesnodeHostPaths.tmpDir(). The git and fetch calls stay. 5a PR 9 moves them onto HostProcesses.tools/agent_skill_write.test.ts:2importsnode:osas a namespace, likeagentSkillsService.tssince 🤖 refactor: skill read tools use HostFiles and HostPaths #7329. The test'sspyOn(os, "homedir")then reaches the home thatagentSkillsService.tsreads, 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.jsonandscripts/noHostImports.test.ts: the 4 pairs leave the allow map and the frozen list.Validation
bun testof the plan's list (agent_skill_delete,agent_skill_write,skillsCatalogFetch, plusscripts/noHostImports.test.ts): 124 pass, 0 fail. The 16spyOn(fsPromises, ...)spies ofskillsCatalogFetch.test.tsstill control the clone directory and the reads, so the namespace calls reach them.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.tsandagentPlugins/installService.installMethods.test.tspass 104 of 104.scripts/tests and the others of CI's line): 726 pass, 0 fail.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$74.48