[kyle-sexton]

No related issue: follow-up fixture for #884 (review-input hardening from claude-code-plugins#9875 review, F5)

Summary

main's own .gitattributes marks package-lock.json, packages.lock.json and *.lock as -diff. Before #884, a change to a lock file's resolved URL reached the reviewer only as Binary files differ. #884 fixed that with git diff --text, but its test covered only attributes that the head adds. This adds the base-attribute case.

Fix

The full-diff test now does three things in both review jobs:

• gives the base main's lock-file -diff attributes and a package-lock.json;
• changes that file's resolved URL at the head;
• requires both the removed and the added "resolved" line in the written diff.

Test only, so no release is needed.

Verification

• node --test .github/scripts/pr-review-cli.test.cjs passes: 30 pass, 0 fail.
• Against the v0.65.8 workflow (4f79cc92) the test fails: pass 0, fail 1.
• biome ci .github is clean.

Related

• fix(pr-review-cli): show every changed file's text in the full diff #884, released as v0.65.9.

🤖 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-10-25T01:12:32.932667Z	3eff8c9	PR opened

ℹ️ 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.

[claude]

Claude finished @kyle-sexton's task in 32s —— View job

---

Security review complete

• Gather context (PR diff, description)
• Apply skip gate
• Review changed file for security-relevant surface
• Post findings / summary

Skip gate: no security-relevant surface. The only change is in .github/scripts/pr-review-cli.test.cjs (diff): it extends an existing test that spins up a throwaway git repo under a temp directory and asserts the CLI's full-diff output includes both the removed and added "resolved" lines when a -diff-attributed lock file's URL changes at the base. No production code, no new input-handling path, no secrets/credentials/tokens, and no injection surface (command, SQL, path, template) is introduced — the new fixture data (lockAttributes, synthetic npm registry URLs) is static, hard-coded test data consumed only by git inside an ephemeral sandbox directory. No plugin manifest or trusted-path file is touched.

No security issues found.

critical-findings: 0

claude-lane-summary: lane=claude-security-review sha=3eff8c9cf2d8fe417a93aa0861990058c6893099 run=40149652716

[claude]

Claude finished @kyle-sexton's task in 2m 24s —— View job

---

Code review complete

• Gather context (diff, AGENTS.md, REVIEW.md, .claude/rules/)
• Review the test change for correctness
• Attempt to run the test suite to verify the author's claimed verification
• Check PR body contract / closing-issue criteria
• Post findings

Scope: 1 file changed, test-only (.github/scripts/pr-review-cli.test.cjs), no production code touched.

Findings

None. The added fixture in the -diff/binary full-diff test (lines 1109–1201) is constructed correctly:

• lockAttributes is written into .gitattributes at both the base and head commits, which accurately models the real scenario described in the PR body — main's .gitattributes marks lock files -diff before any given PR's head commit, rather than that attribute being newly introduced by the head (as hidden.sh/opaque.sh are in the pre-existing part of this test). That's a meaningful, non-redundant addition to coverage.
• lock()'s literal indentation (6 spaces before "resolved") matches the new regexes' {6}-space assertions, and the regex escaping (\., \/) is correct for the literal URLs used — I traced both by hand against unified-diff output, which prefixes -/+ directly onto the original line's indentation.
• Both the removed (registry.npmjs.org) and added (lockfile.example) resolved lines are asserted, for both lanes (key and subscription) in the existing loop.

PR body contract: compliant — opens with No related issue: <reason> and includes ## Summary, ## Fix, ## Verification, ## Related (pr-body-contract.md is advisory guidance, not a path-scoped rule matching this file). No closing issue is declared, so no acceptance-criteria check applies.

Verification note: I was not able to execute node --test .github/scripts/pr-review-cli.test.cjs myself — the sandbox in this lane declined to run it. The PR's "30 pass, 0 fail" and "fails against v0.65.8" results are therefore author-claimed and unverified by me, not independently confirmed.

claude-lane-summary: lane=claude-review sha=3eff8c9cf2d8fe417a93aa0861990058c6893099 run=40149652716

[github-actions]

claude-security-review has reviewed this pull request through 3eff8c9; a later push is reviewed from there.

[github-actions]

claude-review has reviewed this pull request through 3eff8c9; a later push is reviewed from there.