Repository navigation
🤖 tests: delete test-only helpers in node/common and test the production paths - #7601
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. |
|
@codex review |
🛡️ 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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 693e098e0f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
693e098 to
a671559
Compare
a671559 to
b4aaeca
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 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 passed22 agent steps · 7 replayed from cache · 15 model calls · 45.9k tokens (28% cached) All 14 tests in 9 files
e2e 0.20.0 · 1m 28s · web · run artifacts 🟢 e2e Electron stories (Haiku): 3 passed3 agent steps · 3 model calls · 8.0k tokens (32% cached) All 3 tests in 3 files
e2e 0.20.0 · 22.1s · electron · run artifacts 🟢 e2e agent repros (Haiku): no tests selectedSpend: $0.00 of the $1.00 cap (proxy ledger) e2e 0.20.0 · 6.9s · 2 targets (web, phone) · run artifacts |
…ion paths - Delete src/cli/serverSecurity.ts: nothing imports it. The server uses serverAuthToken.ts, which has its own tests. - Delete withReviewStateSection: the review-state service imports legacy state through deltas. The two test doubles call assignSection. - Delete ensureWorkspaceScratchDir: host scratch dirs are made by ensureScratchDirForSpec, which now has the mkdir-failure test. - Delete buildToolCatalogOverview: the overview tests read the tool_catalog_search description that prepareToolSearch builds.
b650ab5 to
1f4aae9
Compare
Summary
Delete four node/common helpers that only tests called, and move their tests to the code production runs. All four are in P3-2's
pendingDeletionlist (#7561).src/cli/serverSecurity.ts(resolveServerAuthToken,isLoopbackHost)xum serverusesserverAuthToken.ts, an older policy with its own tests.serverAuthToken.test.ts.withReviewStateSection(reviewState.ts)ReviewStateService.importLegacyimports through deltas. Only two test doubles called it.ReviewStateStore.test.tsand the story mock) callassignSection.ensureWorkspaceScratchDir(workspaceScratchDir.ts)ensureScratchDirForSpec.runtimeScratchDir.test.ts. The why-comment moved toensureScratchDirForSpec. The path tests stay ongetWorkspaceScratchDir.buildToolCatalogOverview(toolCatalog.ts)prepareToolSearch.tool_catalog_searchdescription thatprepareToolSearchbuilds (advertisedOverviewhelper).Kept (P3-2 moves these to
allowed)computePriorHistoryFingerprint: the reference implementation that the async twin's tests compare against.replayFixture.tsappendReplayFixtureCompactionBoundary,generateReplayFixtureSession: the replay golden-fixture harness.PhoneChrome: the type of the generatedPHONE_CHROME_JSONskeleton. Only tests parse that JSON.updateCodeWorkspaceFile: the unit boundary of the.code-workspaceedit algorithm (23 tests).syncProjectCodeWorkspaceruns the sameupdateCodeWorkspaceFileLockedunder its own lock.Validation
I broke the production code for each moved test, ran it, saw it fail, and restored the code:
ensureScratchDirForSpechost case with a throwing mkdir: the moved failure test fails.buildOverview/groupCatalogByServer: the "other tools" label, the ", and more" suffix, the total budget, the name sort, and the label clamp. Each mutant fails at least one overview test.make typecheck,make lint-eslint,fmt-checkandcheck-test-routingpass. P3-2's checker passes on this stack once the stale entries are removed. Only the four kept items above (five entries) stay.Part of test-audit phase 3, workstream P3-3.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high