Skip to content

[#1813 follow-up] Harden the comment-disposition gate: workflow-enforced provenance, ADR-0009, retire blanket @coderabbitai resolve, audit trail, no marker-less agent comments #1846

Open

Description

@don-petry

Summary

#1813 merged as PR #1816 on 2026-09-13T16:51Z with ACs 11–14 and 16 unimplemented. Those criteria were added to #1813's body after dev-lead's pass had already started, and #1816 was approved and auto-merged (auto-merge squash was armed) before they were picked up. The shipped gate works (ACs 1–10, 15 are live on main), but the trust boundary the Solution Architect escalated is still open.

Verified on origin/main today:

AC State on main Evidence
11 — workflow-enforced disposition provenance missing scripts/lib/comment-disposition-verify.sh contains no run_id/provenance check; prompts/dev-lead/fix-reviews.md:131 still tells the model to post its own reply with gh pr comment
12 — ADR-0009 recording the gate contract missing docs/architecture/adr/ ends at 0008-personas-post-as-the-owner-account.md
13 — retire blanket @coderabbitai resolve missing 4 occurrences remain in scripts/dev-lead-fix-reviews.sh
14 — audit trail for minimized comments missing only a ::notice:: count line
16 — no marker-less agent comments missing no harness-enforced marker; the defect was observed on #1816 itself

Why it matters

The merge gate now decides "addressed" from a disposition marker that the model writes and posts itself. For fixed the harness verifies the cited sha against the diff, so the risk is bounded. For invalid, answered, informational and out-of-scope there is nothing to verify against beyond "a non-empty reply exists" — so a model assertion, or anything else holding the owner token, can retire a real finding and let a PR merge. That is precisely the gap the Solution Architect escalated on #1813 ("confirm dev-lead's disposition-marker enforcement is workflow-side before this becomes a merge-gate trust boundary"), and it is the inverse of ADR-0008's model, where the persona runner's workflow enforces the marker and the agent cannot post at all.

The blanket @coderabbitai resolve (AC13) is the same failure in a second form: it retires every CodeRabbit thread wholesale with no per-finding verification. It was observed closing a half-applied finding on PR #1789 (#1727 AC4 parity).

Acceptance criteria

Carried over verbatim in intent from #1813 — see that issue's body for the full text of ACs 11, 12, 13, 14 and 16.

  1. AC11 — the model emits each disposition as structured output; the harness validates, verifies and posts it with the marker. The marker carries the dev-lead run_id, and the gate accepts a disposition only when that run exists, is a dev-lead-reusable.yml run on this repo, and completed. A marker posted by any other path does not clear the gate.
  2. AC12 — record ADR-0009 for the gate's "addressed" contract: verified disposition, harness-posted provenance and why marker-only trust is insufficient under ADR-0008, the human-comment rule, and the minimizeComment auditability trade-off.
  3. AC13 — remove notify_coderabbit_resolve and any equivalent "resolve all" instruction to a review bot. Resolution happens only through verified per-finding dispositions.
  4. AC14 — a minimized comment stays discoverable from a visible harness-posted reply that records the verification evidence, and the count of minimized-as-resolved findings is available to the reviewer report.
  5. AC16 — every comment the model or harness posts carries an agent marker, enforced by the harness rather than the prompt. A lint or bats check fails if any dev-lead prompt instructs posting a comment by another path.
  6. Regression bats for each: a disposition marker without valid run provenance does not clear the gate; a marker-less agent comment cannot be posted by the sanctioned path; @coderabbitai resolve no longer appears in the harness.

Activity

  1. github-actions commented on Sep 19, 2026

    @github-actions
    Contributor

    Thank you for this thorough follow-up and for clearly mapping each unimplemented AC to its current state on main — this is exactly the kind of evidence that makes triage actionable. Given the trust-boundary concern (non-fixed dispositions accepted on model-only authority), could you confirm whether the gap has been exercised in practice since #1816 merged — i.e., has any disposition marker been posted and accepted by the gate without a valid dev-lead-reusable.yml run backing it? That will help the team gauge urgency and whether an interim mitigation (e.g., temporarily blocking those disposition types) is needed before the full AC11/AC16 fix lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugBug reportsneeds-human-reviewFlagged by automated PR review agentsecuritySecurity-related PRs and issues

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions