fix(noema): stop LLM prose from spoofing the body-side head SHA match

[seonghobae]

Bug

Devin's automated review on PR #1415 flagged: "Model text invalidates valid reviews — When a model summary or finding repeats the head line, NOEMA_BODY_HEAD_RE counts it alongside the receipt. The handoff rejects the valid review and times out." (scripts/ci/noema_review_handoff.py:109-111)

PR #1415 only inherited this code via a merge from main; the root cause is pre-existing on main itself, introduced by the dual head-SHA binding #1480/#1483 added earlier.

noema_review_state() validated a Noema review body with an unanchored regex searching the entire body:

NOEMA_BODY_HEAD_RE = re.compile(r"Head SHA:\s*`([0-9a-fA-F]{40})`")
body_heads = NOEMA_BODY_HEAD_RE.findall(body)
if len(marker_heads) != 1 or len(body_heads) != 1:
    continue

submit_review() in noema_review_gate.py builds the body as: the LLM's own generated summary/findings text (and now, after #1497 landed, its adversarial-validation evidence too), then a fixed-format footer (- Result:, - Head SHA:, - Reviewer credential:, - Actor:), then the HTML marker comment. Because that LLM text is not sanitized against this exact shape, a response that happened to contain the phrase Head SHA: <40 hex chars>`` anywhere — plausible whenever Noema reviews a PR touching this exact mechanism — produced 2+ matches, len(body_heads) != 1 became true, and a perfectly valid, correctly-authored Noema verdict was silently discarded, eventually timing out the handoff poll loop.

Live risk

Self-referential: any PR whose diff, commit messages, or (per #1497) Noema's own adversarial-probe evidence discusses this exact head-SHA-matching mechanism is at elevated risk of a correct Noema verdict being silently discarded.

Fix — three iterations

First iteration (commit 2fe2fa8b): anchored NOEMA_BODY_HEAD_RE to a whole line via re.MULTILINE and restricted the search to the text before the trusted end marker. This closed the "phrase anywhere in prose" case but Devin's follow-up review of this PR correctly found it incomplete: a complete, correctly-formatted standalone `- Head SHA: ``` line in the LLM's own text still matched, because the extraction was still content-pattern-based (scanning for any line matching the shape), just with a narrower pattern.

Second iteration (commit e99defa7): replaced the content-pattern approach with a positional one, per Devin's suggested direction:

• submit_review() now emits a second, purpose-specific HTML marker, NOEMA_REVIEW_FOOTER_MARKER = "<!-- noema-review-gate-footer -->", wrapping the start of its fixed-format footer — a marker only submit_review() itself ever emits, the same trust basis the existing closing <!-- noema-review-gate head_sha=... --> comment already relies on.
• noema_review_handoff.py adds _isolate_trusted_footer(), which extracts the span strictly between that new footer-start marker and the closing marker — both machine-emitted, never reachable by the LLM's own generated text — and only searches that span for the - Head SHA: bullet (the whole-line re.MULTILINE anchor is kept as defense-in-depth inside that now-trusted span).

Because the isolated span can never contain LLM-authored content (it sits strictly between two markers only the trusted script emits), no amount of LLM text elsewhere — mid-sentence, a complete standalone bullet, a different SHA or the same one — can ever count toward the body-side match.

Third iteration (commit 0430f7f8): Devin's review of the second iteration correctly found the positional fix was applied asymmetrically — only to the body-side bullet, not to the marker-side check. noema_review_state() still ran NOEMA_MARKER_HEAD_RE.findall(body) against the entire unsanitized body, so an LLM summary/finding that happens to contain a complete, correctly-formatted <!-- noema-review-gate head_sha=... decision=... -->-shaped string of its own (the same self-referential scenario as before) could still produce 2 matches and wrongly reject a valid verdict via the len(marker_heads) != 1 guard.

submit_review()'s "\n".join([...]) guarantees the real closing marker is unconditionally the last element of the review body — nothing follows it — so everything from NOEMA_REVIEW_FOOTER_MARKER to the end of the body is, like the already-isolated footer span, exclusively machine-emitted text. Added a sibling helper, _isolate_trusted_marker_tail(), returning body.rsplit(NOEMA_REVIEW_FOOTER_MARKER, 1)[1] (or "" when the footer marker is absent, failing closed the same way _isolate_trusted_footer() does), and noema_review_state() now runs NOEMA_MARKER_HEAD_RE against that trusted tail instead of the raw body — the same position-anchoring mechanism, applied to the marker side.

This preserves the dual-binding property #1480/#1483 introduced: the marker comment and the human-readable bullet must still independently agree with the live head, unchanged from before.

Regression tests

• test_noema_state_accepts_valid_review_despite_incidental_body_text — a mid-sentence phrase resembling the footer (different SHA, and the same SHA) no longer causes rejection.
• test_noema_state_ignores_standalone_body_head_bullet_before_footer — a complete, correctly-formatted standalone `- Head SHA: ``` bullet line placed in the LLM's own text before the real footer (parametrized: a different SHA and the same SHA) no longer causes rejection.
• test_noema_state_ignores_standalone_closing_marker_before_footer (new, third iteration, addressing Devin's marker-side finding directly) — a complete, correctly-formatted standalone <!-- noema-review-gate head_sha=... decision=... --> string placed in the LLM's own text before the real footer (parametrized: a different SHA and the current head's own SHA) no longer causes rejection, while the real footer and closing marker remain valid and the review is still correctly recognized as APPROVED.
• test_noema_state_rejects_missing_stale_or_duplicate_head_bindings — exercises missing/wrong-value/duplicated bindings from inside the trusted footer span and trusted marker tail, proving the dual-binding security property fix: verify Noema GitHub App identity #1480/fix: bind Noema App slug to review identity #1483 added is unweakened (including the genuinely-duplicated-marker case, which stays rejected since both real markers still fall inside the trusted tail).
• Two other hand-rolled Noema review body fixtures (test_repository_branch_coverage_review_schedulers.py) updated to include the new footer marker so they keep exercising the intended happy path.

Verification

• coverage run -m pytest tests && coverage report --show-missing → 2132 passed, 1 skipped (up from 2130 after the third iteration's new tests), 100% coverage on scripts/ci/.
• interrogate → 100% docstrings.
• git diff --check → clean.

Scope

Central infra only (scripts/ci/noema_review_gate.py, scripts/ci/noema_review_handoff.py, and their tests). Does not touch fix/noema-batched-preflight-413-evidence (PR #1415) or any other in-flight branch. Rebased cleanly onto main twice as it moved (#1497 landed mid-review; submit_review()'s new adversarial-evidence section required no changes to this fix's insertion point). Not self-merging — leaving for review per the org's OpenCode/Noema governance model.

[coderabbitai]

Warning

Review limit reached

Next included review available in 43 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 65567845-ded0-458f-a8e6-66d5c5fd68c6

📥 Commits

Reviewing files that changed from the base of the PR and between 1cbb6aa and cc233cd.

📒 Files selected for processing (6)

• CHANGELOG.md
• scripts/ci/noema_review_gate.py
• scripts/ci/noema_review_handoff.py
• scripts/ci/test_strix_quick_gate.sh
• tests/test_noema_review_handoff.py
• tests/test_repository_branch_coverage_review_schedulers.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

• X
• Mastodon
• Reddit
• LinkedIn

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

[seonghobae]

Ported a fix for the exact-head-path-policy/Strix quick-gate required check, which was failing on this and every other open .github-repo PR due to a bug in test_strix_quick_gate.sh's own job-scoping logic (not in opencode-review.yml, which has always been correct). Root cause and fix are in #1506 (a concurrent session's more complete PR, which also documents this in docs/product-technical-gap-baseline.md) — I opened an equivalent fix independently as #1505, now closing that in favor of #1506. Since this check runs on plain pull_request: against each PR's own head branch (not pull_request_target: against main), merging #1506 alone won't fix already-open PRs — the same patch needs porting into each one, which is what this push does (cc233cdd).

---

Generated by Claude Code

[devin-ai-integration]

Devin Review found 1 new potential issue.

[seonghobae]

opencode-review failed on this head (cc233cdd), but this is not this PR's fault: it's the same base-branch-wide race documented in #1485 and already fixed (pending merge) in #1494 — the opencode-review required check evaluates synchronously seconds after push, while the actual opencode-agent review is posted much later (sometimes over an hour) by the separate, async opencode-review-dispatch.yml path, and nothing currently re-triggers the already-failed check once that review lands. This PR's own diff (scripts/ci/noema_review_gate.py, scripts/ci/noema_review_handoff.py) doesn't touch anything in that dispatch/verdict-polling chain.

#1494 is currently blocked by its own merge conflict against current main; I'm resolving that now since it's the single highest-leverage fix available (it should self-resolve this same spurious failure across #1500, #1415, and the other affected PRs #1494 already lists once merged). Once it lands, this check should clear on its next natural re-evaluation. Not re-running the check myself right now since re-running before that fix lands would just reproduce the identical race.

---

Generated by Claude Code