Skip to content

ci(argus): post via a deterministic arbiter; review inline#2349

Merged
andybevan-scope3 merged 2 commits into
mainfrom
abevan/argus-arbiter-split
Jul 9, 2026
Merged

ci(argus): post via a deterministic arbiter; review inline#2349
andybevan-scope3 merged 2 commits into
mainfrom
abevan/argus-arbiter-split

Conversation

@andybevan-scope3

Copy link
Copy Markdown
Collaborator

Why

The Argus reviewer stalls on source-code PRs and never posts an approval — they sit at REVIEW_REQUIRED while a fallback drops a non-approval COMMENT. Diagnosis (three runs on #2345 vs a working run on #2333 and Brian's #2346):

  • Argus is a single agent that must both run its Task sub-reviewers and call gh pr review itself.
  • On source PRs it dispatches the subagents, then ends its turn early ("waiting for the experts to report") before posting. Posting is the last thing it does, so nothing lands.
  • Config/docs PRs skip subagents (the prompt's skip-everything list), so they post fine — which is why maintainer PRs like Migrate conductor.json to .conductor/settings.toml #2346 approve and source PRs don't. It's not an author-permission issue (the PR author has write; the actor gate passes).

What Changed

Split the flow into reviewer → arbiter, mirroring how Argus v2 works (posting decoupled from the reviewer):

  • Reviewer writes its verdict (approve/comment/request-changes) and review body to two files (ARGUS_VERDICT, ARGUS_REVIEW_BODY); it no longer calls gh pr review.
  • Post review (arbiter) — a new deterministic bash step reads those files and posts the review. Degrades safely in tiers: valid verdict+body → post it; body only → COMMENT; agent final text → COMMENT; nothing → a plain note. Never fabricates an approval. Replaces the old verify + fallback + failure-comment steps.
  • Inline review, no Task. Removed the mandatory subagent dispatch and dropped Task + gh pr review from the tool allowlist. The reviewer reads the diff and files itself. This removes the stall trigger and the permission denials; fix: strip pricing_currencies from get_products for AdCP 3.0 sellers #2333 shows the subagents weren't adding value (it approved despite 13 denials, i.e. they were failing). Domain-scrutiny guidance is preserved inline.

Notes / testing

  • This PR is intentionally not auto-reviewed by Argus — it touches .github/ai-review/** + the workflow, which both paths-ignore and the workflow-mod gate exclude. Needs a human review/merge (no chicken-and-egg).
  • pull_request_target runs the reviewer/prompt from the base SHA, so the new behavior only takes effect after this merges to main. It can't be exercised on an open PR beforehand. Validated pre-merge via npm run lint:workflows, YAML parse, and logic review; the live proof is the next Argus run on a source-code PR after merge.

🤖 Generated with Claude Code

andybevan-scope3 and others added 2 commits July 9, 2026 10:37
The Argus reviewer was a single agent that had to both run its Task
subagents and call `gh pr review` itself. On source-code PRs it dispatched
the subagents, then ended its turn ("waiting for the experts") before
posting — so no verdict ever landed and the PR stalled at REVIEW_REQUIRED
(config/docs PRs skip subagents and were unaffected).

Split the flow into reviewer -> arbiter, mirroring Argus v2:

- The reviewer writes its verdict (approve|comment|request-changes) and
  review body to two files; it no longer calls `gh pr review`.
- A deterministic "Post review (arbiter)" step reads those files and posts
  the review, degrading safely to a COMMENT (never a fabricated approval)
  when no verdict was recorded. Replaces the verify + fallback + failure trio.
- The reviewer now reviews inline; the mandatory Task subagent dispatch and
  the `Task` / `gh pr review` tools are removed (subagent dispatch stalled the
  run in this harness and produced the permission denials; it added no value).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@andybevan-scope3 andybevan-scope3 merged commit 68914b7 into main Jul 9, 2026
30 checks passed
@andybevan-scope3 andybevan-scope3 deleted the abevan/argus-arbiter-split branch July 9, 2026 14:50
LukasGoTom pushed a commit to gotom-io/adcp-client that referenced this pull request Jul 10, 2026
…ine security scrutiny

Follow-up to adcontextprotocol#2349 (arbiter split). Addresses four review findings where a
recorded `request-changes` could silently degrade to a non-blocking COMMENT:

1. Verdict/body files moved from ${{ runner.temp }} to /tmp — the path proven
   writable by the reviewer's Write tool in this harness. Added a probe that
   warns when neither file was written. (4 paths + POST_FILE for consistency.)
2. Reworded the two surviving "Consult security-reviewer" prompt references to
   inline security-grade scrutiny — the Task subagent tool was removed in adcontextprotocol#2349.
3. Tolerant verdict parsing: match request-changes synonyms first (fail closed),
   bounded read via head -c 200 instead of exact bare-token match.
4. Valid verdict + empty body now posts under the verdict with a placeholder
   body instead of dropping to a COMMENT; tier-2 preamble distinguishes
   "no verdict recorded" from "verdict recorded but post failed".

.github/ only (dev tooling) — no changeset needed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

2 participants