Skip to content

ci(review): review the legacy FAILs file in full again - #12632

Merged
kyle-sexton merged 1 commit into
mainfrom
chore/drop-legacy-fails-review-exemption
Nov 3, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
chore/drop-legacy-fails-review-exemption

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: closes the review exemption #12601 opened for #12558's one-time add

Hold: merge only after #12558 is on main. Until then this carries do-not-merge; merging it first would leave #12558's review partial again.

Summary

Removes verify-legacy-fails.json from scripts/review-generated-data.txt, so both review lanes read every later change to it in full.

Fix

Verification

  • The list file keeps its header and no entries, which declares nothing (ci-workflows#971: an empty list exempts nothing).

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01VcupomBPg7kuobwLoNpxEW

verify-legacy-fails.json was listed as generated data only so the commit
that adds it could pass review. Its loader checks structure, not content,
so every later edit to it is reviewed in full; a whole-file rewrite cannot
be read whole and fails the lanes as partial.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01VcupomBPg7kuobwLoNpxEW
@kyle-sexton kyle-sexton added the do-not-merge Hard merge gate: do not merge while applied. label Nov 3, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

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-03T03:12:10.690183Z 1ef80fb 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.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

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

Code review: no findings

Scope: I read the full diff. It changes one file, scripts/review-generated-data.txt, and deletes its only entry, plugins/source-control/skills/babysit-prs/verify-legacy-fails.json.

Checks:

  • The file now holds only its header comment. The header still describes how the list is meant to work: a path stays listed only while the commit that adds the file lands, and later edits get a full review. Removing the entry follows that rule.
  • Two workflows use the list, both as the generated-data input: .github/workflows/pr-review-hosted.yml:247 and .github/workflows/pr-review-security-hosted.yml:247. The header says the list is read at the merge base. So the removal takes effect only once this PR is on main, and it does not affect how this PR itself is reviewed.
  • No script or test in scripts/ reads the list, so nothing breaks when it has no entries. The PR says an empty list exempts nothing, citing ci-workflows#971. That claim comes from the author and I did not verify it.

Merge order: The PR description says to merge only after #12558 is on main, and that a do-not-merge label enforces this. That is a sequencing decision for a person. The diff itself is correct either way.

Prior findings: none. Round 0 of 3, so the round cap does not apply.

Spend: none.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

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

Security review: PR #12632

No security issues found.

What changed: the PR deletes one line from scripts/review-generated-data.txt: plugins/source-control/skills/babysit-prs/scripts/verify-legacy-fails.json. That file lists generated data the review lanes count as read without reading them (the ci-workflows generated-data input).

Effect on review coverage: this makes review stricter, not looser. After the change, any later edit to verify-legacy-fails.json gets read in full. A whole-file rewrite would fail the lanes as partial, so it could not pass unread. The file now has only its six-line comment header and no entries, so it exempts nothing (head file).

Trust controls:

  • scripts/review-generated-data.txt matches none of the listed trusted-path globs.
  • The change adds no trusted actor and grows no token scope.
  • No new untrusted text reaches a credentialed lane, and no merge hold is weakened.
  • No plugin manifest is touched.

Ordering note (not a finding): the PR description says this should merge only after #12558 is on main and that it carries do-not-merge until then. Merging it first would make #12558's review partial, but that is a coverage effect and not a security risk. The ordering is up to the owner and their label.

No embedded instructions were found in the PR context.

critical-findings: 0

@kyle-sexton kyle-sexton removed the do-not-merge Hard merge gate: do not merge while applied. label Nov 3, 2026
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit 2b9cbd9 Nov 3, 2026
84 of 87 checks passed
@kyle-sexton
kyle-sexton deleted the chore/drop-legacy-fails-review-exemption branch November 3, 2026 04:08
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Post-merge mutation testing

Killed 6, survived 1, timeout 0, no coverage 0, invalid 0, not run 0.
Covered-code score 86%; mutation score 86%.

File Killed Survived Timeout No coverage Invalid Not run
.github/scripts/maintenance-code-tidying-upkeep/upkeep.mjs 2 1 0 0 0 0
plugins/code-tidying/skills/tidy/scripts/open-pr-count.sh 1 0 0 0 0 0
plugins/code-tidying/skills/tidy/scripts/upkeep-setting.sh 3 0 0 0 0 0

Surviving mutants

Location Operator Disposition Cause
.github/scripts/maintenance-code-tidying-upkeep/upkeep.mjs:109 ROR productive input-gap

The run's summary has the full report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant