Skip to content

check_bot_anchoring.sh: the review-state branch has no fixture, so deleting it leaves every test green - #456

Closed
jaylfc wants to merge 1 commit into
masterfrom
exec/tsk-6pc3tu
Closed

check_bot_anchoring.sh: the review-state branch has no fixture, so deleting it leaves every test green#456
jaylfc wants to merge 1 commit into
masterfrom
exec/tsk-6pc3tu

Conversation

@jaylfc

@jaylfc jaylfc commented Aug 31, 2026

Copy link
Copy Markdown
Owner

CARD TITLE (intent, not commit subject): check_bot_anchoring.sh: the review-state branch has no fixture, so deleting it leaves every test green

Autonomous build of board card tsk-6pc3tu.

The bot-anchoring gate's review-state filter was behaviourally live but
unpinned by its own tests: a bot review in a DISMISSED state at head makes the
gate exit rc=10, yet no fixture exercised that branch. That is why a 9-mutant
campaign survivor on the filter (mutating line 34 to if False:) left every
test green, deleting the filter while the suite still reported PASSED.

Add pr_bot_dismissed.json, a coderabbitai review in DISMISSED state at head
with a non-empty body, and test_bot_dismissed_at_head_fails asserting the gate
rejects it (rc=10, "FAILED: ...").

Red-first proof (counts read from the pytest summary, never from $? -- the
pre-existing taosmd.mcp_server.build_server import errors pin exit status at 1):

  • Mutate line 34 if state not in (...) to if False: by line number via
    sed, then: uv run --extra dev pytest tests/test_merge_gate.py -q -> 1 failed,
    6 passed (FAILED_COUNT=1, ERROR_COUNT=0). The mutated gate emits
    "SUCCESS: anchored substantive bot review found" (rc=0) for the dismissed
    review, and the new test catches it.
  • Restore line 34: uv run --extra dev pytest tests/test_merge_gate.py -q ->
    7 passed.

Anchoring and author filter branches are untouched; they are already pinned.

changelog: tsk-6pc3tu-merge-gate-review-state-fixture

Files:
changelog.d/tsk-6pc3tu-merge-gate-review-state-fixture.md | 3 +++
tests/fixtures/merge_gate/pr_bot_dismissed.json | 14 ++++++++++++++
tests/test_merge_gate.py | 11 +++++++++++
3 files changed, 28 insertions(+)

…dismissed-bot-review fixture

The bot-anchoring gate's review-state filter was behaviourally live but
unpinned by its own tests: a bot review in a DISMISSED state at head makes the
gate exit rc=10, yet no fixture exercised that branch. That is why a 9-mutant
campaign survivor on the filter (mutating line 34 to `if False:`) left every
test green, deleting the filter while the suite still reported PASSED.

Add pr_bot_dismissed.json, a coderabbitai review in DISMISSED state at head
with a non-empty body, and test_bot_dismissed_at_head_fails asserting the gate
rejects it (rc=10, "FAILED: ...").

Red-first proof (counts read from the pytest summary, never from $? -- the
pre-existing taosmd.mcp_server.build_server import errors pin exit status at 1):
  - Mutate line 34 `if state not in (...)` to `if False:` by line number via
    sed, then: uv run --extra dev pytest tests/test_merge_gate.py -q -> 1 failed,
    6 passed (FAILED_COUNT=1, ERROR_COUNT=0). The mutated gate emits
    "SUCCESS: anchored substantive bot review found" (rc=0) for the dismissed
    review, and the new test catches it.
  - Restore line 34: uv run --extra dev pytest tests/test_merge_gate.py -q ->
    7 passed.

Anchoring and author filter branches are untouched; they are already pinned.

changelog: tsk-6pc3tu-merge-gate-review-state-fixture
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 52 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c102d9c5-76c2-41f0-8384-0579956953a5

📥 Commits

Reviewing files that changed from the base of the PR and between 0baa9af and e77024f.

📒 Files selected for processing (3)
  • changelog.d/tsk-6pc3tu-merge-gate-review-state-fixture.md
  • tests/fixtures/merge_gate/pr_bot_dismissed.json
  • tests/test_merge_gate.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gitar-bot

gitar-bot Bot commented Aug 31, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar

@kilo-code-bot

kilo-code-bot Bot commented Aug 31, 2026

Copy link
Copy Markdown

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Red-first fixture/test pair correctly pins the state not in ('COMMENTED', 'APPROVED', 'CHANGES_REQUESTED') filter in scripts/merge-gate/check_bot_anchoring.sh:34. Traced the logic: with a coderabbitai author, DISMISSED state, matching commit.oid/headRefOid, and a non-empty body, the gate falls through to print('FAILED: ...'); sys.exit(10) — and would emit SUCCESS if the filter were mutated to if False:, which the new test catches. The fixture follows the existing pr_anchored_head.json shape exactly. Changelog category (### Fixed) is consistent with prior test-hardening entries in this directory.

Files Reviewed (3 files)
  • changelog.d/tsk-6pc3tu-merge-gate-review-state-fixture.md
  • tests/fixtures/merge_gate/pr_bot_dismissed.json
  • tests/test_merge_gate.py

Reviewed by minimax-m3:free · Input: 28.8K · Output: 2.4K · Cached: 177.3K

@jaylfc

jaylfc commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

Adjudicated PASS. Waiting on a merge call, not on review.

Posting the verdict here because it lived only in my queue.

Why this one is trusted, given that CI cannot vouch for it. scripts/merge-gate/ is wired into no CI workflow, so a green CI proves nothing about this change. The adjudicator therefore ran the gate itself, end to end, with a fake gh on PATH and rc captured directly: the bot-dismissed fixture returns rc=10 FAILED, the anchored-head fixture returns rc=0 SUCCESS as a positive control, and against real PRs it gives #190 rc=10 and #451 rc=0. The new fixture differs from the passing one in exactly one field, so the split can only come from the line under test.

It is pinned, not merely covered. Three separate mutations of that line all SURVIVE against master's tests and all are KILLED by the new test, with two control mutations killing in both arms and zero errors on every row, so no survivor is a syntax error masquerading as one. That is the standard other PRs on this board have been blocked for failing.

Suite on the merged tree 1807 passed / 0 failed / 10 skipped / 7 errors against the master baseline of 1806/10/7 for bb8dfc09, so exactly the one new test and nothing else. Trailing newline present, deleted-symbols clean, ruff clean, and the CI step conclusions were read individually rather than off the combined tick.

Touches the same files as open PR #419, but a merge-tree check shows zero conflicts between them, so merge order does not matter.

Context worth knowing before merge: tsk-ergobm, confirmed 2026-09-04, is that none of scripts/merge-gate/* is referenced anywhere in .github/. This is a real fixture for a gate nothing currently runs, so it is a prerequisite for wiring that gate up rather than a fix to a live one.

@jaylfc

jaylfc commented Sep 6, 2026

Copy link
Copy Markdown
Owner Author

Closing on Jay's per-PR call (2026-09-06). scripts/merge-gate/check_bot_anchoring.sh is referenced nowhere under .github/ (confirmed on card tsk-ergobm, 2026-09-04), so no workflow runs the gate this PR hardens. Until tsk-ergobm decides whether the gate is wired in or removed, review spend on it is not justified. Source card closed in the same action; the hardening gets re-carded if and when the gate is wired.

@jaylfc jaylfc closed this Sep 6, 2026
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