Start mutation-checking the scripts/ folder - #1944
Merged
Merged
Conversation
The parser hands back a tree, so no node is ever reached twice and the WeakSet could never skip anything. Record the parser's filename argument as a known-equivalent mutant: it only labels diagnostics.
Lock the Biome package version to the one biome.json is written against, name the copy catalog folder once and check it really holds the site's English copy, and assert the message on the combined-failure error. Split the copy-check tests so each one sits beside the file it tests.
Contributor
📝 WalkthroughWalkthroughThe changes extract the PR queue CLI into focused modules, centralize process and copy-check contracts, expand compact reporter and mutation-summary coverage, simplify AST traversal, add equivalent-mutant records, and test several script utility edge cases. ChangesPR queue reporting
Compact TAP reporting
Validation and process utilities
Mutation and script consistency
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant runPrQueue
participant GhIntegration
participant GitHub
User->>runPrQueue: provide CLI arguments
runPrQueue->>GhIntegration: resolve repository and fetch queue
GhIntegration->>GitHub: execute GraphQL request
GitHub-->>GhIntegration: return pull request data
GhIntegration-->>runPrQueue: return queue and pagination state
runPrQueue-->>User: print report
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![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: b4aa4e40b3
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd3c0d62ad
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/scripts/check-copy/run.test.ts`:
- Around line 83-88: Strengthen the real copy catalog test in “the real copy
catalog” by asserting a known complete {file, key, value} entry from the
expected English catalog, rather than relying only on catalog.length and the
presence of common.json. Keep the existing broad checks, and use an established
stable catalog entry that verifies its file, key, and translated value together.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: da08ac26-0a2c-4bdd-a514-1ff85d845520
📒 Files selected for processing (15)
scripts/bench/bundle-composition/javascript-ast.tsscripts/check-copy.tsscripts/check-copy/run.tsscripts/compact-test-reporter.tsscripts/mutation/equivalent-mutants.txttest/scripts/biome-command.test.tstest/scripts/check-copy/rules.test.tstest/scripts/check-copy/run.test.tstest/scripts/cleanup.test.tstest/scripts/compact-test-reporter/diagnostics.test.tstest/scripts/compact-test-reporter/estimate.test.tstest/scripts/compact-test-reporter/progress.test.tstest/scripts/compact-test-reporter/reporter.test.tstest/scripts/compact-test-reporter/summary.test.tstest/scripts/mutation/summary.test.ts
…y tests Adds the missing checks for the shared start-up files, worker counts, the project root, stream decoding, and the file lock's exclusivity. The mutation summary's tests now sit in three files, one per report.
stefan-burke
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83c1291b07
ℹ️ 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".
A plain 20ms wait could pass on a busy runner before the second holder had even asked for the lock. It now waits for the attempt itself, then gives it time to get in, so the check only passes because the lock kept it out.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/scripts/lock-file.test.ts`:
- Line 108: Replace the fixed setTimeout sleep in the lock contention test with
deterministic synchronization tied to the second holder’s actual
lock-acquisition attempt. Add or use a test seam/hook to signal that attempt,
assert the second holder remains blocked, and only then resolve releaseFirst;
avoid real-time sleeps.
In `@test/scripts/mutation/summary/markdown.test.ts`:
- Around line 119-122: Reuse the shared ANSI normalizer by importing plain from
./fixtures.ts in test/scripts/mutation/summary/markdown.test.ts and replacing
the inline regex callback with logs.map(plain); make the same callback
replacement in test/scripts/mutation/summary/terminal.test.ts, ensuring the
terminal test also imports plain as needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f31fb747-d282-40e8-a44a-52f937b85ef0
📒 Files selected for processing (13)
TODO.mdscripts/mutation/equivalent-mutants.txttest/scripts/check-copy/run.test.tstest/scripts/lock-file.test.tstest/scripts/mutation/summary.test.tstest/scripts/mutation/summary/fixtures.tstest/scripts/mutation/summary/markdown.test.tstest/scripts/mutation/summary/score.test.tstest/scripts/mutation/summary/terminal.test.tstest/scripts/project-root.test.tstest/scripts/specs/parallel.test.tstest/scripts/stream-lines.test.tstest/scripts/test-subjects.test.ts
💤 Files with no reviewable changes (1)
- test/scripts/mutation/summary.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7d0733f01
ℹ️ 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".
…odules The entry script now only wires things up: reading the arguments and every gh call live in scripts/pr-queue/, hand back a message and exit code rather than ending the process, and take the command to run as a parameter. Adds the shared captureOutput helper so the gh runner and the git runner decode output one way, and collapses their two identical result types into one. Also fixes the file-lock test: it now waits for the second holder to really ask for the lock before checking it is still blocked (the earlier edit never landed).
They now check that a child shares all three streams, that removing a folder takes its contents with it, and that a force-stopped child is waited for before the cleanup runs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: feb5ddaa2e
ℹ️ 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".
The entry script is now one line. Following Codex's notes: the gh helpers are module-private again and tested through the two calls the report actually makes, the argument reader is named for what it does rather than for the functional pattern, and the comment restating the runner type is gone.
The stand-in gh now steps through its answers instead of indexing past the end, so it has no unreachable fallback, and the re-poll waits with the shared delay helper rather than its own copy.
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Jul 26, 2026
…-scripts # Conflicts: # TODO.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/pr-queue/gh.ts`:
- Around line 173-179: Update the repository access in the fetchQueue flatMap
logic to remove optional chaining from data.repository, preserving the existing
!fresh guard for missing pull-request aliases and allowing malformed payloads
without repository to fail loudly.
In `@test/scripts/precommit.test.ts`:
- Around line 19-36: Centralize the duplicated CapturedOutput fixture
construction in a shared helper under `#test-utils`, preserving support for
successful and failed outputs. In test/scripts/precommit.test.ts lines 19-36 and
test/scripts/precommit/mutation-step.test.ts lines 13-36, replace the local
ok/fail implementations with the shared factory while retaining domain-specific
wrappers such as sha. In test/scripts/specs/evidence-git.test.ts lines 7-19,
replace the local result object construction with the same shared factory.
In `@test/scripts/process.test.ts`:
- Around line 107-130: Update the force-stop test around stopProcess and
fakeChild so it synchronizes on the SIGKILL request instead of using a fixed 20
ms delay. Resolve a promise from the SIGKILL branch, await that promise before
invoking endKilled, and preserve the expected child-ended then cleanup ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 57faeebd-4e70-4ecd-8df0-cabbad3e71f9
📒 Files selected for processing (21)
TODO.mdscripts/mutation/equivalent-mutants.txtscripts/pr-queue.tsscripts/pr-queue/args.tsscripts/pr-queue/gh.tsscripts/pr-queue/run.tsscripts/precommit/git.tsscripts/process.tstest/scripts/cleanup.test.tstest/scripts/lock-file.test.tstest/scripts/mutation/summary/fixtures.tstest/scripts/mutation/summary/markdown.test.tstest/scripts/mutation/summary/terminal.test.tstest/scripts/pr-queue/args.test.tstest/scripts/pr-queue/fixtures.tstest/scripts/pr-queue/gh.test.tstest/scripts/pr-queue/run.test.tstest/scripts/precommit.test.tstest/scripts/precommit/mutation-step.test.tstest/scripts/process.test.tstest/scripts/specs/evidence-git.test.ts
💤 Files with no reviewable changes (1)
- test/scripts/mutation/summary/fixtures.ts
Reads the re-poll reply straight rather than optionally, so a malformed one fails loudly like the rest of the fetch. Shares one command-output fixture between the three test files that were each building their own. Waits for the force-stop to be asked for instead of guessing at twenty milliseconds.
A module-level alias is what the code-quality rule forbids, and it hid the helper behind a second name for no gain.
This was referenced Jul 27, 2026
This was referenced Aug 16, 2026
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
2 participants
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.
Mutation testing has only ever been pointed at
src/. This branch points it atscripts/for the first time, and closes the gaps it found in sixteen of those files.What a gap means here: a mutation run makes one small change to the code — swaps a
+for a-, empties a piece of text, deletes a line — and runs that file's tests. If they still pass, nothing was really checking that piece of code. Every gap closed below was a change that used to slip through unnoticed.The survey
65 files under
scripts/have a test beside them. I ran all of them:Fixed in this branch
compact-test-reporter.tsmutation/summary.tspr-queue(four files)ghcall, and both ways of printing the reportprocess.tscleanup.tsbench/…/javascript-ast.tslock-file.tscheck-copy.ts/check-copy/run.tscheck-copy/rules.tsbiome-command.tsbiome.jsonis written fortest-subjects.tsspecs/parallel.tsproject-root.tsdeno.jsonstream-lines.tsAll sixteen now score 100%.
Two files had to be restructured, not just tested
pr-queue.tsandcheck-copy.tswere untestable by construction: they did their work as the file loaded and ended the process with an exit code, so no test could ever call them.pr-queue.tsis now one line. Reading the arguments, talking togh, and printing the report each live in their own file underscripts/pr-queue/, and hand back an answer instead of ending the process. The command to run is a parameter, so the whole report can be exercised without a network.check-copy.tsnow names the folder holding the site's English copy in a place a test can reach.Along the way the
ghrunner and the git runner turned out to be doing the same job, so they now share onecaptureOutputhelper, and their two identical result types became one.Code that came out
Some things gained no test because nothing could ever reach them: a repeat-visit guard on a tree that has no repeats, a colon check on text that always has colons, a "no total yet" branch that a running count always fills in, a duplicate way of spotting
--reporter, and an optional read that contradicted the fail-loudly rule beside it.Eleven changes genuinely cannot be caught by any test — the parser filename that only labels error messages, a fallback for a pattern that always matches — and those are written down in
equivalent-mutants.txtwith the reason.Test files that had grown past the 400-line mark were split: the copy checks now sit beside the file each one tests, and the mutation summary's tests are three files, one per report.
Still to do
24 files still have misses. The big ones are
mutation/isolation.ts(49),stripe-mock.ts(48),stripe-mock/install.ts(39),specs/run.ts(38) andmutation.ts(30). Likepr-queuewas, most are command-line entry points whose work happens in child processes, so they need the same treatment: pull the decisions out where a test can see them.TODO.mdalso records the follow-up to splitcompact-test-reporter.ts, which is over the 400-line target but was already so before this branch.