admin-bypass-sweep (3) a truncated diff read is unchecked, not reviewed - #498
Open
EdbertChan wants to merge 4 commits into
Open
Conversation
… a full diff read per PR before an admin merge, and mark a narrowed read unchecked. Review claim: A PR whose diff was read through a truncating filter is unchecked, never reviewed. Review lane: behavior Safety invariant: A truncated or filtered diff read marks that PR unchecked, never reviewed; no PR that is reviewable today becomes unmergeable, because the gate reports rather than blocks the merge command. Effectiveness measurement: A fixture review command piped through `head` marks its PR unchecked, and the same command without the pipe marks it reviewed. Both asserted in the skill's test directory. Slice rationale: One step of the sweep skill's review procedure; stacked behind the tally step because both edit the same SKILL.md. Architectural effect: Step 4 gains an explicit read-completeness precondition expressed as the same three-outcome contract the rest of the stack uses. Goal: Change Step 4 of product/skills/admin-bypass-sweep/SKILL.md so each PR's diff is written to a file, fully read, and asserted line-complete before `gh pr merge --admin`, with a narrowed read reported as unchecked. Motivation: A sweep reported 'Reviewed all 19' after two commands that between them truncated one PR entirely and filtered every test file out of five others. The largest diff in the sweep, touching six hook detectors, was cleared for merge on about 21% of its non-test changes. Alternative considerations: Telling the operator not to pipe through head was rejected as unenforceable prose. Blocking the merge outright on a narrowed read was rejected for this slice: the skill's own consent model puts the merge decision with the human, so the gate reports unchecked and the operator decides. Implementation details: In Step 4, replace the ad hoc `gh pr diff` skim with: write each PR's diff to a file, record `wc -l`, read the whole file, and assert lines_read equals the recorded total before merging that PR. Any read through head, tail, grep, awk, or sed marks that PR unchecked. Use the same three outcomes as the rest of the stack: reviewed, flagged, unchecked. Add fixtures under the skill's tests directory for a full read and a truncated read. Non-goals: Does not change what counts as a review finding, does not add an automated code review, does not alter the consent requirements in the STOP section, and does not touch any other skill. Layer: domain Feature state: active Files: product/skills/admin-bypass-sweep/SKILL.md, product/skills/admin-bypass-sweep/tests/ Change types: - product/skills/admin-bypass-sweep/SKILL.md: docs-only - product/skills/admin-bypass-sweep/tests/fires_truncated_diff_read.md: create - product/skills/admin-bypass-sweep/tests/stays_silent_full_diff_read.md: create Acceptance criteria: - `python3 -m unittest discover -s product/skills/admin-bypass-sweep/tests -v` exits 0. - Step 4 requires a recorded line count and a full read before `gh pr merge --admin`. - A read through head, tail, grep, awk, or sed is documented as unchecked. Solution: Require a full diff read per PR before an admin merge, and mark a narrowed read unchecked. Review claim: A PR whose diff was read through a truncating filter is unchecked, never reviewed. Review lane: behavior Safety invariant: A truncated or filtered diff read marks that PR unchecked, never reviewed; no PR that is reviewable today becomes unmergeable, because the gate reports rather than blocks the merge command. Effectiveness measurement: A fixture review command piped through `head` marks its PR unchecked, and the same command without the pipe marks it reviewed. Both asserted in the skill's test directory. Slice rationale: One step of the sweep skill's review procedure; stacked behind the tally step because both edit the same SKILL.md. Architectural effect: Step 4 gains an explicit read-completeness precondition expressed as the same three-outcome contract the rest of the stack uses. Goal: Change Step 4 of product/skills/admin-bypass-sweep/SKILL.md so each PR's diff is written to a file, fully read, and asserted line-complete before `gh pr merge --admin`, with a narrowed read reported as unchecked. Motivation: A sweep reported 'Reviewed all 19' after two commands that between them truncated one PR entirely and filtered every test file out of five others. The largest diff in the sweep, touching six hook detectors, was cleared for merge on about 21% of its non-test changes. Alternative considerations: Telling the operator not to pipe through head was rejected as unenforceable prose. Blocking the merge outright on a narrowed read was rejected for this slice: the skill's own consent model puts the merge decision with the human, so the gate reports unchecked and the operator decides. Implementation details: In Step 4, replace the ad hoc `gh pr diff` skim with: write each PR's diff to a file, record `wc -l`, read the whole file, and assert lines_read equals the recorded total before merging that PR. Any read through head, tail, grep, awk, or sed marks that PR unchecked. Use the same three outcomes as the rest of the stack: reviewed, flagged, unchecked. Add fixtures under the skill's tests directory for a full read and a truncated read. Non-goals: Does not change what counts as a review finding, does not add an automated code review, does not alter the consent requirements in the STOP section, and does not touch any other skill. Layer: domain Feature state: active Files: product/skills/admin-bypass-sweep/SKILL.md, product/skills/admin-bypass-sweep/tests/ Change types: - product/skills/admin-bypass-sweep/SKILL.md: docs-only - product/skills/admin-bypass-sweep/tests/fires_truncated_diff_read.md: create - product/skills/admin-bypass-sweep/tests/stays_silent_full_diff_read.md: create Acceptance criteria: - `python3 -m unittest discover -s product/skills/admin-bypass-sweep/tests -v` exits 0. - Step 4 requires a recorded line count and a full read before `gh pr merge --admin`. - A read through head, tail, grep, awk, or sed is documented as unchecked. Invoker-Finalize-Id: 86232251-f80f-4a4b-a379-36417182163d
…terministic proof for the full-diff-read gate. Review claim: The skill's fixtures assert a truncated diff read is reported unchecked. Review lane: proof Safety invariant: Proof-only; adds no product behavior. Effectiveness measurement: The suite fails if the truncated-read fixture is reported as reviewed. Slice rationale: One proof slice for this step. Architectural effect: None; verification only. Goal: Prove the full-diff-read gate deterministically. Motivation: The defect is an over-claimed review, so the proof asserts the narrowed case is named unchecked. Alternative considerations: Manual verification was rejected as non-deterministic. Implementation details: Execute the command below as the terminal proof. Non-goals: No product edits here; proof only. Layer: app_regression Feature state: active Exit code: 0 Invoker-Finalize-Id: 4cde49f2-d3fe-41d7-ad5f-4c24fb36f491
… that no ephemeral inter-task handoff files remain. Review claim: The workflow leaves no scratch handoff artifacts behind. Review lane: cleanup Safety invariant: Read-only; never deletes files, alters the index, or commits caller work. Effectiveness measurement: The gate fails when a plans/invoker-handoff.* or lens-*.json file is still present after the leaf tasks complete. Slice rationale: One terminal hygiene gate for the workflow. Architectural effect: None; check only. Goal: Confirm no ephemeral handoff files survive the run. Motivation: Inter-task scratch files leak into diffs and read as part of the work. Alternative considerations: Deleting them automatically was rejected; the gate reports, it does not mutate. Implementation details: Run scripts/scrub-handoff-artifacts.sh without --apply. Non-goals: No deletion, no index changes, no commits. Layer: app_regression Feature state: active Exit code: 0 Invoker-Finalize-Id: 401825e5-c8a7-488b-9472-5b8212a9ca68
…aff8b535d-05d30b31 — Terminal check that no ephemeral inter-task handoff files remain. Review claim: The workflow leaves no scratch handoff artifacts behind. Review lane: cleanup Safety invariant: Read-only; never deletes files, alters the index, or commits caller work. Effectiveness measurement: The gate fails when a plans/invoker-handoff.* or lens-*.json file is still present after the leaf tasks complete. Slice rationale: One terminal hygiene gate for the workflow. Architectural effect: None; check only. Goal: Confirm no ephemeral handoff files survive the run. Motivation: Inter-task scratch files leak into diffs and read as part of the work. Alternative considerations: Deleting them automatically was rejected; the gate reports, it does not mutate. Implementation details: Run scripts/scrub-handoff-artifacts.sh without --apply. Non-goals: No deletion, no index changes, no commits. Layer: app_regression Feature state: active
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_7f851215-dd01-455a-9706-eb1c7abd8154) |
Contributor
|
Tick the box to add this pull request to the merge queue (same as
|
EdbertChan
added a commit
that referenced
this pull request
Sep 13, 2026
…ody checker on PR #498's live body after the rewrite. Goal: Prove the body passes as published. Motivation: A PATCH that returned 200 is not proof the live body passes. Safety invariant: Read-only; a failed fetch fails the task instead of passing. Verify: exit code 0 and output PR body validation passed. Effectiveness measurement: The same checker the required GitHub check runs passes on the live body, so the PR is no longer blocked on its text. Exit code: 0
EdbertChan
added a commit
that referenced
this pull request
Sep 13, 2026
…ody checker on PR #498's live body after the rewrite. Goal: Prove the body passes as published. Motivation: A PATCH that returned 200 is not proof the live body passes. Safety invariant: Read-only; a failed fetch fails the task instead of passing. Verify: exit code 0 and output PR body validation passed. Effectiveness measurement: The same checker the required GitHub check runs passes on the live body, so the PR is no longer blocked on its text. Exit code: 0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This manual merge guide helps an operator check that each change was fully read before merging it.
The old guide allowed a quick look that could miss the end of a change.
The cause was no check that the read covered every diff line.
The fix saves each diff, counts its lines, checks the full read, and records incomplete reads clearly.
Review Claim
The guide now distinguishes a complete review from one that did not cover the whole change, without changing the operator’s merge choice.
Review Lane
behavior
Review Unit
product-skill
Safety Invariant
The guide reports review state without blocking the human’s existing decision to proceed with an admin merge.
Slice Rationale
This isolates the read-completeness contract and its focused fixtures from other sweep changes.
Non-goals
Test Plan
Test Plan
python3 -m unittest discover -s product/skills/admin-bypass-sweep/tests -v— 4 tests passed (OK).bash scripts/scrub-handoff-artifacts.sh—scrub-handoff-artifacts-ok.Revert Plan
Revert Plan
git revert <sha>.