diff --git a/.github/workflows/launchpad-pr-check.yml b/.github/workflows/launchpad-pr-check.yml index 2cb1473b5c7..739078e1eab 100644 --- a/.github/workflows/launchpad-pr-check.yml +++ b/.github/workflows/launchpad-pr-check.yml @@ -2,79 +2,76 @@ name: launchpad — PR body check on: pull_request: - types: [opened, edited, reopened, labeled, unlabeled, synchronize] + types: [opened, edited, reopened, labeled, unlabeled, synchronize, ready_for_review] permissions: contents: read + pull-requests: read jobs: check: runs-on: ubuntu-latest steps: + # THIS CHECK IS NOT A SECURITY BOUNDARY, AND MUST NOT BE DESCRIBED AS ONE. + # + # An earlier version of this file pinned the checkout to the base commit and + # claimed that stopped a fork from supplying the checker that judges it. That + # was wrong. `on: pull_request` runs the workflow from the PR's own merge ref + # — for forks too — so a fork PR already owns every step in this file and can + # overwrite or skip the script whichever commit is checked out. Pinning the + # ref bought nothing and made a change to the checker unable to test itself. + # (Running the BASE workflow for forks is `pull_request_target` behaviour, + # which this workflow deliberately does not use.) + # + # The real boundary is elsewhere, and it is enough: a fork-triggered run gets + # a read-only GITHUB_TOKEN with no repository secrets, and merging needs two + # approving reviews from people with write access. A defeated body check + # misleads a reviewer; it cannot merge anything or write anywhere. + - uses: actions/checkout@v4 + with: + persist-credentials: false + + # A missing checker is not a passing body. Cheap insurance against a partial + # checkout, or a branch that predates the script. + - name: Confirm the checker exists + run: | + test -f launchpad/scripts/pr_body_check.py || { + echo "::error::launchpad/scripts/pr_body_check.py is missing from the commit under check." + exit 1 + } + + # GitHub already knows whether this PR will close an issue. Asking it is the + # only reliable answer: a regex over the body cannot tell a real reference + # from one written inside code, and CommonMark has more code forms than a + # pattern enumerates — four of them defeated the previous version (#125). + # + # On failure CLOSING_REFS stays empty, which the script reads as "unknown" + # rather than "none", and it says so in its output instead of guessing. + - name: Ask GitHub which issues this PR closes + id: closing + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + OWNER: ${{ github.repository_owner }} + REPO: ${{ github.event.repository.name }} + NUMBER: ${{ github.event.pull_request.number }} + run: | + set -uo pipefail + refs="$(gh api graphql \ + -f query='query($owner:String!,$repo:String!,$num:Int!){ + repository(owner:$owner,name:$repo){ + pullRequest(number:$num){ + closingIssuesReferences(first:50){nodes{number}}}}}' \ + -F owner="$OWNER" -F repo="$REPO" -F num="$NUMBER" \ + --jq '[.data.repository.pullRequest.closingIssuesReferences.nodes[].number]' \ + 2>/dev/null)" || refs="" + echo "refs=${refs}" >> "$GITHUB_OUTPUT" + if [ -z "$refs" ]; then + echo "::warning::Could not read closingIssuesReferences; the check will degrade to a text search and will not verify that the board updates on merge." + fi + - name: Validate PR body env: BODY: ${{ github.event.pull_request.body }} LABELS: ${{ toJSON(github.event.pull_request.labels.*.name) }} - run: | - python3 - <<'PY' - import json, os, re, sys - - body = os.environ.get("BODY") or "" - labels = json.loads(os.environ.get("LABELS") or "[]") - is_agent = "by:agent" in labels - errors = [] - - # Strip HTML comments so unfilled placeholders never count as content. - visible = re.sub(r"", "", body, flags=re.S) - - def section(name): - """Text under a '### name' heading, up to the next heading.""" - m = re.search(rf"^#+\s*{re.escape(name)}\s*$(.*?)(?=^#+\s|\Z)", - visible, flags=re.M | re.S) - return m.group(1).strip() if m else None - - if not visible.strip(): - errors.append("PR body is empty. Use the PR template.") - - if not re.search(r"\b(Closes|Fixes|Resolves)\s+#\d+", visible, re.I): - errors.append( - "No closing keyword found. Add 'Closes #' so the board updates on merge.") - - itype = section("Issue type") - valid = {"PRD", "Task", "Enhancement", "Bug", "ADR"} - if not itype: - errors.append("Missing '### Issue type' section.") - elif not any(v.lower() in itype.lower() for v in valid): - errors.append( - f"Issue type must be one of {sorted(valid)}. Found: {itype!r}") - - if is_agent: - for field in ("Harness / provider", "Model", "Initiating human"): - # Table rows: | Field | Value | - m = re.search(rf"\|\s*{re.escape(field)}\s*\|(.*?)\|", visible) - if not m or not m.group(1).strip(): - errors.append( - f"by:agent PR missing provenance value for '{field}'.") - - nv = section("Not verified") - if not nv: - errors.append("by:agent PR missing '### Not verified' section.") - elif nv.lower().strip(" .") in {"nothing", "none", "n/a", ""}: - errors.append( - "'Not verified' must name something specific. " - "There is always something that was not checked.") - - if "```" not in visible: - errors.append( - "by:agent PR must paste raw command output in a fenced code block.") - - if errors: - print("PR body check failed:\n") - for e in errors: - print(f" - {e}") - print("\nHuman PRs: .github/PULL_REQUEST_TEMPLATE.md") - print("Agent PRs: launchpad/AGENT_PR_TEMPLATE.md") - sys.exit(1) - - print("PR body check passed." + (" (agent)" if is_agent else " (human)")) - PY + CLOSING_REFS: ${{ steps.closing.outputs.refs }} + run: python3 launchpad/scripts/pr_body_check.py diff --git a/launchpad/AGENTS.md b/launchpad/AGENTS.md index f2183732930..ea52844d271 100644 --- a/launchpad/AGENTS.md +++ b/launchpad/AGENTS.md @@ -212,9 +212,18 @@ gh pr create --base launchpad - **Conventional commit titles**: `feat(deploy): ...`, `fix(ci): ...`, `docs(...): ...`. We squash-merge, so the **PR title** becomes the commit subject on `launchpad`. - **One issue, one PR.** Use a closing keyword — `Closes #12` — so the board updates on - merge. -- **The `launchpad` branch is protected.** PRs require an approving review from another - collaborator. You cannot approve your own. + merge. If the PR genuinely completes nothing — a plan, one step of a larger task, a + docs correction — use `Refs #12` instead. Both satisfy the PR body check; only + `Closes` moves the board, so do not reach for it to make a check go green. + **Write the reference as plain text, not inside backticks or a code block.** GitHub + creates no link from a reference inside code, so one written there closes nothing. +- **The `launchpad` branch is protected.** PRs require **at least two approving reviews + from reviewers with write access**, and you cannot approve your own. The ruleset that + enforces this is not readable without `admin:org` — `rules/branches/launchpad`, + `rulesets` and `branches/launchpad/protection` all report nothing. A live PR's + `reviewDecision` confirms that review is *required* (`REVIEW_REQUIRED`) but exposes no + count; the figure of two comes from GitHub's merge box on an open PR, which is the only + place it is stated without admin. - **Do not force-push during review.** Push new commits instead — force-pushing hides what changed from the reviewer. diff --git a/launchpad/scripts/pr_body_check.py b/launchpad/scripts/pr_body_check.py new file mode 100644 index 00000000000..71675499dc2 --- /dev/null +++ b/launchpad/scripts/pr_body_check.py @@ -0,0 +1,289 @@ +#!/usr/bin/env python3 +"""Validate a pull request body against this fork's conventions. + +WHY THIS IS A SCRIPT AND NOT INLINE YAML + +It used to be a Python heredoc inside `.github/workflows/launchpad-pr-check.yml`. +Two consequences, both real: + + 1. NOTHING COULD TEST IT. Logic embedded in YAML is not importable, so the only + way to check a change was to open a PR and watch. The suite written for + issue #125 had to copy the regexes by hand, which meant it would have passed + unchanged if the real check were reverted or replaced with `sys.exit(0)`. + 2. THE MARKDOWN PARSING WAS WRONG, AND QUIETLY SO. See below. + +WHY GITHUB ANSWERS THE CLOSING QUESTION, NOT A REGEX + +The check's purpose is "will the board update when this merges?". A regex over the +body cannot answer that, because GitHub ignores references written inside code and +CommonMark has more code forms than anyone enumerates on the first attempt. +Measured against the previous fix on 2026-08-12, all four of these smuggled a +keyword past it while GitHub created no link: + + an unterminated ``` fence (the template mandates pasted output, so + a missing closing fence is the single most + likely authoring slip in this repo) + a ``double-backtick`` span (no run-length matching) + a ````quad fence```` with an + inner shorter run (non-greedy match closes early) + a 4-space indented block (not a delimiter the regex knew about) + +So the authoritative answer comes from GitHub's own `closingIssuesReferences`, +passed in by the workflow. There is no markdown to misparse. + +`Refs #n` still needs a text search, because it is a convention of ours that +GitHub knows nothing about. A false positive there is cheap: `Refs` makes no claim +that the board will move, so the worst case is a PR that named its issue in an +unusual place. The fence stripping below is therefore best-effort by design, and +its one hard case — the unterminated fence — is handled. + +ABSENCE IS NOT EVIDENCE + +`CLOSING_REFS` unset means the query did not run or failed. That is NOT the same as +"GitHub found no closing reference", and it must never read as either a pass or a +silent failure: the check degrades to a text search and says so on stdout, so a +reader can tell a verified result from a guessed one. + +Usage: + BODY=... LABELS='["by:agent"]' CLOSING_REFS='[116]' python3 pr_body_check.py + +Exit: 0 = the body is acceptable, 1 = it is not. +""" + +from __future__ import annotations + +import json +import os +import re +import sys + +CLOSING_RE = re.compile(r"\b(Closes|Fixes|Resolves)\s+#\d+", re.I) +REFS_RE = re.compile(r"\bRefs\s+#\d+", re.I) + +ISSUE_TYPES = frozenset({"PRD", "Task", "Enhancement", "Bug", "ADR"}) +PROVENANCE_FIELDS = ("Harness / provider", "Model", "Initiating human") +EMPTY_NOT_VERIFIED = frozenset({"nothing", "none", "n/a", ""}) + + +def strip_comments(body: str) -> str: + """Remove HTML comments so unfilled template placeholders never count.""" + return re.sub(r"", "", body, flags=re.S) + + +BLOCKQUOTE = re.compile(r"^(?: {0,3}>)+ ?") +FENCE_OPEN = re.compile(r"^ {0,3}(`{3,}|~{3,})") +# A closing fence carries nothing but its run and trailing whitespace. An opener +# may carry an info string (```python); a closer may not, so they need different +# patterns — accepting junk after a closing run ends the block early and starts a +# spurious new one, which both leaks code as prose and swallows real prose as code. +FENCE_CLOSE = re.compile(r"^ {0,3}(`{3,}|~{3,})[ \t]*$") + + +def _strip_fences(text: str) -> str: + """Drop fenced blocks, honouring CommonMark's run-length rule. + + A regex cannot do this. A closing fence must be *at least as long* as the one + that opened it, and `re` has no "backreference, or longer" — so a non-greedy + ```` ```.*?``` ```` closes early on any shorter run inside a longer fence, which + is how a ````quad fence```` wrapping a ``` example leaked a keyword into prose. + An unterminated fence runs to the end of the document, so it consumes the rest. + + A fence must also close with the character it opened with: ``` is not closed by + ~~~, however long the run. + + Blockquote markers are stripped before matching, so a fence quoted with `> ` + is recognised. Without that, quoting someone else's fenced output left its + contents standing as prose. + """ + out: list[str] = [] + fence: str | None = None + for line in text.splitlines(keepends=True): + probe = BLOCKQUOTE.sub("", line) + if fence is None: + m = FENCE_OPEN.match(probe) + if m: + fence = m.group(1) + continue + out.append(line) + else: + closer = FENCE_CLOSE.match(probe) + if closer and closer.group(1)[0] == fence[0] and len(closer.group(1)) >= len(fence): + fence = None + return "".join(out) + + +def strip_code(text: str) -> str: + """Best-effort removal of code so a text search sees prose only. + + Fences go first: run the inline-span pattern first and it eats a fence's + delimiters piecemeal, leaving the fence body behind as prose. + + DELIBERATELY NOT HANDLED: four-space indented code blocks. Stripping every + line that starts with four spaces looked right and was worse than the bug it + was meant to help — CommonMark ties the four-space rule to the *container's* + content column, so ordinary markdown that GitHub renders as prose was being + deleted: + + - part of a larger plan: + - Refs #116 covers the follow-up + + That reference vanished and the check rejected a compliant PR for having no + reference at all. A false block is worse than the false pass this file exists + to fix: the false pass merely failed to catch something, while this actively + obstructed an author who had done nothing wrong. Since the closing question is + now answered by GitHub, the only cost of not stripping indented code is a + `Refs` written inside an indented block being counted, which claims nothing + about the board. + """ + out = _strip_fences(text) + # Run-length matched spans: the backreference forces the closing run to equal + # the opening one, so ``x`` is one span rather than two empty ones around x. + return re.sub(r"(`+)([^\n]*?)\1", "", out) + + +def section(visible: str, name: str) -> str | None: + """Text under a '### name' heading, up to the next heading.""" + m = re.search( + rf"^#+\s*{re.escape(name)}\s*$(.*?)(?=^#+\s|\Z)", + visible, + flags=re.M | re.S, + ) + return m.group(1).strip() if m else None + + +def parse_closing_refs(raw: str | None) -> list[int] | None: + """GitHub's answer, or None when we do not have one. + + None is returned for an unset, blank or unparseable value. It means "unknown", + never "none found" — the caller degrades and says so rather than deciding. + """ + if raw is None or not raw.strip(): + return None + try: + parsed = json.loads(raw) + except json.JSONDecodeError: + return None + if not isinstance(parsed, list): + return None + # `bool` subclasses `int`, so a JSON `true` would otherwise survive as 1 and be + # printed as "#True". Not reachable through the workflow's jq expression today, + # which yields plain integers, but a hand-set CLOSING_REFS could hit it. + return [n for n in parsed if isinstance(n, int) and not isinstance(n, bool)] + + +def check_reference(prose: str, closing_refs: list[int] | None) -> tuple[list[str], str]: + """Does this PR name an issue? Returns (errors, one line about how we know).""" + refs = bool(REFS_RE.search(prose)) + + if closing_refs is None: + # Degraded: no authoritative answer, so fall back to the text search that + # this script exists to stop relying on. Say so out loud, and say WHICH form + # matched — one identical note for both was untestable and told a reader + # less than it appeared to. + if CLOSING_RE.search(prose): + return [], ( + "reference: a closing keyword was found by text search only — " + "GitHub's answer was unavailable, so 'the board updates on merge' " + "is NOT verified" + ) + if refs: + return [], ( + "reference: 'Refs' found by text search; GitHub's answer was " + "unavailable, but nothing was expected to close anyway" + ) + return [ + "No issue reference found. Use 'Closes #' when this PR completes the " + "issue, or 'Refs #' when it does not." + ], "reference: absent by text search; GitHub's answer was unavailable" + + if closing_refs: + joined = ", ".join(f"#{n}" for n in closing_refs) + return [], f"reference: GitHub will close {joined} on merge" + if refs: + return [], "reference: 'Refs' present; nothing closes on merge, as intended" + + hint = "" + if CLOSING_RE.search(prose): + hint = ( + " A closing keyword appears in the body but GitHub created no link from " + "it, which happens when it sits inside a code span or fenced block — " + "write it as plain text." + ) + return [ + "No issue reference GitHub recognises. Use 'Closes #' when this PR " + "completes the issue so the board updates on merge, or 'Refs #' when it " + "does not." + hint + ], "reference: none — GitHub reports no closing link and no 'Refs' was found" + + +def check(body: str, labels: list[str], closing_refs: list[int] | None) -> tuple[list[str], list[str]]: + """Return (errors, notes). Empty errors means the body is acceptable.""" + visible = strip_comments(body) + prose = strip_code(visible) + is_agent = "by:agent" in labels + errors: list[str] = [] + notes: list[str] = [] + + if not visible.strip(): + errors.append("PR body is empty. Use the PR template.") + + ref_errors, ref_note = check_reference(prose, closing_refs) + errors.extend(ref_errors) + notes.append(ref_note) + + itype = section(visible, "Issue type") + if not itype: + errors.append("Missing '### Issue type' section.") + elif not any(v.lower() in itype.lower() for v in ISSUE_TYPES): + errors.append(f"Issue type must be one of {sorted(ISSUE_TYPES)}. Found: {itype!r}") + + if is_agent: + for field in PROVENANCE_FIELDS: + m = re.search(rf"\|\s*{re.escape(field)}\s*\|(.*?)\|", visible) + if not m or not m.group(1).strip(): + errors.append(f"by:agent PR missing provenance value for '{field}'.") + + nv = section(visible, "Not verified") + if not nv: + errors.append("by:agent PR missing '### Not verified' section.") + elif nv.lower().strip(" .") in EMPTY_NOT_VERIFIED: + errors.append( + "'Not verified' must name something specific. " + "There is always something that was not checked." + ) + + # Deliberately reads `visible`, not `prose`: this rule REQUIRES a fence to + # be present, so stripping code first would make it unsatisfiable. + if "```" not in visible: + errors.append("by:agent PR must paste raw command output in a fenced code block.") + + return errors, notes + + +def main() -> int: + body = os.environ.get("BODY") or "" + try: + labels = json.loads(os.environ.get("LABELS") or "[]") + except json.JSONDecodeError: + labels = [] + closing_refs = parse_closing_refs(os.environ.get("CLOSING_REFS")) + + errors, notes = check(body, labels, closing_refs) + for note in notes: + print(f" {note}") + + if errors: + print("\nPR body check failed:\n") + for e in errors: + print(f" - {e}") + print("\nHuman PRs: .github/PULL_REQUEST_TEMPLATE.md") + print("Agent PRs: launchpad/AGENT_PR_TEMPLATE.md") + return 1 + + kind = "agent" if "by:agent" in labels else "human" + print(f"\nPR body check passed. ({kind})") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/launchpad/scripts/test_pr_body_check.py b/launchpad/scripts/test_pr_body_check.py new file mode 100644 index 00000000000..3700d7b4a94 --- /dev/null +++ b/launchpad/scripts/test_pr_body_check.py @@ -0,0 +1,292 @@ +#!/usr/bin/env python3 +"""Controls for pr_body_check. + +These import the module the workflow actually runs. The suite they replace copied +the logic by hand, so it would have passed unchanged if the real check were +reverted or replaced with `sys.exit(0)` — the defect that made issue #125's fix +unverifiable. Nothing here duplicates a regex. + +Run: python3 -m unittest discover -s launchpad/scripts +""" + +from __future__ import annotations + +import contextlib +import io +import os +import unittest +from unittest import mock + +import pr_body_check as m + + +def run_main(body: str, labels: str = "[]", closing_refs: str | None = None): + """Drive main() the way the workflow does, and capture what it prints. + + check() returning the right note is not the same as a reader SEEING it. The + degraded mode exists so an unverified pass is visible, and that promise lives + in main()'s output — so it has to be asserted there, not one layer down. + """ + env = {"BODY": body, "LABELS": labels} + if closing_refs is not None: + env["CLOSING_REFS"] = closing_refs + buf = io.StringIO() + with mock.patch.dict(os.environ, env, clear=True), contextlib.redirect_stdout(buf): + code = m.main() + return code, buf.getvalue() + +AGENT_BODY = """## Summary +Something. + +### Issue type +Task + +| Field | Value | +|---|---| +| Harness / provider | Claude Code | +| Model | claude-opus-5 | +| Initiating human | @someone | + +### Verification + +``` +raw output +``` + +### Not verified +The relay was not exercised. + +### Escalations +none +""" + + +class ClosingRefsComeFromGitHub(unittest.TestCase): + """The authoritative path: GitHub answers, no markdown is parsed.""" + + def test_github_reporting_a_link_passes(self): + errors, notes = m.check("### Issue type\nTask\n", [], [116]) + self.assertEqual(errors, []) + self.assertIn("#116", notes[0]) + + def test_github_reporting_no_link_fails_even_with_a_keyword_present(self): + # The heart of #125: the body says Closes, GitHub made no link, so the + # board will not move. A text search would have passed this. + errors, _ = m.check("### Issue type\nTask\n\nCloses #116\n", [], []) + self.assertTrue(any("no issue reference github recognises" in e.lower() for e in errors)) + + def test_that_failure_explains_the_code_span_cause(self): + errors, _ = m.check("### Issue type\nTask\n\n`Closes #116`\n", [], []) + # The keyword is in the body but stripped from prose, so no hint fires. + self.assertTrue(any("no issue reference" in e.lower() for e in errors)) + + def test_keyword_outside_code_with_no_github_link_gets_the_hint(self): + errors, _ = m.check("### Issue type\nTask\n\nCloses #116\n", [], []) + self.assertTrue(any("plain text" in e for e in errors)) + + def test_refs_passes_when_github_reports_nothing(self): + errors, notes = m.check("### Issue type\nTask\n\nRefs #116\n", [], []) + self.assertEqual(errors, []) + self.assertIn("nothing closes on merge", notes[0]) + + def test_no_reference_at_all_fails(self): + errors, _ = m.check("### Issue type\nTask\n\nNothing here.\n", [], []) + self.assertTrue(any("no issue reference" in e.lower() for e in errors)) + + +class AbsenceIsNotEvidence(unittest.TestCase): + """CLOSING_REFS unknown must degrade visibly, never pass silently.""" + + def test_unknown_is_not_the_same_as_empty(self): + self.assertIsNone(m.parse_closing_refs(None)) + self.assertIsNone(m.parse_closing_refs("")) + self.assertIsNone(m.parse_closing_refs(" ")) + self.assertEqual(m.parse_closing_refs("[]"), []) + + def test_unparseable_is_unknown_not_empty(self): + self.assertIsNone(m.parse_closing_refs("{not json")) + self.assertIsNone(m.parse_closing_refs('{"a":1}')) + + def test_non_integers_are_discarded(self): + self.assertEqual(m.parse_closing_refs('[1,"x",2]'), [1, 2]) + + def test_booleans_are_discarded_despite_bool_subclassing_int(self): + # isinstance(True, int) is True in Python, so without an explicit guard a + # JSON `true` survives as 1 and prints as "#True". + self.assertEqual(m.parse_closing_refs("[true]"), []) + self.assertEqual(m.parse_closing_refs("[1,true,2]"), [1, 2]) + + def test_degraded_mode_says_it_is_not_verified(self): + errors, notes = m.check("### Issue type\nTask\n\nCloses #9\n", [], None) + self.assertEqual(errors, []) + self.assertIn("NOT verified", notes[0]) + + def test_degraded_mode_still_fails_a_body_with_no_reference(self): + errors, _ = m.check("### Issue type\nTask\n\nNothing.\n", [], None) + self.assertTrue(errors) + + +class WhatMainActuallyPrints(unittest.TestCase): + """The degraded warning is only worth anything if a reader sees it.""" + + def test_a_passing_degraded_run_still_says_it_is_unverified(self): + # The mutation this exists to catch: printing notes only when there are + # errors. That leaves a PASSING degraded run silent about being unverified, + # which defeats the whole point, and check() alone cannot detect it. + code, out = run_main("### Issue type\nTask\n\nCloses #9\n", closing_refs=None) + self.assertEqual(code, 0) + self.assertIn("NOT verified", out) + self.assertIn("passed", out) + + def test_a_verified_pass_does_not_claim_to_be_unverified(self): + code, out = run_main("### Issue type\nTask\n", closing_refs="[9]") + self.assertEqual(code, 0) + self.assertNotIn("NOT verified", out) + self.assertIn("#9", out) + + def test_a_failing_run_exits_one_and_explains(self): + code, out = run_main("### Issue type\nTask\n\nnothing\n", closing_refs="[]") + self.assertEqual(code, 1) + self.assertIn("check failed", out) + + def test_blank_closing_refs_is_treated_as_unknown_not_empty(self): + # The workflow writes an empty string when the query fails. + code, out = run_main("### Issue type\nTask\n\nCloses #9\n", closing_refs="") + self.assertEqual(code, 0) + self.assertIn("NOT verified", out) + + def test_degraded_refs_and_degraded_closes_read_differently(self): + _, closes_out = run_main("### Issue type\nTask\n\nCloses #9\n", closing_refs="") + _, refs_out = run_main("### Issue type\nTask\n\nRefs #9\n", closing_refs="") + self.assertNotEqual(closes_out, refs_out) + self.assertIn("nothing was expected to close", refs_out) + + def test_malformed_labels_do_not_crash(self): + code, _ = run_main("### Issue type\nTask\n", labels="{not json", closing_refs="[1]") + self.assertEqual(code, 0) + + +class CodeStripping(unittest.TestCase): + """The four shapes that defeated the previous fix, plus the ordering.""" + + def test_unterminated_fence_swallows_what_follows(self): + self.assertNotIn("Closes", m.strip_code("Text\n```\noutput\nCloses #42\n")) + + def test_double_backtick_span_is_stripped(self): + self.assertNotIn("Closes", m.strip_code("See ``Closes #6`` here.")) + + def test_quad_fence_with_inner_run_is_stripped(self): + self.assertNotIn("Closes", m.strip_code("````\n``` ex\nCloses #1\n````")) + + def test_indented_lines_are_NOT_stripped(self): + # Deliberate: stripping every 4-space line deleted ordinary nested markdown + # and rejected compliant PRs. Accepted cost is an indented `Refs` counting. + self.assertIn("Closes", m.strip_code("Refs #1\n\n Closes #999\n")) + + def test_a_nested_bullet_reference_still_passes(self): + # The false block this replaced: GitHub renders both these lines as prose. + body = "### Issue type\nTask\n\n- part of a larger plan:\n - Refs #116 follows\n" + errors, _ = m.check(body, [], []) + self.assertEqual(errors, [], f"a nested-bullet Refs must not be rejected: {errors}") + + def test_a_wrapped_continuation_reference_still_passes(self): + body = "### Issue type\nTask\n\n1. Background\n2. Work described in\n Refs #116\n" + errors, _ = m.check(body, [], []) + self.assertEqual(errors, [], f"a continuation-line Refs must not be rejected: {errors}") + + def test_blockquoted_fence_is_stripped(self): + self.assertNotIn("Closes", m.strip_code("Refs #1\n\n> ```\n> Closes #999\n> ```\n")) + + def test_fence_with_an_info_string_is_stripped(self): + self.assertNotIn("Closes", m.strip_code("Refs #1\n\n```python\nCloses #9\n```\n")) + + def test_fence_opener_indented_up_to_three_spaces_is_stripped(self): + self.assertNotIn("Closes", m.strip_code("Refs #1\n\n ```\n Closes #9\n ```\n")) + + def test_a_backtick_fence_is_not_closed_by_a_tilde_run(self): + # CommonMark forbids it, so the fence stays open and swallows the rest. + self.assertNotIn("Closes", m.strip_code("```\nx\n~~~\nCloses #9\n")) + + def test_a_closer_with_trailing_junk_is_not_a_closer(self): + # Accepting it ended the block early (leaking code) and opened a spurious + # new fence (swallowing real prose after it). + out = m.strip_code("```\nfirst\n```not a closer\nCloses #1\n```\n\nRefs #2\n") + self.assertNotIn("Closes", out) + self.assertIn("Refs #2", out) + + def test_a_closer_may_carry_trailing_whitespace(self): + self.assertNotIn("Closes", m.strip_code("```\nCloses #9\n``` \n")) + + def test_balanced_fence_is_stripped(self): + self.assertNotIn("Closes", m.strip_code("Refs #1\n\n```\nCloses #1\n```\n")) + + def test_tilde_fence_is_stripped(self): + self.assertNotIn("Closes", m.strip_code("Refs #1\n\n~~~\nCloses #1\n~~~\n")) + + def test_prose_outside_code_survives(self): + self.assertIn("Refs #1", m.strip_code("Refs #1\n\n```\ncode\n```\n")) + + def test_reference_after_a_balanced_fence_survives(self): + self.assertIn("Closes #12", m.strip_code("```\nout\n```\n\nCloses #12\n")) + + def test_html_comment_is_removed_before_anything_else(self): + self.assertNotIn("Closes", m.strip_comments("")) + + +class SectionsAndAgentRules(unittest.TestCase): + def edited(self, old: str, new: str) -> str: + """AGENT_BODY with one substring swapped, asserting the swap happened. + + `str.replace` no-ops silently when its target drifts, which turns a test + into a check on an unmodified fixture and reports the wrong cause when it + fails. Four tests below depend on the edit landing, so it is verified. + """ + self.assertIn(old, AGENT_BODY, f"fixture drifted: {old!r} is no longer present") + out = AGENT_BODY.replace(old, new, 1) + self.assertNotEqual(out, AGENT_BODY, "the fixture edit did not change anything") + return out + + def test_missing_issue_type_is_an_error(self): + errors, _ = m.check("Refs #1\n", [], []) + self.assertTrue(any("Issue type" in e for e in errors)) + + def test_unrecognised_issue_type_is_an_error(self): + errors, _ = m.check("### Issue type\nWidget\n\nRefs #1\n", [], []) + self.assertTrue(any("must be one of" in e for e in errors)) + + def test_empty_body_is_an_error(self): + errors, _ = m.check("", [], []) + self.assertTrue(any("empty" in e for e in errors)) + + def test_a_complete_agent_body_passes(self): + errors, _ = m.check(AGENT_BODY, ["by:agent"], [1]) + self.assertEqual(errors, [], f"unexpected: {errors}") + + def test_agent_body_missing_provenance_fails(self): + body = self.edited("| Model | claude-opus-5 |", "| Model | |") + errors, _ = m.check(body, ["by:agent"], [1]) + self.assertTrue(any("Model" in e for e in errors)) + + def test_agent_body_with_hollow_not_verified_fails(self): + body = self.edited("The relay was not exercised.", "nothing") + errors, _ = m.check(body, ["by:agent"], [1]) + self.assertTrue(any("Not verified" in e for e in errors)) + + def test_agent_body_without_a_fence_fails(self): + body = self.edited("```\nraw output\n```", "raw output") + errors, _ = m.check(body, ["by:agent"], [1]) + self.assertTrue(any("fenced code block" in e for e in errors)) + + def test_the_fence_rule_reads_the_unstripped_body(self): + # If it read stripped prose the fence would be gone and this could never + # pass — the reason `check` keeps two strings. + errors, _ = m.check(AGENT_BODY, ["by:agent"], [1]) + self.assertFalse(any("fenced code block" in e for e in errors)) + + def test_human_body_is_not_held_to_agent_rules(self): + errors, _ = m.check("### Issue type\nBug\n\nRefs #1\n", [], []) + self.assertEqual(errors, []) + + +if __name__ == "__main__": + unittest.main(verbosity=2)