Repository navigation
fix(pstack): reuse a verdict only when the exact whole-PR diff matches - #10
Conversation
A git patch id ignores line numbers, so moving a change between two identical blocks keeps the same id while different code runs. Shipping step 3 now hashes the raw diff from the saved merge base to the verdict head and compares it with the current one. A rebase over unrelated trunk changes keeps the hash; one that shifts the PR's lines gets a fresh check, which the playbooks already give a rebase. The other playbooks now point at the diff rule. Fixes #9 (comment) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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. |
![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: 5f16f2a6a3
ℹ️ 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 .gitattributes textconv driver can map different blobs to the same diff text, so the verdict hash could match a different artifact. The hash now uses --no-textconv --no-ext-diff --binary --full-index --no-renames --no-color. Checked in a scratch repo with a head -c 1 textconv: plain git diff hashed two different changes the same, the flagged diff did not. Fixes #10 (comment) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
Flags on git diff kept missing config: diff.ignoreSubmodules=all and diff.relative=true still made different changes hash the same (found by the GPT verifier). git diff-tree is plumbing and ignores that config, and its full blob ids cover line positions, binaries and submodule commits. A scratch repo with textconv, ignoreSubmodules=all and relative=true set, run from a subdirectory, gave distinct hashes for all four collision cases, and a rebase over unrelated trunk kept the hash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! 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". |
The GPT verifier broke the diff-tree one-liner twice more: a git replace ref made two heads hash the same, and a git failure produced an empty diff that sha256sum hashed with exit 0. scripts/verdict-hash.sh turns off replace refs and grafts, verifies both commits, refuses an empty change, and fails closed under set -euo pipefail. Its bun tests cover the moved-block, textconv plus diff.relative, replace-ref, git-failure and unrelated-rebase cases, and each guard's removal fails a test. Shipping step 3 now calls the script. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4484e2e472
ℹ️ 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".
Fixes #10 (comment) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The GPT verifier showed GIT_OBJECT_DIRECTORY and GIT_ALTERNATE_OBJECT_DIRECTORIES pointing at a corrupt overlay could serve one tree's contents under another's id. The script now unsets both, and the verifier's reproduction gives the pristine, distinct hashes again. A corrupt object store inside the repo itself is out of scope: the rule guards against reusing a stale verdict by accident, and git fsck reports such a store. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
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". |
Why
Codex's review bot found that
git patch-idignores line numbers, so a conflict resolution that moves a change between identical blocks keeps the same id while different code runs. Shipping would then carry an old verdict to untested behavior.What changed
sha256sumof the raw whole-PR diff (saved merge base to verdict head) with the current one.Tradeoffs
Verification
8e8e271ff0before and after).node scripts/check-t3-port.mjsclean.bun test73 of 73.🤖 Generated with Claude Code
Note
Medium Risk
Tightens merge-gating logic for when verification verdicts carry across new heads; incorrect hashing would block merges or (if too loose) allow stale verdicts.
Overview
Fixes verdict reuse so an old swarm result cannot carry when the whole-PR change is not actually the same code, after review found
git patch-idcould match when a fix only moves edits between identical blocks.Shipping step 3 now compares hashes from
verdict-hash.sh(saved merge-base/head vs current) instead of pipinggit diffthroughpatch-id. The script hashesgit diff-treeblob-level output with hardening against replace refs, grafts, and diff config. Autopilot-full, autopilot-stack, and multi-phase-plan rename the rule to the diff rule and point at the same step.Adds
verdict-hash.test.ts(including the patch-id failure case and rebase stability) and wires it intopackage.jsonbun test. Rebases that only shift line positions may need a fresh verifier; playbooks already treat rebases as a single-verifier event.Reviewed by Cursor Bugbot for commit 4128055. Configure here.