Repository navigation
🤖 perf: keep toolDefinitions out of the main chunk and pin the Node 22.23.4 sandbox image - #7369
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. |
Contributor
🟢 e2e stories (Haiku): 14 passed27 agent steps · 7 replayed from cache · 32 model calls · 230.1k tokens (71% cached) All 14 tests in 9 files
e2e 0.20.0 · 3m 14s · 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 · 24.3s · electron · run artifacts 🟢 e2e agent repros (Haiku): no tests selectedSpend: $0.00 of the $1.00 cap (proxy ledger) e2e 0.20.0 · 6.6s · 2 targets (web, phone) · run artifacts |
openAskUserQuestions.ts (eager via StreamingMessageAggregator) imported TOOL_DEFINITIONS only to parse ask_user_question args, which put every tool schema into main-*.js. Move the ask_user_question arg schemas to askUserQuestionSchemas.ts (zod only); toolDefinitions re-exports them. (cherry picked from commit fd6b719)
ibetitsmike
changed the title
Contributor
Author
|
Codex Review: Didn't find any major issues. Swish! 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
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 -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 27, 2026
…oder#7671) ## Summary The "Bug-bash sandbox image" workflow now publishes the image by itself. A push to main that changes an image input runs the publish job. The job creates the image only if the key has none yet: each inputs key gets one stable tag, `ghcr.io/coder/xum-bugbash-sandbox:inputs-<key>`. A key that has its tag pushes nothing. This is PR 1 of 2. Runners still trust only the digest in `image.json` until PR 2. ## Background An image change takes four steps today, and two of them need a person. coder#7362 changed the Dockerfile. Its story lanes refused the stale image, so it needed an admin merge, and the merge queue then stayed red for 3.5 hours. After that, a maintainer ran the manual publish on main, and coder#7369 pinned the new digest. Four images went through that process between Oct 8 and Oct 23. The plan, which the user approved: main publishes once per key with no person (this PR). Then every sandbox job pulls `inputs-<key>`, checks its build attestation (`gh attestation verify --source-ref refs/heads/main`), and builds locally when there is no verified image (PR 2). `image.json` and the `BUGBASH_SANDBOX_BUILD_STALE` opt-in go away in PR 2. ## Implementation 1. **New trigger:** `push` to main, filtered to the image inputs (`build.sh --key` hashes the Dockerfile, `build.sh` and the playwright-core version in `bun.lock`) and to this workflow file. A change to this file therefore publishes the current key once. `Makefile` is not in the push filter. A Makefile change does not change the key, and an existing key needs no publish. 2. **Create if missing:** the publish step looks up the stable tag with `docker buildx imagetools inspect`. If the tag exists, the step writes "exists" to the summary and stops. Only a `: not found` error counts as missing. Any other lookup error fails the job. If the tag is missing, the step runs the publish as before. 3. **Tag last:** Attest signs the new digest. Only then does a new step, "Tag inputs-<key>", point the stable tag at it with `imagetools create --prefer-index=false`. That flag copies the manifest as it is. The default wraps a single manifest in a new index with a new, unsigned digest. The step reads the tag's digest back and fails unless it equals the signed digest. The stable tag marks the key as done, so a failed Attest leaves no tag, and a re-run publishes again. 4. **No races:** the existing concurrency group runs one publish at a time and never cancels it. A second push of the same key waits, then finds the tag and stops. 5. **Docs:** AGENTS.md and the bug-bash skill now say that the merge push publishes the image. A manual run on main retries a failed publish. This PR does not change any image input, so the inputs key stays `2bf9e12d…`, the key in `image.json`. No old-style publish is needed for this PR. ## Validation - I ran the step's lookup branch against GHCR with an empty Docker config, with these results: - The current key's stable tag: `PUBLISH: … missing`. - An existing published tag: `SKIP: … exists`. - An unreachable registry: `FAIL` with exit 1, and the DNS error printed. - I ran the "Tag inputs-<key>" step's script against a local registry, with a single-platform Docker v2 manifest like the published image. It kept the source digest. Without `--prefer-index=false`, the tag got a different digest. - `actionlint` 1.7.13: clean. `make lint-zizmor` and `make static-check`: pass. - After the merge, I will check two runs. The merge push must publish `inputs-2bf9e12d3ac7dcb10397698df2792bf1b244b3fcbe68225378302ef695ba302a`. Then a manual run on main must report "exists" and push nothing. I will link both runs here. ## Risks Low. Publishing stays main-only, from the run's own commit, with a fresh build and an attestation. A new path is the one trigger: every `bun.lock` change on main starts a run that ends after the lookup, at about one minute of runner time each. A failed lookup fails the run and pushes nothing. <details> <summary>Decisions in the approved plan</summary> 1. A job can build the image itself in CI and on a developer machine. 2. Only main pushes images, after a merge. The merge queue does not push. 3. A job uses a registry image only if its attestation shows that main's publish workflow built it. Otherwise it builds locally. 4. Two PRs: this publish change first, then the runner, `build.sh` and docs. </details> --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh` • Cost: `$263.47`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=xhigh costs=263.47 -->
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
This PR fixes the two problems that keep main red, in one PR, because each fix fails CI without the other. The first commit is Thomas's fix from #7364 (cherry-picked, his authorship kept). It moves the ask_user_question argument schemas out of
toolDefinitions.ts, so the renderer entry chunk no longer pulls in every tool schema andrendererFirstLoad.test.tspasses again. The second commit pins the bug-bash sandbox image that main published from #7362 (basenode:22.23.4-slim, the CVE-2026-111497 fix), so the model-driven story jobs run again on main and in the merge queue. Fixes #7363.Background
openAskUserQuestions.ts, which the eagerStreamingMessageAggregatorloads, importTOOL_DEFINITIONS. That movedtoolDefinitions.tsand its imports intomain-*.js(+92 KB raw, +23.7 KB br) and failsTest / Unit (5/6): "renderer first load > the entry's static imports do not reach the tool definitions module".2bf9e12d…, butimage.jsonstill pins the old image.make test-storiesrefuses the old record ("sandbox refused: stale image: image.json has inputs key 08a81da3ddeb1ec1, this checkout 2bf9e12d3ac7dcb1").Test / StoriesandTest / Stories (Haiku)count inRequired(STORIES_MODEandSTORIES_PR_MODEareblock), so the queue removed 🤖 refactor: agent-file, config and bash stat calls use HostFiles #7360 and 🤖 perf: keep toolDefinitions out of the main chunk (#7363) #7364 on that failure.So #7364 alone fails the story jobs, and the pin alone fails the unit test. The queue groups entries with
ALLGREEN, so neither can merge first. This PR carries both.Implementation
perf: keep toolDefinitions out of the main chunk (#7363)is fd6b719 from 🤖 perf: keep toolDefinitions out of the main chunk (#7363) #7364 with no change (the samegit patch-id). The newsrc/common/utils/tools/askUserQuestionSchemas.tsimports only zod and holds the three ask_user_question schemas.toolDefinitions.tsimports and re-exports them, andopenAskUserQuestions.tsparses withAskUserQuestionToolArgsSchemadirectly. 🤖 perf: keep toolDefinitions out of the main chunk (#7363) #7364 has the bundle measurements: the main chunk is back within 750 B br of the build before 🤖 feat: keep open ask_user_question forms sticky above the chat input #6887. 🤖 perf: keep toolDefinitions out of the main chunk (#7363) #7364 can close after this merges.tests/bugbash/sandbox/image.jsongets this record, written with the samejq -ncommand thatbuild.sh --pushuses:digestsha256:e3804b5418a8e6783698d807c75cc256efae45b81b03400644ae7f13364c9a03inputsKey2bf9e12d3ac7dcb10397698df2792bf1b244b3fcbe68225378302ef695ba302a, whatmake -s bugbash-sandbox-keyprints on this commitplaywrightCore1.63.0(unchanged)publishedFrom2e2a2b769f417aedcecc09f33abb379d6c8be7b1(main, the #7362 merge)Validation
rendererFirstLoad.test.tsfails on the pin commit alone (21f9c3f) and passes with the cherry-pick.2e2a2b769f. The digest is the subject of its build provenance attestation.gh attestation verify oci://ghcr.io/coder/xum-bugbash-sandbox@sha256:e3804b54… --repo coder/xumpasses. The signer is.github/workflows/bugbash-sandbox-image.yml@refs/heads/main(workflow_dispatch, run 40016150145), and the source commit is2e2a2b769f.org.xum.bugbash.inputslabel is2bf9e12d…a302a, itsorg.xum.bugbash.playwright-corelabel is1.63.0, andnode -vin it printsv22.23.4.Co-authored-by: Thomas Kosiewski tk@coder.com