Skip to content

chore(ci): move Argus to the AAO Secretariat App identity#2338

Merged
andybevan-scope3 merged 1 commit into
mainfrom
secretariat-identity
Jul 9, 2026
Merged

chore(ci): move Argus to the AAO Secretariat App identity#2338
andybevan-scope3 merged 1 commit into
mainfrom
secretariat-identity

Conversation

@bokelley

@bokelley bokelley commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

What changed

Migrates the Argus AI-review workflow from the IPR App identity to the org's new AAO Secretariat GitHub App, matching the spec repo's migration (adcontextprotocol/adcp#5832).

  • .github/workflows/ai-review.yml
    • App-token minting now uses secrets.SECRETARIAT_APP_ID / secrets.SECRETARIAT_APP_PRIVATE_KEY (was IPR_APP_ID / IPR_APP_PRIVATE_KEY).
    • ARGUS_BOT_LOGIN pinned to aao-secretariat[bot] (was aao-ipr-bot[bot]) — the skip-check and post-review verifier both key off this.
    • Header comments now describe Argus as the AAO Secretariat's review desk applying the WG constitution, not a reviewer in a maintainer's voice.
    • The prompt-building step appends the WG constitution after the prompt body under a ## WG constitution (from adcontextprotocol/adcp@main) heading, fetched at review time via curl -sf from the spec repo's main. Emitted only when the fetch returns content; the randomized heredoc sentinel discipline is unchanged.
    • The reviewer prompt was already loaded fail-closed from the PR's base SHA (git show "${BASE_SHA}:...") — unchanged.
  • .github/ai-review/expert-adcp-reviewer.md
    • Identity clause only: Argus is the review desk of the AAO Secretariat serving the AdCP Working Group; applies the appended WG constitution and cites decision records (DR-NNNN in the spec repo's governance/decisions/) when a question is settled precedent. Review logic (MUST FIX list, coverage rules, expert delegation) untouched.

Nothing else in the repo is modified — no other workflows, no changesets, no version files.

After merge

Argus reviews will post as aao-secretariat[bot] instead of aao-ipr-bot[bot].

⚠️ DO NOT MERGE yet

Do not merge until the org-level SECRETARIAT_APP_ID / SECRETARIAT_APP_PRIVATE_KEY secrets are visible to this repo. They are still being provisioned. If this merges before the secrets land, the Mint App token step fails and Argus reviews stop on every PR.

Security note

The WG constitution is fetched from adcontextprotocol/adcp@main at review time — it is not read from this repo's checkout or from the PR, so a PR cannot alter the rules it is reviewed under. If the fetch fails, the review runs without the constitution rather than failing.

🤖 Generated with Claude Code

@andybevan-scope3

Copy link
Copy Markdown
Collaborator

confirmed with Ben that this is now acceptable to be merged - will resolve issues then merge

Mint the review App token from the AAO Secretariat GitHub App
(SECRETARIAT_APP_ID / SECRETARIAT_APP_PRIVATE_KEY) instead of the IPR
App, pin ARGUS_BOT_LOGIN to aao-secretariat[bot], and describe Argus as
the Secretariat's review desk applying the WG constitution rather than
reviewing in a maintainer's voice.

The prompt-building step now appends the WG constitution fetched from
adcontextprotocol/adcp@main at review time; the reviewer prompt itself
still loads from the PR's base SHA, so neither is PR-controlled.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bokelley

bokelley commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Noted — standing by. Let me know if you need anything before the merge.


Generated by Claude Code

@bokelley

bokelley commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Acknowledged. PR is parked on the SECRETARIAT_APP_ID / SECRETARIAT_APP_PRIVATE_KEY secrets being provisioned — nothing to act on until those land.


Generated by Claude Code

@andybevan-scope3 andybevan-scope3 enabled auto-merge (squash) July 9, 2026 20:11
@andybevan-scope3 andybevan-scope3 merged commit ed98473 into main Jul 9, 2026
30 checks passed
@andybevan-scope3 andybevan-scope3 deleted the secretariat-identity branch July 9, 2026 20:24
andybevan-scope3 added a commit that referenced this pull request Jul 9, 2026
…; retire bash reviewer

Supersedes the bash Argus reviewer (incl. #2338's inline WG-constitution/App-identity
edits, now carried by the TS reviewer composite + orchestrator). Keeps
pull_request_target base-SHA checkout + head fetch + workflow-mod gate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
andybevan-scope3 added a commit that referenced this pull request Jul 13, 2026
…tion (#2354)

* feat(aao-secretariat): fork Scope3 review action tree (setup/reviewer/arbiter/review) into .github/aao-secretariat

Node/TS GitHub Action set forked from Scope3's Argus review action and adapted
for AdCP: validated findings-json contract, arbiter decides via a constrained
submit_decision enum, auth via the AAO Secretariat App, pull_request_target
head-read handling, and the WG constitution fetched from adcp@main at review time.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(aao-secretariat): add repo-root AAO-SECRETARIAT.md; fence action tree from published package

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(aao-secretariat): cut ai-review.yml over to the forked TS action; retire bash reviewer

Supersedes the bash Argus reviewer (incl. #2338's inline WG-constitution/App-identity
edits, now carried by the TS reviewer composite + orchestrator). Keeps
pull_request_target base-SHA checkout + head fetch + workflow-mod gate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(aao-secretariat): drop write-capable gh grant from reviewer allowlist

With CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=0 the installation token is in the Bash env;
the `gh api repos/*/issues/*` grant allowed a POST, so fork-PR prompt-injection
could exfiltrate the token via an issue comment. The reviewer posts inline comments
through the MCP tool, not Bash gh, so the grant is unnecessary. Removed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(aao-secretariat): add empty changeset (no-release; dev-tooling only)

Root-level files (AAO-SECRETARIAT.md, .prettierignore) map to the @adcp/sdk root
package (workspaces includes "."), so `changeset status` requires an entry. This
PR ships no library change, so an empty (no-release) changeset satisfies the gate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(aao-secretariat): source setup's diff from the GitHub API; drop pull_request_target head fetch

CodeQL flagged actions/untrusted-checkout (critical) on the `git fetch <head>`
step: fetching PR-head code in a privileged pull_request_target workflow. setup
now derives changed files (pulls.listFiles), the delta (compareCommits), and the
diff patches (diff media type) from the GitHub API — matching the pattern in
adcontextprotocol/adcp and this repo's prior reviewer — so the head is never
fetched, checked out, or executed. Removed the fetch step.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(aao-secretariat): drop redundant mergeable reassignment in setup

github-code-quality flagged the initial `let mergeable = true` as always
overwritten. The catch re-assigned the same default; removing it makes the
initial value the transient-failure default, resolving the finding.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(secretariat): move review action tree to .secretariat/ai-review

Relocates the forked review action tree from .github/aao-secretariat/ to a
root-level, dot-prefixed .secretariat/ai-review/ (Option C: the Secretariat's
ai-review desk). The dot prefix marks it as repo infrastructure (like .github)
and keeps it out of glob-based tooling; root placement matches the actions-repo
layout and simplifies the rebuild hook. Rewires uses:/paths-ignore/mod-gate/
codeql/prettierignore refs. Identity (AAO-SECRETARIAT bot, labels, AAO-SECRETARIAT.md,
input names) is unchanged — the desk vs argus naming call is still pending.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(secretariat): rewire workflow/codeql/prettierignore to .secretariat/ai-review

The tree-move commit missed these three reference files (staging error), leaving
the workflow uses:/paths-ignore/mod-gate pointing at the removed
.github/aao-secretariat path. Point them at .secretariat/ai-review.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(secretariat): add pre-commit dist-rebuild hook (self-contained; wired via install-hooks.js)

Keeps the setup/arbiter committed dist/ in sync with source: when a staged change
touches a node action's src/build-config, rebuild that action and re-stage its
dist/. Logic lives in .secretariat/ai-review/scripts/precommit.sh so it travels
with the tree (Phase-2 extraction); install-hooks.js installs a thin delegating
pre-commit that calls it. No husky, no core.hooksPath change — existing
commit-msg/pre-push hooks are untouched. Fast no-op for commits that don't touch
the tree.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(secretariat): fail closed on unreliable PR surface; drop write-capable gh grant

Review deep-dive fixes:
- #1/#2: computePrSurfaceFiles now returns {files, reliable}. reliable=false on a
  pulls.listFiles error OR the 3000-file cap; setup fails closed (throws) rather
  than clearing the high-risk/gated gates on an incomplete surface — a transient
  API blip or huge PR can no longer let a security-critical PR auto-approve.
- #3: removed the write-capable `gh api repos/*/contents/*` grant from the reviewer
  allowlist (gh api can't be method-restricted, and with ENV_SCRUB=0 the token is
  in the Bash env → prompt-injection exfil path). The agent now reads changes via
  the diff files + gh pr diff. Also removed # comment lines from inside claude_args
  (that block is passed verbatim to the CLI as args).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(secretariat): deep-review follow-ups (#5,#6,#8,#9,#10) + CI dist guard

- #5: writeDiffFile degrades to a placeholder (not a hard fail) on an oversized
  diff / HTTP 406, so large PRs still get a human-flagged review.
- #6: coverage.md largest-file rule reconciled to the base-SHA checkout — read
  changes from the diff files, cite head line numbers from the diff's + side.
- #8: install-hooks.js no longer clobbers a foreign pre-commit hook (warns +
  prints how to chain the delegator instead).
- #9: WG-constitution fetch gets --max-filesize + a fail-open warning; the
  prompt/findings GITHUB_OUTPUT heredocs use randomized sentinels (was fixed,
  a predictable-sentinel injection risk).
- #10: reviewer prompt no longer claims the delta diff is intersected/trivial-
  filtered (the API compare diff is not); wording now matches reality.
- CI dist guard: .secretariat/ai-review/scripts/check-dist.sh + a workflow that
  rebuilds each node action and fails if committed dist/ is stale — the real
  backstop for the bypassable pre-commit hook.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(ci): set least-privilege permissions on the secretariat dist-check workflow

CodeQL flagged the new workflow for not restricting GITHUB_TOKEN. The job only
checks out, builds, and diffs — add a top-level `permissions: contents: read`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* style(secretariat): prettier-format install-hooks.js

The #8 edit introduced a formatting drift that failed the format:check lane.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

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.

3 participants