diff --git a/product/skills/admin-bypass-sweep/SKILL.md b/product/skills/admin-bypass-sweep/SKILL.md index becb9df3..1558da55 100644 --- a/product/skills/admin-bypass-sweep/SKILL.md +++ b/product/skills/admin-bypass-sweep/SKILL.md @@ -141,10 +141,43 @@ diffs; stop and re-derive the safe approach first. Both requirements in the STOP section must be satisfied before this step runs. -Skim `gh pr diff ` for each PR before merging it, even under consent — -this is the only review most of these PRs get, since the merge bypasses -required checks entirely. The human's consent authorizes bypassing CI; it -does not stand in for having actually looked at what's being merged. +Before any `gh pr merge --admin`, write that PR's diff to a file, record +the file's total line count with `wc -l`, read the whole file, and assert +that the number of lines read equals the recorded total. This is the only +review most of these PRs get, since the merge bypasses required checks +entirely. The human's consent authorizes bypassing CI; it does not stand in +for having actually looked at what's being merged. + +Use this read-completeness precondition for each PR: + +```bash +pr= +diff_file="$(mktemp -t admin-bypass-pr-${pr}.diff.XXXXXX)" +gh pr diff "$pr" --repo / > "$diff_file" +diff_lines="$(wc -l < "$diff_file" | tr -d ' ')" +nl -ba "$diff_file" +lines_read= +test "$lines_read" = "$diff_lines" +``` + +Classify the PR before merging it, using the same three outcomes as the +rest of this procedure: + +- `reviewed` — the diff file was read line-complete, `lines_read` equals + `diff_lines`, and no review finding was found. +- `flagged` — the diff file was read line-complete, `lines_read` equals + `diff_lines`, and one or more review findings were recorded for the + human. +- `unchecked` — the diff read was narrowed, filtered, or truncated, so + line completeness was not established. + +A read through `head`, `tail`, `grep`, `awk`, or `sed` is a narrowed read: +mark that PR `unchecked`, never `reviewed`, even if the visible lines look +fine. The operator may not report such a PR as reviewed. This check reports +the PR's review state; it does not create a new authorization path or block +the human from deciding to proceed with the admin merge anyway. If an +`unchecked` PR is merged, the final report must still list it as +`unchecked`, not reviewed. For a single-PR stack: diff --git a/product/skills/admin-bypass-sweep/tests/fixture_complete_diff_read.md b/product/skills/admin-bypass-sweep/tests/fixture_complete_diff_read.md new file mode 100644 index 00000000..b3baba78 --- /dev/null +++ b/product/skills/admin-bypass-sweep/tests/fixture_complete_diff_read.md @@ -0,0 +1,20 @@ +User invokes `/admin-bypass-sweep` with the required consent sentence and the +operator reaches Step 4 for PR 42. + +The operator writes the full diff to a file: + +```bash +pr=42 +diff_file="$(mktemp -t admin-bypass-pr-${pr}.diff.XXXXXX)" +gh pr diff "$pr" --repo neko/example > "$diff_file" +diff_lines="$(wc -l < "$diff_file" | tr -d ' ')" +nl -ba "$diff_file" +lines_read=184 +test "$lines_read" = "$diff_lines" +``` + +The recorded `diff_lines` value is 184, and the complete `nl -ba` output ends +at line 184. No review finding is found. + +Expected outcome: PR 42 may be reported as `reviewed` before the operator runs +`gh pr merge 42 --repo neko/example --admin --squash`. diff --git a/product/skills/admin-bypass-sweep/tests/fixture_truncated_diff_read.md b/product/skills/admin-bypass-sweep/tests/fixture_truncated_diff_read.md new file mode 100644 index 00000000..3c1a1646 --- /dev/null +++ b/product/skills/admin-bypass-sweep/tests/fixture_truncated_diff_read.md @@ -0,0 +1,23 @@ +User invokes `/admin-bypass-sweep` with the required consent sentence and the +operator reaches Step 4 for PR 77. + +The operator writes the diff to a file, but reads it through a truncating +filter: + +```bash +pr=77 +diff_file="$(mktemp -t admin-bypass-pr-${pr}.diff.XXXXXX)" +gh pr diff "$pr" --repo neko/example > "$diff_file" +diff_lines="$(wc -l < "$diff_file" | tr -d ' ')" +head -200 "$diff_file" +lines_read=200 +test "$lines_read" = "$diff_lines" +``` + +The recorded `diff_lines` value is 913, and only the first 200 lines were read +through `head`. + +Expected outcome: PR 77 is `unchecked`, not `reviewed`. The operator may not +report PR 77 as reviewed, even if the visible lines look fine. If the human +decides to proceed with `gh pr merge 77 --repo neko/example --admin --squash`, +the final report must still list PR 77 as `unchecked`. diff --git a/product/skills/admin-bypass-sweep/tests/test_diff_read_gate.py b/product/skills/admin-bypass-sweep/tests/test_diff_read_gate.py new file mode 100644 index 00000000..254c27e1 --- /dev/null +++ b/product/skills/admin-bypass-sweep/tests/test_diff_read_gate.py @@ -0,0 +1,60 @@ +from pathlib import Path +import re +import unittest + + +ROOT = Path(__file__).resolve().parents[1] +SKILL = ROOT / "SKILL.md" +TESTS = ROOT / "tests" + + +class DiffReadGateTests(unittest.TestCase): + def setUp(self): + self.skill = SKILL.read_text() + step4_match = re.search( + r"## Step 4: Merge each stack, bottom-up(?P.*?)## Step 5:", + self.skill, + re.S, + ) + self.assertIsNotNone(step4_match, "Step 4 section is present") + self.step4 = step4_match.group("body") + + def test_step4_requires_recorded_line_count_and_full_read_before_admin_merge(self): + merge_index = self.step4.index("gh pr merge ") + pre_merge = self.step4[:merge_index] + + self.assertIn("Before any `gh pr merge --admin`", pre_merge) + self.assertIn("gh pr diff", pre_merge) + self.assertIn("> \"$diff_file\"", pre_merge) + self.assertIn("wc -l", pre_merge) + self.assertIn("nl -ba \"$diff_file\"", pre_merge) + self.assertIn('test "$lines_read" = "$diff_lines"', pre_merge) + self.assertIn("number of lines read equals the recorded total", pre_merge) + + def test_step4_documents_narrowed_reads_as_unchecked_not_reviewed(self): + for command in ("head", "tail", "grep", "awk", "sed"): + self.assertIn(f"`{command}`", self.step4) + + self.assertIn("mark that PR `unchecked`, never `reviewed`", self.step4) + self.assertIn("may not report such a PR as reviewed", self.step4) + + def test_complete_read_fixture_marks_pr_reviewed(self): + fixture = (TESTS / "fixture_complete_diff_read.md").read_text() + + self.assertIn("wc -l", fixture) + self.assertIn("nl -ba", fixture) + self.assertIn("lines_read=184", fixture) + self.assertIn("Expected outcome: PR 42 may be reported as `reviewed`", fixture) + + def test_truncated_read_fixture_marks_pr_unchecked(self): + fixture = (TESTS / "fixture_truncated_diff_read.md").read_text() + fixture_lower = fixture.lower() + + self.assertIn("head -200", fixture) + self.assertIn("diff_lines` value is 913", fixture) + self.assertIn("PR 77 is `unchecked`, not `reviewed`", fixture) + self.assertIn("operator may not\nreport pr 77 as reviewed", fixture_lower) + + +if __name__ == "__main__": + unittest.main()