[kyle-sexton]

Refs: #12939

Summary

Owner decision Q10 (a), 2026-11-04: commit no binary fixtures for the revert eval cases, so the review lanes read every file (record: melodic-software/architecture#28). This replaces #13225's 16 fixtures.tar.gz archives with plain-text fixture directories. This is part A: the scaffold change and 7 cases. Part B adds the other cases as data only. Each PR stays under the security review's 300,000-byte read budget (ci-workflows pr-review-cli.yml:358-359), so every file is read.

Fix

• revert-cases.sh scaffold builds a case from <case>/fixtures/{base,head}/<path>.txt and fixtures/pr-context.md.txt. It refuses a case with no fixtures directory. build writes that directory, and archive support is gone.
• Fixtures for revert-12114, 12121, 12162, 12186, 12201, 4899 and 4903. They are copied from the archives lane-2 scanned for secrets. Each file matches git show of its commit byte for byte.
• The fixtures are copies of history, so two lints skip plugins/review/evals/revert-*/fixtures/, each next to an existing skip of the same kind:
  • typos (the Spell-check step's exclude);
  • editorconfig (two pr-context files keep trailing whitespace).
• build keeps the exec bit on a fixture that starts with a shebang, so the exec-bit check needs no skip.
• The revert-1835 case is dropped. Its fixtures are 102 files and about 1.3 MB, so no review can read them whole. The set keeps 15 cases, over the script's minimum of 5.
• Markdownlint, lychee, shellcheck and ruff already pass over .txt files.

Verification

• plugins/review/evals/revert-cases.test.sh: 39 pass, 0 fail. New cases: byte equality with git show, build writes no archive, the scaffold has the base on main and the merge on change, and a missing fixtures directory exits 2.
• The diff is 27 files and 221,874 bytes. gitleaks is clean.

Related

• Supersedes test(review): add the revert cases' fixture archives #13225, which lane-2 closes once part B lands.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-04T17:03:24.243344Z	ce7a37c	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

claude-security-review reviewed cdde1bd19453c87f162dac2a45e4bf437d6ae347: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #13299, incremental (b13ba44 → cdde1bd)

No security issues found. This lane reviewed an earlier head, so this pass reported only blocking findings (CRITICAL and IMPORTANT). None came up.

What changed and what I checked (I read every hunk of both files in scope):

• plugins/review/evals/revert-cases.sh: build used to set the exec bit on a fixture copy when the file started with #!. It now sets it when git recorded mode 100755 for the file (git ls-tree --format='%(objectmode)' "$at" -- "$path"). Both arguments are quoted, and -- separates the path from options, so nothing gets interpreted as an option or a shell command. $at is a commit resolved by git rev-parse --verify. $path comes from this repository's own history (pair_lines). This runs only in the maintainer-side build, not in any credentialed lane. The change follows the source file's recorded mode, so it is narrower than the old shebang check.
• plugins/review/evals/revert-cases.test.sh: the test now commits the app.sh fixture as executable and adds two assertions: each copy keeps the exec bit git recorded, and the context file is not executable. Test code only.

No plugin manifest, trusted actor, token scope, kill switch or merge-hold path is touched in this increment. The previous review's summary-only post (no line anchor) reported 0 findings, so there is nothing to carry forward.

critical-findings: 0

[github-actions]

claude-review: 1 inline finding(s) on ce7a37c6d70c7cf2e8b53089b4ede12f251d4b26; the summary is in this lane's pull request comment.

claude-lane: claude-review

[github-actions]

claude-review reviewed cdde1bd19453c87f162dac2a45e4bf437d6ae347: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

claude-review (code-review lane) on cdde1bd19453c87f162dac2a45e4bf437d6ae347. This was an incremental, blocking-only review. The lane had already reviewed earlier heads, so only blocking findings would be posted.

What I read: every hunk of both files in .claude-lane/incremental.diff, which covers the changes since b13ba44d:

• plugins/review/evals/revert-cases.sh
• plugins/review/evals/revert-cases.test.sh

I also read the changed regions of both files at head for context.

Result: no blocking findings.

• build exec bit (revert-cases.sh:209): the copy's mode now comes from git ls-tree --format='%(objectmode)' "$at" -- "$path". Only a 100755 entry gets chmod +x. The #! content check is gone.
  • The command runs after to_top, so the pathspec is relative to the repository root, and a nested path still resolves to its own entry.
  • Symlinks (120000) and plain files (100644) stay non-executable.
  • Copies on the base side and the head side each take the mode recorded at their own commit. A side that is the same in both commits therefore adds no mode change to the scaffold's pr.diff.
• Test (revert-cases.test.sh:44-47, :91-96):
  • app.sh is created executable before its first commit, so every later commit records it as 100755.
  • The new assertions check that both copies are -x and that pr-context.md.txt is not.
  • app.sh has no #! line, so the test also tells this approach apart from the old shebang check.
  • Each mode result is assigned before expect reads it.

Prior findings: this lane's earlier advisory at discussion_r4470700039 said the exec bit was guessed from a #! check and had no test. Both parts are fixed at this head: build reads the mode from the tree, and there is now a test for it. The thread is resolved and its lines have changed, so I am not repeating it.

Settled threads: the one thread, on revert-cases.sh:209, is resolved, and the fix above backs that up.

Not verified by me: the owner says the test fails when the chmod line is disabled. That claim is unconfirmed. I had no shell to run the suite.

Closing issue: the PR says Refs: #12939 and closes no issue, so there are no acceptance criteria to check.

Manifests: none changed in this push.

Spend: none. The push adds no lane runs, models or schedules.