Repository navigation
Conversation
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.
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 review |
|
Perf owner gate decision (head fd6b719): I accept Generated with |
|
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". |
🟢 e2e stories (Haiku): 14 passed27 agent steps · 7 replayed from cache · 31 model calls · 221.9k tokens (70% cached) All 14 tests in 9 files
e2e 0.20.0 · 3m 3s · web · run artifacts 🟢 e2e Electron stories (Haiku): 3 passed3 agent steps · 3 model calls · 8.0k tokens (21% cached) All 3 tests in 3 files
e2e 0.20.0 · 20.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 |
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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. |
|
Perf owner readiness record for head
Generated with |
|
Perf owner: the merge queue removed this PR at 19:46Z (merge-group run 40012690165). The only failing job is Generated with |
|
Perf owner: closing as superseded. #7369 (merged 2633118) cherry-picked this PR's commit together with the sandbox image pin. Generated with |
|
Your commit fd6b719 is on main now: #7369 carried it unchanged (same patch-id), and the squash commit 2633118 has a Main was deadlocked. This PR alone failed the queue's story jobs on the stale sandbox image key from #7362, and the image pin alone failed |
…2.23.4 sandbox image (coder#7369) ## 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 coder#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 and `rendererFirstLoad.test.ts` passes again. The second commit pins the bug-bash sandbox image that main published from coder#7362 (base `node: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 coder#7363. ## Background - coder#6887 made `openAskUserQuestions.ts`, which the eager `StreamingMessageAggregator` loads, import `TOOL_DEFINITIONS`. That moved `toolDefinitions.ts` and its imports into `main-*.js` (+92 KB raw, +23.7 KB br) and fails `Test / Unit (5/6)`: "renderer first load > the entry's static imports do not reach the tool definitions module". - coder#7362 changed the sandbox Dockerfile, so the inputs key on main is now `2bf9e12d…`, but `image.json` still pins the old image. `make test-stories` refuses the old record ("sandbox refused: stale image: image.json has inputs key 08a81da3ddeb1ec1, this checkout 2bf9e12d3ac7dcb1"). `Test / Stories` and `Test / Stories (Haiku)` count in `Required` (`STORIES_MODE` and `STORIES_PR_MODE` are `block`), so the queue removed coder#7360 and coder#7364 on that failure. So coder#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 (coder#7363)` is fd6b719 from coder#7364 with no change (the same `git patch-id`). The new `src/common/utils/tools/askUserQuestionSchemas.ts` imports only zod and holds the three ask_user_question schemas. `toolDefinitions.ts` imports and re-exports them, and `openAskUserQuestions.ts` parses with `AskUserQuestionToolArgsSchema` directly. coder#7364 has the bundle measurements: the main chunk is back within 750 B br of the build before coder#6887. coder#7364 can close after this merges. - `tests/bugbash/sandbox/image.json` gets this record, written with the same `jq -n` command that `build.sh --push` uses: | Field | Value | | ---------------- | ---------------------------------------------------------------------------------------------------------------------------- | | `digest` | `sha256:e3804b5418a8e6783698d807c75cc256efae45b81b03400644ae7f13364c9a03` | | `inputsKey` | `2bf9e12d3ac7dcb10397698df2792bf1b244b3fcbe68225378302ef695ba302a`, what `make -s bugbash-sandbox-key` prints on this commit | | `playwrightCore` | `1.63.0` (unchanged) | | `publishedFrom` | `2e2a2b769f417aedcecc09f33abb379d6c8be7b1` (main, the coder#7362 merge) | ## Validation - `rendererFirstLoad.test.ts` fails on the pin commit alone (21f9c3f) and passes with the cherry-pick. - Publish run on main: [40016150145](https://github.com/coder/xum/actions/runs/40016150145), from `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/xum` passes. The signer is `.github/workflows/bugbash-sandbox-image.yml@refs/heads/main` (workflow_dispatch, run 40016150145), and the source commit is `2e2a2b769f`. - I pulled the image by digest. It is linux/amd64, its `org.xum.bugbash.inputs` label is `2bf9e12d…a302a`, its `org.xum.bugbash.playwright-core` label is `1.63.0`, and `node -v` in it prints `v22.23.4`. Co-authored-by: Thomas Kosiewski <tk@coder.com> --------- Co-authored-by: Thomas Kosiewski <tk@coder.com>
Summary
Fixes #7363. #6887 made the eager renderer entry chunk (
main-*.js) grow by 92 KB raw / 23.7 KB br. The cause was not the ask_user_question dock:openAskUserQuestions.ts, whichStreamingMessageAggregatorloads eagerly, importedTOOL_DEFINITIONSonly to callTOOL_DEFINITIONS.ask_user_question.schema.safeParse. That pulledtoolDefinitions.tsand its imports (every tool schema) out of the lazytoolCardsLoaderchunk into main. This PR moves the three ask_user_question argument schemas to a zod-only module, so main is back within 750 B br of the build before #6887.Background: step 0 (bundle diff)
I built the renderer (
make build-renderer, Bun 1.4.3) at 32b80b2 (before #6887) and at 75d4c2b (#6887), and attributedmain-*.jsbytes per source through its source map (sources+ generated columns).AskUserQuestionDock,ToolMessage,AskUserQuestionToolCall,ChatPaneandaskUserQuestionPlacementare all in the lazy page chunk. A grep forAskUserQuestionDockandtool-card-fallbackinmain-*.jsfinds 0 hits. So the dock code-split that perf: #6887 ask_user_question dock adds ~24 KB to the entry chunk and slows seeded load (TBT +40 ms mobile, LCP +52 ms desktop) #7363 suggested would move about 0 B. The perf owner dropped it.toolDefinitions.tswas intoolCardsLoader-*.js. At 75d4c2b it is inmain-*.js.Modules that #6887 added to
main-*.js(generated bytes in main), and where they are on this head:src/common/utils/tools/toolDefinitions.tssrc/common/orpc/schemas/workspace.tssrc/common/analytics/schemaSql.tssrc/common/orpc/schemas/artifacts.tssrc/common/orpc/schemas/workspaceAiSettings.tssrc/browser/utils/messages/openAskUserQuestions.tssrc/common/utils/artifactKind.tssrc/common/types/workspaceTurn.tssrc/common/utils/tools/toolInputFilePath.tssrc/common/constants/computerUse.tssrc/common/constants/contextBudget.tssrc/common/config/schemas/configOperations.tssrc/common/constants/memory.tssrc/common/types/tools.tsnode_modules/zod-to-json-schema/dist/esm/parsers/string.jssrc/common/utils/tools/toolCatalog.tssrc/common/constants/subagentLifecycle.tssrc/constants/agentMessaging.tssrc/constants/taskMessages.tsImplementation
src/common/utils/tools/askUserQuestionSchemas.ts(imports only zod) holdsAskUserQuestionOptionSchema,AskUserQuestionQuestionSchemaandAskUserQuestionToolArgsSchema, moved unchanged fromtoolDefinitions.ts.toolDefinitions.tsimports and re-exports them, so its callers (common/types/tools.ts,workspaceService.ts, …) do not change.TOOL_DEFINITIONS.ask_user_question.schemais the same object as the new module'sAskUserQuestionToolArgsSchema.openAskUserQuestions.tsparses with that schema directly and no longer importstoolDefinitions.ts.Validation (gates declared by the perf owner before measuring)
Module gate. Source-map diff of
main-*.js: vs 75d4c2b, 18 of the 19 modules are gone, andtoolDefinitions.tsis back intoolCardsLoader-*.js.openAskUserQuestions.tsstays (see the table). A grep for atoolDefinitions.tsstring (Requires UI interaction) inmain-*.jsfinds 1 hit at 75d4c2b and 0 here. New eager code vs 32b80b2: +1,751 B generated in total (askUserQuestionSchemas.ts819,openAskUserQuestions.ts611, 🤖 feat: keep open ask_user_question forms sticky above the chat input #6887's ownaskUserQuestionSummary.ts+113 andStreamingMessageAggregator.ts+106, and 102 B of identifier noise in files with no source change). Budget 2,048 B.Main chunk size (local builds, same setup):
main-*.jsrawmain-*.js.brbun scripts/perf/firstLoadJs.ts dist, total of 4 first-load files (raw / br / gz KiB): base 2,190.4 / 434.1 / 533.0, 75d4c2b 2,280.4 / 457.1 / 561.2, head 2,192.2 / 434.7 / 533.7. The emitted chunk set (715 chunks, names without hashes) is the same as base's, so this adds no second-wave request.Tests. One new contract test:
TOOL_DEFINITIONS.ask_user_question.schemais the light module'sAskUserQuestionToolArgsSchema(the renderer selector and the backend must validate with the same schema). It fails when the tool uses a copy (.describe("copy")). The existing malformed-args tests (askUserQuestionPlacement.test.ts,StreamingMessageAggregator.test.ts) exercise the new parse path and pass unchanged. A separate test audit found no low-value tests and no test-only exports. Guard to keeptoolDefinitions.tsout of main:firstLoadJs.ts --forbid toolDefinitions.tscatches it (it flags 75d4c2b), but no CI job runsfirstLoadJs.tswith a forbid list. Adding one would be a new mechanism, so I left it out. The perf owner will file a follow-up.make static-check,make lint-eslint,make check-react-compiler(24/25, unchanged),make crap-check,make dry-checkand the ask_user_question / aggregator / tool test files are green.Dogfood.
make dev-server-sandboxwith a seeded workspace holding an open non-blocking ask_user_question. The dock shows the form, and the transcript row shows the stub ("Answer in the panel above the chat input.").Follow-ups
toolDefinitions.tsout ofmain-*.js(for examplefirstLoadJs.ts --forbidin CI).openAskUserQuestions.ts(611 B) stays inmain-*.js, because the aggregator's truncation rule runs it.Risks
Low. The change moves schema definitions without edits, and re-exports keep every import path working. If a future edit redefines the schema in
toolDefinitions.ts, the contract test fails.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$2.10