Repository navigation
ci(mutants): diff against the pull request's real base, read from the API - #1381
Conversation
… API The gate scopes cargo-mutants to the PR's diff, which it took as `git diff origin/$GITHUB_BASE_REF`. For a pull request whose base branch is itself the head of another open pull request, GitHub has been seen to build the test merge commit against that other PR's merge commit and to report the other PR's base in `github.base_ref`: a PR into `next` while `next` has a PR into `main` arrived here with `main` as its base, and the stacked PRs above it with their grandparents' bases. The diff was then everything between `main` and `next`, 670 to 800 files, and the gate hit its two-hour timeout on changes that touched no Rust at all. The step now asks `gh pr view` for the base the PR page shows, fetches that commit and the PR head, and diffs the two with three dots, so the diff holds the PR's own lines whatever GitHub checked out. The log names the base it used beside what the payload said, and lists the files under mutation instead of printing the whole diff. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
Mutation testing (cargo-mutants,
|
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. |
cipherstash-agent
left a comment
There was a problem hiding this comment.
Review attestation
Source: Claude Code /code-review medium, with company-skills:cross-check-review-findings run against head 98edbe7.
Critical/blocking issues: none.
Findings: 1 new minor, non-blocking finding (inline). The diff uses the PR head's line numbers, but mutants run on the checked-out merge commit, so --in-diff can mismatch lines when the base has diverged. Suggested fix: check out the head SHA.
Cross-check: there were no prior review threads or reviews. The bot comments contain no findings. Nothing was dropped as already covered.
Checked and fine:
fetch-depth: 0makes the three-dot merge-base work.pull-requests: writecoversgh pr view.- The
|| echo "(none)"fallback works under the-eo pipefailshell GitHub uses forshell: bash.
Generated by Claude Code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98edbe7215
ℹ️ 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".
…e tree
The previous commit diffed the base against the PR head while
actions/checkout left GitHub's test merge commit in the tree. If the
base had moved and touched earlier lines of a file the PR changes, the
hunks' line numbers no longer matched the tree, and cargo-mutants 27.1.0
refuses such a diff ("Diff content doesn't match source file"), failing
the gate on a valid PR. The checkout now takes the PR head on a pull
request, the diff step asserts that is what it has, and diffs the base
against it. A manual dispatch checks out the default ref as before.
Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Brings in the mutants gate fix from #1381, which reads the pull request's real base from the API and checks out the head, so the gate no longer diffs all of next and times out on this stack. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
…profile Brings in the mutants gate fix from #1381, which reads the pull request's real base from the API and checks out the head, so the gate no longer diffs all of next and times out on this stack. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Brings in the mutants gate fix from #1381, which reads the pull request's real base from the API and checks out the head, so the gate no longer diffs all of next and times out on this stack. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Brings in the mutants gate fix from #1381, which reads the pull request's real base from the API and checks out the head, so the gate no longer diffs all of next and times out on this stack. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Brings in the mutants gate fix from #1381, which reads the pull request's real base from the API and checks out the head, so the gate no longer diffs all of next and times out on this stack. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Brings in the mutants gate fix from #1381, which reads the pull request's real base from the API and checks out the head, so the gate no longer diffs all of next and times out on this stack. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Brings in the mutants gate fix from #1381, which reads the pull request's real base from the API and checks out the head. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Summary
The mutation-testing gate (
Mutantsworkflow) runscargo mutantsover the lines a pull request changes instack-authandstack-encrypt. It found those lines withgit diff origin/<base>where the base came from the workflow'sgithub.base_ref. On the json_tree Go stack that variable was wrong: for a pull request whose base branch is itself the head of another open pull request, GitHub built the test merge commit against the other pull request's merge commit and reported the other pull request's base. PR #1369, which targetsnext, arrived withmainas its base becausenexthas an open pull request intomain; the stacked pull requests above it arrived with their grandparents' bases. The gate then diffed everything betweenmainandnext, 670 to 800 files, and timed out at two hours on changes that touched no Rust.This change makes the gate ask GitHub for the pull request's real base at run time and diff that against the pull request's head. A Go-only pull request yields no mutants again and finishes in minutes; a Rust pull request mutates exactly its own lines.
Changes
.github/workflows/mutants.yml: the "Compute the PR diff" step reads the base branch and commit withgh pr view, fetches that commit and the head, and diffsbase...head. The log prints the base it used beside what the event payload said, and the list of files under mutation instead of the whole diff. The job's token already has the pull-request scope the sticky comment needs.Verification
workflow-paths-filter-parity,crates-ci,workflow-publish-permissions,workflow-mise-cache-image(63 tests).BASE_REF: mainon PR chore(deps): take vitaminc's Go bindings from cipherstash/vitaminc#526 by git reference #1369 (basenext), feat(stack-encrypt,eql): store a JSON document through a data plan as JsonSearch #1372 and feat(stack-encrypt): answer a document field's JSON operators and open its reads from a data plan #1373 (bases in the stack), and the checked-out merge commit on chore(deps): take vitaminc's Go bindings from cipherstash/vitaminc#526 by git reference #1369 is "Merge 4c5ab21 into a7e4494", where a7e4494 is the test merge commit of thenextintomainpull request. The passing gate on PR feat(stack-encrypt): cascade the JSON tree's AEAD tags and store its entries in a random order #1379, also intonext, saw the right base, so the condition is not constant; reading the base from the API is correct either way.Related
https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu