Skip to content

ci: install Ladon PR reviewer, replacing the Argus workflow - #983

Open
andybevan-scope3 wants to merge 1 commit into
mainfrom
install-ladon
Open

ci: install Ladon PR reviewer, replacing the Argus workflow#983
andybevan-scope3 wants to merge 1 commit into
mainfrom
install-ladon

Conversation

@andybevan-scope3

Copy link
Copy Markdown
Collaborator

Why

Ladon is the shared AdCP PR reviewer, maintained in adcontextprotocol/actions and consumed by floating major tag. This repo carried its own reviewer instead — a claude-code-action invocation driving a 245-line vendored prompt at .github/ai-review/expert-adcp-reviewer.md — so every engine fix had to be re-ported here by hand. Installed per ladon/INSTALL.md, following the "replacing an existing reviewer" path.

What changed

  • .github/workflows/ai-review.yml — replaced with the canonical consumer workflow from INSTALL.md, copied verbatim. Pins adcontextprotocol/actions/ladon/review@ladon/review/v1 (tag resolves to 2b4b3aa), keeps model: claude-opus-4-8, and preserves the pull_request_target posture: base-SHA-only checkout, PR head never fetched or executed, every github.event.* value reaching a shell via env:. Adds the workflow-modification gate the old workflow lacked.
  • Deleted .github/ai-review/expert-adcp-reviewer.md — voice, severity model, largest-file rule, test-plan honesty, operational-readiness audit, and inline-comment format now come from the baseline rules in the actions repo (ladon/reviewer/rules/*.md). Nothing else referenced this file.
  • Added LADON.md — the repo-specific half of the retired prompt, restated as parsed config. ## Repo Context carries the mandatory rules that are only true here: semver signal on the adcp.* surface (release-please cuts a minor from feat:, so a breaking diff under it ships without a major), discriminated-union forward compat, the types import-layering allowlist, generated-code-is-not-source including the codegen renumbering-churn rule from CLAUDE.md, the ctx_metadata credential prohibition, and CI-gate regressions. Plus ## High-Risk Paths, ## Trivial Paths, ## Escalation Reviewers, and ## Release Stack Branches.

Job name code_review is unchanged, so the required-status-check context is preserved.

Secrets

The reviewer moves from IPR_APP_ID / IPR_APP_PRIVATE_KEY to SECRETARIAT_APP_ID / SECRETARIAT_APP_PRIVATE_KEY. All of SECRETARIAT_APP_ID, SECRETARIAT_APP_PRIVATE_KEY, and ANTHROPIC_API_KEY already resolve to this repo as org-level secrets (verified via repos/.../actions/organization-secrets) — no new secrets to add.

One prerequisite I could not verify: whether the AAO Secretariat GitHub App is installed on this repo (reading that requires org-admin scope). Per INSTALL.md, secrets existing is not the same as the App being installed — if the token-mint step fails on the first run without a secret-not-found error, that is the cause.

Notes for the reviewer

Two deltas where verbatim-copy conflicts with what this repo was already doing. INSTALL.md says "copy it verbatim: do not reconstruct it," so I did, but both are worth a decision:

  1. Action pins go down. The template uses actions/checkout@v5 and actions/create-github-app-token@v1; the Argus workflow used @v6 and @v3. Same as adcp-client's live workflow.
  2. persist-credentials: false is gone from the checkout step — the Argus workflow set it, the template does not. Low impact (nothing pushes, and PR-head code is never executed), but it was real hardening. Happy to add it back if you'd rather keep it.

Also: ladon/review's app-id input is marked deprecated in favour of client-id, but INSTALL.md's template still uses app-id and the action still accepts it. Left as-is to stay on the documented template.

No ## Gated Paths section — this repo has no CODEOWNERS (removed in #968), and per AUTHORING.md a gated path without a matching CODEOWNERS + branch-protection rule becomes a permanently un-liftable block.

I dropped a ## Skip Bot Authors section I had initially written: the action's own default is dependabot[bot],renovate[bot],github-actions[bot], and because LADON.md overrides rather than extends that default, listing only dependabot[bot] would have narrowed coverage. ## Release Stack Branches handles the release-please PRs (branch release-please--branches--main--components--adcp, authored by app/aao-ipr-bot) deterministically instead.

Test plan

  • Workflow YAML parses; single job code_review; trigger, paths-ignore, permissions, and all five steps verified.
  • LADON.md verified against the real parser (ladon/setup/src/ladon-md.ts) — all six H2 headings resolve, 17 high-risk globs / 3 trivial globs / 1 reviewer / 1 release branch parse, ## Repo Context captures all 7 H3 rule blocks.
  • ladon/review/v1 tag exists; passed inputs match the action's declared inputs at that tag.
  • SECRETARIAT_APP_ID / SECRETARIAT_APP_PRIVATE_KEY / ANTHROPIC_API_KEY confirmed available to this repo.
  • No remaining references to .github/ai-review/ anywhere in the tree.
  • PR title passes scripts/check-pr-title.cjs.
  • End-to-end run — not exercisable from this PR. pull_request_target registers from the default branch and runs from the base SHA, and the workflow-mod gate makes Ladon decline to self-review. This PR needs a human approver; Ladon starts on the next PR. After merge, confirm registration with:
    gh api "repos/adcontextprotocol/adcp-client-python/actions/workflows" --jq '.workflows[] | "\(.path) \(.state)"'
    
    .github/workflows/ai-review.yml must be listed and active.

🤖 Generated with Claude Code

Ladon is the shared AdCP PR reviewer, maintained in
adcontextprotocol/actions (ladon/) and consumed by floating major tag.
This repo carried its own reviewer — a claude-code-action invocation
driving a 245-line vendored prompt — so every engine fix had to be
re-ported by hand.

- Replace .github/workflows/ai-review.yml with the canonical consumer
  workflow from ladon/INSTALL.md, copied verbatim. It pins
  adcontextprotocol/actions/ladon/review@ladon/review/v1 and keeps the
  pull_request_target posture: base-SHA-only checkout, PR head never
  fetched or executed, every github.event.* value reaching a shell via
  env:. Job name code_review is unchanged, so the status-check context
  is preserved.
- Delete .github/ai-review/expert-adcp-reviewer.md. Voice, severity
  model, largest-file rule, test-plan honesty, and inline-comment format
  now come from the baseline rules in the actions repo.
- Add LADON.md carrying the repo-specific half of the retired prompt as
  parsed config: semver signal on the adcp.* surface, discriminated-union
  forward compat, types import layering, generated-code-is-not-source
  (including the codegen renumbering-churn rule from CLAUDE.md),
  ctx_metadata credential prohibition, CI-gate regressions, and the
  high-risk / trivial path globs.

Secrets move from IPR_APP_ID / IPR_APP_PRIVATE_KEY to
SECRETARIAT_APP_ID / SECRETARIAT_APP_PRIVATE_KEY; both pairs plus
ANTHROPIC_API_KEY already resolve to this repo as org-level secrets.

Co-Authored-By: Claude <noreply@anthropic.com>
@aao-ipr-bot

aao-ipr-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

This is an automated message from the Argus AI review workflow.

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