From fee41002c61362a7bd151b34ff6ecd560a432b9f Mon Sep 17 00:00:00 2001 From: Deon Menezes Date: Sat, 29 Aug 2026 16:45:21 -0700 Subject: [PATCH] Test the verdict parser against the reviews that broke it Closes #19. Every defect in the merge gate was found after it shipped: four by Qodo reviewing pull request 14, and a fifth by hand, when it turned out Qodo edits its verdict into an existing comment so the merge step had never once run. Four rounds of review examined what the gate does when it runs. None established that it runs. The parser is the part where a bug merges unreviewed code, and it was an inline heredoc inside the workflow, so it could not be run without opening a pull request. It moves to .github/scripts/qodo_verdict.py and gets fixtures captured from real comments in this repository. The important fixture is Qodo's own four-bug review of the gate, which contains the words "no issues found" twice inside its findings while reporting four bugs: the first version matched that phrase anywhere in the body and would have merged the pull request that broke it. That review no longer exists through the API, because Qodo edited the comment in place once the findings were fixed, so it is preserved here from a copy taken while it was live. A second fixture came from getting the first one wrong. Selecting comments that matched "Bugs (4)" captured a human reply that quoted the phrase in prose, which is the same free-text mistake one layer up, so that comment is now a test case too. The workflow checks out the default branch to get the parser rather than the pull request's copy, so a pull request cannot edit the rules that decide whether it merges. Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: qodo-code-review[bot] <151058649+qodo-code-review[bot]@users.noreply.github.com> Claude-Session: https://claude.ai/code/session_01EqKPWQMhP7XrA3THHV1hkM --- .github/scripts/qodo_verdict.py | 48 ++++ .github/workflows/bun-ci.yml | 5 + .github/workflows/qodo-automerge.yml | 38 +-- .gitignore | 4 + tests/fixtures/human-quoting-counters.md | 20 ++ tests/fixtures/qodo-clean.html | 35 +++ tests/fixtures/qodo-four-bugs.html | 319 +++++++++++++++++++++++ tests/fixtures/qodo-placeholder.html | 3 + tests/fixtures/qodo-summary.html | 137 ++++++++++ tests/test_qodo_verdict.py | 107 ++++++++ 10 files changed, 685 insertions(+), 31 deletions(-) create mode 100644 .github/scripts/qodo_verdict.py create mode 100644 tests/fixtures/human-quoting-counters.md create mode 100644 tests/fixtures/qodo-clean.html create mode 100644 tests/fixtures/qodo-four-bugs.html create mode 100644 tests/fixtures/qodo-placeholder.html create mode 100644 tests/fixtures/qodo-summary.html create mode 100644 tests/test_qodo_verdict.py diff --git a/.github/scripts/qodo_verdict.py b/.github/scripts/qodo_verdict.py new file mode 100644 index 0000000..b986576 --- /dev/null +++ b/.github/scripts/qodo_verdict.py @@ -0,0 +1,48 @@ +"""Read a Qodo review comment and decide whether it authorises a merge. + +Kept out of the workflow so it can be tested. Every rule here exists because +something got past an earlier version of it, so the tests in +tests/test_qodo_verdict.py are the specification, not an afterthought. +""" + +import re + +# Qodo renders its counters as chips. Everything else in the comment is +# free text that quotes findings and diff hunks verbatim, so a phrase like +# "no issues found" appears inside reviews that are not clean and cannot be +# used as a signal. +_COUNTER = re.compile(r"[^<]*?\((\d+)\)") +_BUGS_CHIP = re.compile(r"[^<]*Bugs \(\d+\)") +_COMMIT = re.compile(r"/commit/([0-9a-f]{40})") + + +def parse(body: str) -> tuple[bool, str, list[str]]: + """Return (clean, reviewed_sha, counters). + + clean is True only when every counter is zero, the Bugs chip is present, and + the comment names the commit it reviewed. Anything unrecognised is not clean: + a comment shape this does not understand must stall a merge, never allow one. + """ + counters = _COUNTER.findall(body) + has_bugs = _BUGS_CHIP.search(body) is not None + match = _COMMIT.search(body) + sha = match.group(1) if match else "" + + clean = bool(counters) and has_bugs and all(c == "0" for c in counters) and bool(sha) + return clean, sha, counters + + +if __name__ == "__main__": + import os + import sys + + clean, sha, counters = parse(os.environ.get("BODY", "")) + out = [ + f"clean={'true' if clean else 'false'}", + f"reviewed_sha={sha}", + f"counters={','.join(counters) if counters else 'none'}", + ] + print("\n".join(out)) + # Also echo to stderr so the run log shows the decision without needing the + # step output, which is written to a file. + print(f"verdict clean={clean} sha={sha[:12]} counters={counters}", file=sys.stderr) diff --git a/.github/workflows/bun-ci.yml b/.github/workflows/bun-ci.yml index ad9e0f0..ab06c45 100644 --- a/.github/workflows/bun-ci.yml +++ b/.github/workflows/bun-ci.yml @@ -41,3 +41,8 @@ jobs: - name: Test run: bun run test + + # The merge gate's parser decides whether unreviewed code can land, so it + # is tested here rather than only in the workflow that uses it. + - name: Test the Qodo verdict parser + run: python3 tests/test_qodo_verdict.py diff --git a/.github/workflows/qodo-automerge.yml b/.github/workflows/qodo-automerge.yml index 8073ea8..e47f1fe 100644 --- a/.github/workflows/qodo-automerge.yml +++ b/.github/workflows/qodo-automerge.yml @@ -33,41 +33,17 @@ jobs: github.event.sender.login == 'qodo-code-review[bot]' runs-on: ubuntu-latest steps: + # issue_comment runs against the default branch, which is where the + # parser lives; the pull request's own copy is deliberately not used, so a + # pull request cannot change the rules that decide whether it merges. + - name: Check out the parser + uses: actions/checkout@v4 + - name: Read the verdict and the commit it applies to id: verdict env: BODY: ${{ github.event.comment.body }} - run: | - python3 - <<'PY' >> "$GITHUB_OUTPUT" - import os, re - - body = os.environ["BODY"] - - # Parse only the structured counters Qodo renders as chips, never - # free text. Qodo quotes findings and diff hunks verbatim, so a phrase - # like "no issues found" appears inside the body of reviews that are not - # clean, and any substring test on the whole comment is forgeable by the - # PR's own content. - counters = re.findall(r"[^<]*?\((\d+)\)", body) - - # The third chip is named differently across review types ("Requirement - # gaps", "Skill insights"), so require the one that is always present - # rather than a fixed set. - has_bugs = re.search(r"[^<]*Bugs \(\d+\)", body) is not None - - clean = bool(counters) and has_bugs and all(c == "0" for c in counters) - - # Bind the verdict to the revision Qodo actually read. Qodo footers the - # comment with the reviewed commit; with no such marker there is no - # verifiable revision identity and this must not merge. - m = re.search(r"/commit/([0-9a-f]{40})", body) - sha = m.group(1) if m else "" - - print(f"clean={'true' if (clean and sha) else 'false'}") - print(f"reviewed_sha={sha}") - print(f"counters={','.join(counters) if counters else 'none'}") - PY - echo "Parsed verdict -> clean=${{ steps.verdict.outputs.clean }}" || true + run: python3 .github/scripts/qodo_verdict.py >> "$GITHUB_OUTPUT" - name: Merge the reviewed commit if: steps.verdict.outputs.clean == 'true' diff --git a/.gitignore b/.gitignore index 8d47eeb..e599c84 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,7 @@ node_modules/ # TrueForge local data (SQLite lives in the OS app data dir; this is just a run dir) .trueforge/ + +# python bytecode from the merge-gate parser and its tests +__pycache__/ +*.pyc diff --git a/tests/fixtures/human-quoting-counters.md b/tests/fixtures/human-quoting-counters.md new file mode 100644 index 0000000..82233d1 --- /dev/null +++ b/tests/fixtures/human-quoting-counters.md @@ -0,0 +1,20 @@ +Acted on all four findings. Every one was real, and the first was demonstrated on this very PR. + +**1. Free-text verdict matching.** `grep -qF 'no issues found'` tested the whole comment body. Qodo quotes findings and diff hunks verbatim, so that phrase appears inside reviews that are *not* clean — including the review above, which contains it while reporting `Bugs (4)`. The gate would have merged the PR that broke it. Now only the structured `` counter chips are parsed; every counter must be `0`, and the Bugs chip must be present so an unrecognised comment shape can't pass by having no counters at all. + +**2. No binding to the reviewed revision.** This was live, not theoretical: Qodo reviewed `eccd352` while the head had already moved to `947095f`. The reviewed SHA now comes from the commit marker Qodo footers in the comment, must equal `headRefOid`, and the merge passes `--match-head-commit` so a push racing the comparison is rejected by GitHub rather than slipping in. No marker means no verifiable revision identity, so it fails closed. + +**3. Read failures counted as clean.** Both queries ended in `|| echo 0`, so an outage or auth error became "nothing failed, nothing pending" and merged without confirming the build ran. Check state is now read per-commit via the API, a read failure exits non-zero, and the `build` check must be present *and* successful rather than merely not failing. + +**4. Unreachable pending branch.** Correct on the exit code: `gh pr checks` exits 8 while pending, so `|| echo 0` appended a second zero, making the count `0\n0` — not equal to `0` — so pending checks took the failure exit and the branch meant to handle them never ran. Replaced with a bounded poll against the check-runs API, whose transport status is independent of check conclusions. + +Parser verified against the real comments on this repo: + +| Case | Result | +| --- | --- | +| This review (4 bugs, prose contains "no issues found") | not clean | +| #13 review (0/0/0) | clean | +| "Qodo is busy working" | not clean | +| Clean counters, no commit marker | not clean | + +On the suggested alternative of publishing a required status: agreed it's the better shape, and worth doing if this outlives the hackathon. It needs branch protection plus a status-publishing step, which is more moving parts than a one-day repo warrants — the trade recorded here rather than left implicit. diff --git a/tests/fixtures/qodo-clean.html b/tests/fixtures/qodo-clean.html new file mode 100644 index 0000000..78ddb86 --- /dev/null +++ b/tests/fixtures/qodo-clean.html @@ -0,0 +1,35 @@ + +

Code Review by Qodo

+🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) + +Grey Divider + + + +

Great, no issues found!

+Qodo reviewed your code and found no material issues that require review + +Grey Divider + + + + + +
+Tip of the day + +
+ +
💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full
+ +More tips ↗ | Customize Qodo ↗ | Qodo docs ↗ + +
+ +Grey Divider + + + + + +Qodo Logo diff --git a/tests/fixtures/qodo-four-bugs.html b/tests/fixtures/qodo-four-bugs.html new file mode 100644 index 0000000..71c2385 --- /dev/null +++ b/tests/fixtures/qodo-four-bugs.html @@ -0,0 +1,319 @@ +

Code Review by Qodo

+ +🐞 Bugs (4) 📘 Rule violations (0) 📜 Skill insights (0) + +Grey Divider + +
+ +Action required + +
+ 1. Quoted text approves review 🐞 Bug ⛨ Security + +
+ +>
+>Description +>
+> +>
+>The verdict parser marks the entire Qodo comment clean when no issues found occurs anywhere,
+>including a finding or quoted diff snippet. A non-clean review that quotes PR-controlled text
+>containing this phrase therefore proceeds to the merge step.
+>
+>
+ +>
+>Code +>
+> +>[.github/workflows/qodo-automerge.yml[R36-37]](https://github.com/deonmenezes/edit-ai/pull/14/files#diff-08f97bc772f762f2b8b6047c4b37dc1f1c57672ffcb6333f0a077879ea3a9d32R36-R37) +> +>```diff +>+ if grep -qF 'no issues found' <<<"$BODY"; then +>+ clean=true +>``` +>
+ +>
+>Evidence +>
+> +>
+>The workflow's unanchored grep directly sets clean=true, and that output alone enables the merge
+>step. A public Qodo review demonstrates that Qodo comments include findings and quoted source code,
+>so occurrence anywhere in the free-form body is not equivalent to the authoritative verdict.
+>
+> +> [.github/workflows/qodo-automerge.yml[31-47]](https://github.com/deonmenezes/edit-ai/blob/eccd35235e14dd6dba1f11d89bb648be418efd0a/.github/workflows/qodo-automerge.yml/#L31-L47) +> 🌐 [The Qodo review shown on this PR contains issue prose and quoted source snippets in the same comment body, demonstrating that arbitrary review text is mixed with the verdict.](https://github.com/qodo-ai/pr-agent/pull/1859) +>
+ +>
+>Agent prompt +>
+> +>``` +>The issue below was found during a code review. Follow the provided context and guidance below and implement a solution +> +>## Issue description +>The workflow treats an arbitrary substring in Qodo's free-form comment as a clean verdict, so findings or quoted PR content can falsely authorize a merge. +> +>## Issue Context +>Qodo review comments can contain explanatory findings and source snippets. Validate a uniquely identified summary section or structured verdict, and reject ambiguous bodies or bodies containing nonzero findings. +> +>## Fix Focus Areas +>- .github/workflows/qodo-automerge.yml[31-42] +>``` +> ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools +>
+ +
+
+ + +
+ 2. Verdict ignores reviewed commit 🐞 Bug ⛨ Security + +
+ +>
+>Description +>
+> +>
+>The workflow parses a comment without identifying the commit Qodo reviewed, then queries checks and
+>merges whatever head currently belongs to the PR number. If a new commit reaches the branch after
+>the review comment is created but before this job reads the PR, the newer unreviewed head can be
+>merged.
+>
+>
+ +>
+>Code +>
+> +>[.github/workflows/qodo-automerge.yml[R55-56]](https://github.com/deonmenezes/edit-ai/pull/14/files#diff-08f97bc772f762f2b8b6047c4b37dc1f1c57672ffcb6333f0a077879ea3a9d32R55-R56) +> +>```diff +>+ state=$(gh pr view "$PR" --repo "$REPO" \ +>+ --json state,isDraft,mergeable --jq '[.state,.isDraft,.mergeable]|@tsv') +>``` +>
+ +>
+>Evidence +>
+> +>
+>The event supplies only the comment body to the parser, while gh pr view omits headRefOid and
+>all later commands target the current PR by number. GitHub also documents that issue_comment
+>workflows run with the default branch's commit/ref rather than the pull request head, so the event
+>itself does not bind this run to the reviewed PR revision.
+>
+> +> [.github/workflows/qodo-automerge.yml[7-9]](https://github.com/deonmenezes/edit-ai/blob/eccd35235e14dd6dba1f11d89bb648be418efd0a/.github/workflows/qodo-automerge.yml/#L7-L9) +> [.github/workflows/qodo-automerge.yml[27-43]](https://github.com/deonmenezes/edit-ai/blob/eccd35235e14dd6dba1f11d89bb648be418efd0a/.github/workflows/qodo-automerge.yml/#L27-L43) +> [.github/workflows/qodo-automerge.yml[55-83]](https://github.com/deonmenezes/edit-ai/blob/eccd35235e14dd6dba1f11d89bb648be418efd0a/.github/workflows/qodo-automerge.yml/#L55-L83) +> 🌐 [For `issue_comment`, GitHub documents `GITHUB_SHA` as the last commit on the default branch and `GITHUB_REF` as the default branch, not the pull request head.](https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows) +>
+ +>
+>Agent prompt +>
+> +>``` +>The issue below was found during a code review. Follow the provided context and guidance below and implement a solution +> +>## Issue description +>A Qodo verdict is not tied to the PR head commit that the workflow ultimately checks and merges. +> +>## Issue Context +>Obtain the reviewed commit SHA from authoritative Qodo metadata and compare it with the current `headRefOid` immediately before merging. If Qodo provides no verifiable revision identity, fail closed rather than treating the comment as authorization for the current tip. +> +>## Fix Focus Areas +>- .github/workflows/qodo-automerge.yml[27-43] +>- .github/workflows/qodo-automerge.yml[55-83] +>``` +> ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools +>
+ +
+
+ + +
+ 3. Check errors permit merging 🐞 Bug ☼ Reliability + +
+ +>
+>Description +>
+> +>
+>Both check queries convert every CLI/API failure into 0, so an outage, authorization error,
+>malformed response, or absence of reported checks is interpreted as no failed and no pending checks.
+>The workflow then executes the direct merge without establishing that the Bun build passed.
+>
+>
+ +>
+>Code +>
+> +>[.github/workflows/qodo-automerge.yml[R68-71]](https://github.com/deonmenezes/edit-ai/pull/14/files#diff-08f97bc772f762f2b8b6047c4b37dc1f1c57672ffcb6333f0a077879ea3a9d32R68-R71) +> +>```diff +>+ failed=$(gh pr checks "$PR" --repo "$REPO" --json name,state \ +>+ --jq '[.[] | select(.state == "FAILURE" or .state == "ERROR" or .state == "CANCELLED")] | length' 2>/dev/null || echo 0) +>+ pending=$(gh pr checks "$PR" --repo "$REPO" --json name,state \ +>+ --jq '[.[] | select(.state == "PENDING" or .state == "IN_PROGRESS" or .state == "QUEUED")] | length' 2>/dev/null || echo 0) +>``` +>
+ +>
+>Evidence +>
+> +>
+>Each gh pr checks command has || echo 0; the only subsequent blockers are nonzero string
+>comparisons, and zero/zero falls through to gh pr merge. The repository's actual build and test
+>check is the build job in Bun CI, but this workflow never requires that result to be present.
+>
+> +> [.github/workflows/qodo-automerge.yml[68-83]](https://github.com/deonmenezes/edit-ai/blob/eccd35235e14dd6dba1f11d89bb648be418efd0a/.github/workflows/qodo-automerge.yml/#L68-L83) +> [.github/workflows/bun-ci.yml[17-43]](https://github.com/deonmenezes/edit-ai/blob/eccd35235e14dd6dba1f11d89bb648be418efd0a/.github/workflows/bun-ci.yml/#L17-L43) +>
+ +>
+>Agent prompt +>
+> +>``` +>The issue below was found during a code review. Follow the provided context and guidance below and implement a solution +> +>## Issue description +>Failures retrieving check status are converted to zero and allow an unchecked direct merge. +> +>## Issue Context +>Separate command execution success from parsed check counts. Abort on retrieval/parsing errors and require the expected CI result, or enforce the build as a required branch check, before merging. +> +>## Fix Focus Areas +>- .github/workflows/qodo-automerge.yml[68-83] +>- .github/workflows/bun-ci.yml[17-43] +>``` +> ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools +>
+ +
+
+ + +
View high (1)
+
+ 4. Pending checks never hand off 🐞 Bug ≡ Correctness + +
+ +>
+>Description +>
+> +>
+>gh pr checks exits with status 8 while checks are pending, so || echo 0 appends another 0 to
+>the first query's output and makes failed a multiline value such as 0\n0. The line 73 comparison
+>then treats that value as a failure and exits before the intended pending-check auto-merge branch.
+>
+>
+ +>
+>Code +>
+> +>[.github/workflows/qodo-automerge.yml[R68-69]](https://github.com/deonmenezes/edit-ai/pull/14/files#diff-08f97bc772f762f2b8b6047c4b37dc1f1c57672ffcb6333f0a077879ea3a9d32R68-R69) +> +>```diff +>+ failed=$(gh pr checks "$PR" --repo "$REPO" --json name,state \ +>+ --jq '[.[] | select(.state == "FAILURE" or .state == "ERROR" or .state == "CANCELLED")] | length' 2>/dev/null || echo 0) +>``` +>
+ +>
+>Evidence +>
+> +>
+>The fallback is inside command substitution, and the first guard compares the entire captured string
+>to exactly 0. The official GitHub CLI manual documents exit code 8 for pending checks, proving
+>that the fallback runs in precisely the case intended for lines 76-80.
+>
+> +> [.github/workflows/qodo-automerge.yml[68-80]](https://github.com/deonmenezes/edit-ai/blob/eccd35235e14dd6dba1f11d89bb648be418efd0a/.github/workflows/qodo-automerge.yml/#L68-L80) +> 🌐 [The official `gh pr checks` manual documents additional exit code 8 meaning checks are pending.](https://cli.github.com/manual/gh_pr_checks) +>
+ +>
+>Agent prompt +>
+> +>``` +>The issue below was found during a code review. Follow the provided context and guidance below and implement a solution +> +>## Issue description +>The documented pending-check exit status contaminates the captured count, causing pending checks to be handled as failures before auto-merge can be enabled. +> +>## Issue Context +>Do not append fallback output to successful JSON output. Explicitly distinguish exit status 8 from retrieval failures, or query check runs through an API whose transport status is independent from check conclusions. +> +>## Fix Focus Areas +>- .github/workflows/qodo-automerge.yml[68-80] +>``` +> ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools +>
+ +
+
+ + +
+ +Grey Divider + + + +
Context sources + +
✅ Compliance rules (platform): 30 rules
+
✅ Web pages:
+ + + +
  +17 more
+
Review mode: ⚖️ Balanced: This changes CI merge automation with repository write permissions, bot-authentication checks, verdict parsing, and build-gating behavior; it is security- and release-sensitive but localized enough for one careful review.
+ +
+ +Grey Divider + + + + + +
+Tip of the day + +
+ +
💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full
+ +More tips ↗ | Customize Qodo ↗ | Qodo docs ↗ + +
+ +Grey Divider + + + + \ No newline at end of file diff --git a/tests/fixtures/qodo-placeholder.html b/tests/fixtures/qodo-placeholder.html new file mode 100644 index 0000000..c34c135 --- /dev/null +++ b/tests/fixtures/qodo-placeholder.html @@ -0,0 +1,3 @@ + +

Qodo is busy working

+Check back in a few minutes. Qodo's code review agents are on it. diff --git a/tests/fixtures/qodo-summary.html b/tests/fixtures/qodo-summary.html new file mode 100644 index 0000000..be16a2e --- /dev/null +++ b/tests/fixtures/qodo-summary.html @@ -0,0 +1,137 @@ +

PR Summary by Qodo

+ +Handle edited Qodo verdict comments in the auto-merge gate + +🐞 Bug fix ⚙️ Configuration changes 📝 Documentation 🕐 Less than 10 minutes + +Grey Divider + +
+AI Description + +
+
+
+ +>
+>• Trigger the auto-merge gate when Qodo edits its placeholder with a verdict.
+>• Require Qodo as both comment author and event sender before processing.
+>• Document why edited events and sender verification are necessary.
+>
+ +
+
+ +
+ +
+Diagram + +
+
+ +
+ +```mermaid +sequenceDiagram + actor Qodo + participant GitHub + participant Gate as Auto-merge gate + participant Merge as Merge job + Qodo->>GitHub: Create placeholder + GitHub->>Gate: created event + Gate-->>GitHub: No verdict + Qodo->>GitHub: Edit with verdict + GitHub->>Gate: edited event + Gate->>Gate: Verify author and sender + Gate->>Merge: Clean reviewed verdict + Merge->>GitHub: Merge matching head +``` + +
+
+ +
+ + + + +
+High-Level Assessment + +
+
+ +
+ +>Listening to both lifecycle events is the smallest reliable fix because Qodo publishes its verdict by editing the original placeholder. Polling comments or replacing the integration with a check-run bridge would add latency and operational complexity, while sender validation safely addresses the new edited-event trust ambiguity. + +
+
+ +
+ +
+ Files changed (2) +16 / -3 + +
+
+ +
+ +
+Bug fix (1) +10 / -3 + +
+
+ +
+qodo-automerge.ymlProcess edited Qodo verdict comments securely +10/-3 + +
+ +>Process edited Qodo verdict comments securely +> +>
+>• Adds edited issue comments to the workflow trigger so Qodo's final verdict reaches the merge gate. Requires the event sender to be Qodo in addition to validating the comment owner and bot account type, preventing human edits from being treated as bot-authored verdicts.
+>
+> +>.github/workflows/qodo-automerge.yml + +
+ +
+
+ +
+ +
+Documentation (1) +6 / -0 + +
+
+ +
+README.mdDocument edited-verdict event handling +6/-0 + +
+ +>Document edited-verdict event handling +> +>
+>• Explains Qodo's placeholder-then-edit behavior and why the auto-merge workflow checks both the comment owner and the editor identity.
+>
+> +>README.md + +
+ +
+
+ +
+ +
+
+ +
diff --git a/tests/test_qodo_verdict.py b/tests/test_qodo_verdict.py new file mode 100644 index 0000000..a1d1c12 --- /dev/null +++ b/tests/test_qodo_verdict.py @@ -0,0 +1,107 @@ +"""Fixtures are real comments from this repository, not invented ones. + +Each case here is a way an earlier version of the gate could be fooled, or was. + +Run with `python3 tests/test_qodo_verdict.py`. Deliberately no pytest: this repo +tests with bun, and a single pure function does not justify adding a Python test +dependency to CI. +""" + +import pathlib +import sys + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parents[1] / ".github" / "scripts")) + +from qodo_verdict import parse # noqa: E402 + +FIXTURES = pathlib.Path(__file__).parent / "fixtures" + + +def load(name: str) -> str: + return (FIXTURES / name).read_text() + + +def test_findings_are_not_clean_even_when_the_prose_says_otherwise(): + # This is the case that broke the first version. Qodo quotes findings + # verbatim, so its review of the parser contains the literal words the + # parser was matching on, while reporting four bugs. + body = load("qodo-four-bugs.html") + assert "no issues found" in body, "fixture no longer exercises the trap" + clean, _, counters = parse(body) + assert clean is False + assert counters[0] != "0" + + +def test_a_human_quoting_counter_text_is_not_a_verdict(): + # This fixture exists because capturing the one above with a naive filter + # grabbed a human reply that quoted "Bugs (4)" in prose. Free text that + # mentions counters is not a verdict, and the chips are markup a comment + # body cannot accidentally contain. + clean, _, counters = parse(load("human-quoting-counters.md")) + assert clean is False + assert counters == [] + + +def test_all_zero_counters_are_clean(): + clean, sha, counters = parse(load("qodo-clean.html")) + assert clean is True + assert set(counters) == {"0"} + assert len(sha) == 40 + + +def test_placeholder_is_not_a_verdict(): + # Qodo posts this first and edits the verdict in later. It has no counters, + # so it must not be read as an all-clear. + clean, sha, counters = parse(load("qodo-placeholder.html")) + assert clean is False + assert counters == [] + assert sha == "" + + +def test_summary_comment_is_not_a_verdict(): + # The PR summary describes the change and carries no counters. + clean, _, _ = parse(load("qodo-summary.html")) + assert clean is False + + +def test_clean_counters_without_a_commit_marker_are_not_clean(): + # No commit marker means no verifiable revision, so there is nothing to + # bind the verdict to and the merge must not proceed. + body = ( + "\U0001f41e Bugs (0) Rule violations (0) " + "

Great, no issues found!

" + ) + clean, sha, _ = parse(body) + assert clean is False + assert sha == "" + + +def test_a_missing_bugs_chip_is_not_clean(): + # Guards against an unrecognised comment shape passing by having no + # counters at all, or only counters this does not know about. + body = "Rule violations (0) /commit/" + "a" * 40 + clean, _, _ = parse(body) + assert clean is False + + +def test_nonzero_in_any_counter_is_not_clean(): + body = ( + "\U0001f41e Bugs (0) Rule violations (1) " + "/commit/" + "b" * 40 + ) + clean, _, _ = parse(body) + assert clean is False + + +if __name__ == "__main__": + tests = [v for k, v in sorted(globals().items()) if k.startswith("test_")] + failures = [] + for fn in tests: + try: + fn() + print(f" pass {fn.__name__}") + except Exception as exc: # noqa: BLE001 + failures.append((fn.__name__, exc)) + print(f" FAIL {fn.__name__}: {exc}") + print(f"\n{len(tests) - len(failures)} passed, {len(failures)} failed") + raise SystemExit(1 if failures else 0)