Skip to content

feat(audit): portfolio audit toolchain + fix Copilot reusable pointer#2

Merged
arthur-debert merged 3 commits into
mainfrom
feat/audit-toolchain
May 8, 2026
Merged

feat(audit): portfolio audit toolchain + fix Copilot reusable pointer#2
arthur-debert merged 3 commits into
mainfrom
feat/audit-toolchain

Conversation

@arthur-debert

Copy link
Copy Markdown
Owner

Summary

Three new scripts that turn portfolio onboarding from whack-a-mole into a verifiable baseline:

  • `bin/audit-repo` — per-repo check across 8 surfaces:
    ruleset, RELEASE_TOKEN, copilot-review.yml (presence + correct pointer), CODEOWNERS, Dependabot security, dependabot.yml policy compliance, main-branch CI green, private go module auth.

  • `bin/audit-portfolio` — loops audit-repo over auto-discovered onboarded repos (same ruleset-discovery as `install-release-token`), prints a summary table + detail view of problem repos.

  • `bin/audit-smoke-test` — real end-to-end test. Opens a no-op PR on a target repo, verifies:

    1. Copilot Review workflow run is created within 90s
    2. Copilot is actually added as reviewer (timeline event)
    3. Other CI workflows trigger
      Then closes the PR. Proves the loop really works rather than should work.

Bonus: a portfolio-wide silent failure surfaced by the first smoke run

Smoke-running against `arthur-debert/dodot` (the canary) caught it: the canonical `copilot-review.yml` template pointed at `arthur-debert/gh-dagentic/.github/workflows/copilot-review.yml@main`, whose reusable workflow uses `GITHUB_TOKEN`. `GITHUB_TOKEN` authenticates as `github-actions[bot]` (Integration actor), and Integration actors silently no-op the Copilot review-attach — `gh pr edit --add-reviewer @copilot` returns the PR URL with no error but doesn't add anything.

The fix already exists at `arthur-debert/release@v1` (which requires the caller to pass a user PAT via `secrets.gh_token`). Only lex-fmt/lex had been migrated; 17 other onboarded repos were silently broken. Surveyed:

gh-dagentic (broken) release@v1 (fixed)
Repos 17 1 (lex-fmt/lex)

This PR repoints the canonical template (`templates/rust/workflows/copilot-review.yml`) at release/@v1 with the secret passthrough. The audit-repo `copilot_review` check now verifies the pointer too — gh-dagentic reports as a hard FAIL.

Other findings from the first portfolio audit run

24 onboarded repos audited, 17 failures + 14 warnings. Highlights for follow-up:

  • 7 repos missing RELEASE_TOKEN (treex, dotcat, nanodoc, falala, arami-core, lex-fmt/zed-lex, simple-gal-action) — `install-release-token` propagation isn't reliably running or completing.
  • 2 repos with broken main-branch CI: arthur-debert/homebrew-tools (`brew test-bot`), lex-fmt/comms (`Deploy to GitHub Pages`).
  • 3 repos with off-policy dependabot ecosystems: standout (cargo), clapfig (cargo), lex-fmt/lexed (npm).
  • 2 Go repos with arthur-debert/* deps but no `insteadOf`+RELEASE_TOKEN auth: dotcat, nanodoc.
  • simple-gal-action returned a workflow run with `name=null` — needs investigation.

Test plan

  • Tested audit-repo against 4 representative repos (canary, broken-multi, recent-onboard, all-green).
  • Tested audit-portfolio across 24 onboarded repos.
  • Tested audit-smoke-test against dodot end-to-end (caught the gh-dagentic bug).
  • CI green on this PR.

Follow-up (separate PRs)

  1. Sweep the 17 consumers to drop in the new copilot-review.yml template (mechanical, scriptable as `bin/migrate-copilot-review`).
  2. Address per-repo findings from the audit (RELEASE_TOKEN, broken CI, off-policy dependabot).
  3. Investigate why `install-release-token` isn't propagating despite reported successful runs.

🤖 Generated with Claude Code

Adds three scripts that turn portfolio onboarding from whack-a-mole
into a verifiable baseline:

- bin/audit-repo: per-repo check across 8 surfaces (ruleset,
  RELEASE_TOKEN, copilot-review.yml + correct pointer, CODEOWNERS,
  dependabot security, dependabot.yml policy compliance, main-branch
  CI green, private go module auth).

- bin/audit-portfolio: auto-discovers onboarded repos via the
  main-branch-protection ruleset (same logic install-release-token
  uses), runs audit-repo on each, prints a summary table + detail
  view for problem repos.

- bin/audit-smoke-test: real end-to-end test — opens a no-op PR on
  a target repo, verifies (1) Copilot Review workflow run is created
  within 90s, (2) Copilot is actually added as a reviewer
  (review_requested timeline event), (3) other CI workflows trigger.
  Closes the PR after. Proves the loop "really works" instead of
  "should work".

First smoke run on arthur-debert/dodot caught a portfolio-wide
silent failure: the canonical copilot-review.yml template was
pointing at arthur-debert/gh-dagentic@main, whose reusable workflow
uses GITHUB_TOKEN. GITHUB_TOKEN authenticates as
github-actions[bot] (Integration actor), and Integration actors
silently no-op the Copilot review-attach. So `gh pr edit
--add-reviewer @copilot` "succeeded" (no error, returned the PR URL)
but added nothing.

The fix already exists at arthur-debert/release@v1, which requires
the caller to pass a user PAT (RELEASE_TOKEN) via secrets. Only
lex-fmt/lex had been migrated; 17 other onboarded repos still
silently broken.

This PR repoints the canonical template to release/@v1 with the
secret passthrough. The audit-repo `copilot_review` check now
verifies the pointer too — gh-dagentic is reported as a hard
FAIL.

Follow-up: sweep the 17 broken consumers to drop in the new
template (one-line YAML change per repo).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a portfolio auditing toolchain to verify onboarding/CI policy across repos, and fixes the canonical Copilot Review workflow template to call the correct reusable workflow (arthur-debert/release@v1) with PAT passthrough so Copilot reviewer attachment actually works.

Changes:

  • Repoint templates/rust/workflows/copilot-review.yml to arthur-debert/release/.github/workflows/copilot-review.yml@v1 and pass secrets.gh_token: ${{ secrets.RELEASE_TOKEN }}.
  • Add bin/audit-repo to audit a single repo across ruleset/secrets/workflows/dependabot/CI/private Go module auth.
  • Add bin/audit-portfolio and bin/audit-smoke-test to run audits across onboarded repos and validate the end-to-end PR/Copilot loop.

Reviewed changes

Copilot reviewed 1 out of 4 changed files in this pull request and generated no comments.

File Description
templates/rust/workflows/copilot-review.yml Updates the reusable-workflow pointer and secret passthrough for Copilot review requests.
bin/audit-repo Adds a per-repo baseline audit across policy/config/CI surfaces.
bin/audit-portfolio Adds portfolio-wide orchestration over audit-repo with summary + details output.
bin/audit-smoke-test Adds an end-to-end smoke test that opens a PR and checks Copilot/CI signals before cleanup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

This was referenced May 8, 2026
arthur-debert and others added 2 commits May 8, 2026 17:38
…elease-token

bin/migrate-copilot-review:
  Auto-discovers consumers using the broken
  arthur-debert/gh-dagentic@main reusable workflow and opens
  per-repo PRs swapping it for the templates/rust/workflows/copilot-review.yml
  in this repo (which references release/@v1 + passes RELEASE_TOKEN).
  --dry-run lists candidates without PRing.

bin/install-release-token:
  - Surfaces gh CLI errors instead of swallowing them with 2>/dev/null
    (past versions reported "ok" for repos where the set never
    actually persisted — this caused a debugging cycle this session
    when 7 repos showed RELEASE_TOKEN missing despite a reportedly
    successful run).
  - Verifies by re-listing after each set; distinguishes "rc != 0"
    from "rc == 0 but secret absent on subsequent list".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
go.mod's 'module github.com/arthur-debert/X' line declares the repo's
own module name, not a dependency. The previous grep matched it,
producing false positives on dotcat/nanodoc.

Now: skip lines starting with 'module' before grepping for
arthur-debert/* dependencies.
@arthur-debert arthur-debert merged commit f6f2849 into main May 8, 2026
@arthur-debert arthur-debert deleted the feat/audit-toolchain branch May 8, 2026 20:54
arthur-debert added a commit that referenced this pull request May 15, 2026
Four threads, all accept-fix:

- Group B (Rollout-verify) now explicitly names its constituent phases:
  validation half of Phase 2 (per-repo SessionStart hooks) + active-use
  half of Phase 4a (consumer agents invoking release-issue-relay). The
  skill itself shipped in Group A; what Group B does is exercise it in
  production. (gemini #1, gemini #3)

- Group E gains a "Done when" line for symmetry with A/B/C/D — audit
  routine on cadence, convergence heuristics live for the safety hatch,
  rebase merge default across the portfolio. (gemini #2)

- §9.2 grammar fix: "the unify" → "the unify step" so the dependency
  bullet parses cleanly. (copilot)

No content reorganization, just clarifications and a grammar tweak.
arthur-debert added a commit that referenced this pull request May 15, 2026
…rlay (#20)

* docs(rollout): add Foundation → Verify → Workflows → Enhancements overlay

The original phased plan grouped work by deliverable but didn't make
explicit the "verify it holds together end-to-end" step between
shipping Foundation pieces and shipping Workflows on top of them.

Adds two pieces:

1. §1.1 — Product groups overlay. Five groups (A Foundation, B
   Rollout-verify, C CI workflows, D Release workflows, E Enhancements)
   on top of the existing phase numbering. Phases within a group are
   parallel-doable; groups are strictly sequential. The B → C
   transition is the load-bearing one: shipping CI workflows on top of
   an unverified foundation would compound defects into per-consumer
   rework, so Group B exists to surface them first.

2. §9.1-9.2 — Reframes sequencing notes around groups. The strict
   constraints within phases stay; the group-level ordering becomes
   the new "spine" of the rollout. §9.3 calls out the A→B boundary as
   the most valuable checkpoint pause point.

3. §9.4 — Updates the cloud-branch status: Phase 1 merged, branch is
   dormant, decision no longer urgent. Revisit if a real
   stability-channel need emerges later (e.g. Group C/D breaking
   changes worth gating).

Why now: Phase 1 (cloud → main unify + portable skills) and Phase 1.x
(env-level stack tools, PR #19) just landed. The next decision is
"Group B pilot pass" — explicit Rollout-verify across vscode + nvim +
electron + a couple others — before any Group C/D work starts. The
overlay makes that sequencing easy to follow without re-reading the
whole phase list.

No phase content changed; this is purely a structural / sequencing
overlay on top of what's already in the doc.

* fixup: address review feedback on rollout-doc overlay

Four threads, all accept-fix:

- Group B (Rollout-verify) now explicitly names its constituent phases:
  validation half of Phase 2 (per-repo SessionStart hooks) + active-use
  half of Phase 4a (consumer agents invoking release-issue-relay). The
  skill itself shipped in Group A; what Group B does is exercise it in
  production. (gemini #1, gemini #3)

- Group E gains a "Done when" line for symmetry with A/B/C/D — audit
  routine on cadence, convergence heuristics live for the safety hatch,
  rebase merge default across the portfolio. (gemini #2)

- §9.2 grammar fix: "the unify" → "the unify step" so the dependency
  bullet parses cleanly. (copilot)

No content reorganization, just clarifications and a grammar tweak.
arthur-debert added a commit that referenced this pull request May 18, 2026
Seven valid findings — three are gotchas from the cascade doc I
should have folded in but missed (#2 shell-injection, #7 manifest
drift, #10 UNRELEASED.md seed), plus a fourth real gap (stale
release branch on re-fire), plus three doc/log fixes.

.github/workflows/cascade-handler.yml:

- Copilot line 70 — shell-injection on `Announce trigger`. The step
  interpolated `${{ github.event.client_payload.* }}` directly into
  a `run:` body. Routed through `env:` so a compromised upstream
  can't inject command substitutions. (Gotcha #2 in the cascade
  doc — I had this on later steps but missed it on the announce.)

- Copilot line 134 — manifest-drift collision protection. The naive
  patch bump from `get-current-version` collides if the manifest
  has drifted behind real tags (tree-sitter-lex hit this exact
  case per Gotcha #7). Added max(manifest, latest GH release tag)
  via `gh release list` + `sort -V` before bumping.

- Copilot line 146 — re-fire safety on stale release branch. A
  previous handler that failed after pushing the branch but before
  admin-merge left a remote branch + open PR; a re-fired dispatch
  died at `git push` because the branch already existed. New step
  closes any stale PR (with --delete-branch) or deletes the stale
  remote branch, restoring the documented "failed pre-tag handlers
  can be re-fired cleanly" recovery contract.

- Copilot line 143 — UNRELEASED.md seed. nvim's `update-release`
  hard-fails on empty UNRELEASED.md; cascade runs have no
  human-authored notes. Added a new "Seed UNRELEASED.md if empty"
  step before `update-release` that writes
  `- Triggered by upstream release of <src>@<ver>.` (Gotcha #10.)

- Copilot line 190 — success log was tag-push-specific ("tag-push
  will fan into release.yml"). lex uses workflow_dispatch, not
  tag-push. Made the message generic.

docs/lex-release-cascade.md:

- Copilot line 224 — referred to "gotcha below" but the list is
  above in document order. Fixed wording.
- Gemini line 227 — recommended caller-shape section now lists the
  available inputs (bump-kind, git-author-name, git-author-email)
  and notes that run lookups happen under the caller's filename,
  not the reusable workflow's.
arthur-debert added a commit that referenced this pull request May 18, 2026
…des (Wave 1.3, closes #61) (#64)

* feat(cascade-handler): reusable workflow for cross-repo release cascades (Wave 1.3)

Closes #61. Generalises the copy-per-repo on-upstream-released.yml
shape used today in all six lex-fmt/* cascade repos. Consumers
onboard via a 6-line thin caller; new portfolios (arami,
simple-gal) adopt without re-deriving the gotcha list.

.github/workflows/cascade-handler.yml — reusable workflow:

- Triggered via `workflow_call` (caller registers
  repository_dispatch separately in its on-upstream-released.yml).
- Inputs: `bump-kind` (default patch), `git-author-name`,
  `git-author-email` (sensible portfolio defaults). Secrets:
  `RELEASE_TOKEN` (required — default GITHUB_TOKEN can't fire
  cross-repo dispatches and doesn't have admin scope for the
  release PR's admin-merge).
- All 10 cascade-doc gotchas folded in: GH_TOKEN on every
  primitive-running step, --admin flag for the release PR's
  branch-protection bypass, git reset --hard origin/main after
  admin-merge to handle pre-commit-side-effect repos like
  tree-sitter-lex (parser.c regen) and lexed (husky touches),
  submodule restore on the post-merge checkout, annotated tags
  honoured by the underlying primitive scripts.
- Logic mirrors the existing tree-sitter-lex handler verbatim
  except: parameterised inputs, gotchas documented inline as
  comments that point at docs/lex-release-cascade.md§Gotchas.

docs/lex-release-cascade.md — §"Adding a new repo to the cascade"
updated. The 6-line caller becomes the recommended shape; the
copy-per-repo shape is noted as "being phased out via Wave-3
migration sweep."

Caller shape (what each consumer's on-upstream-released.yml
becomes):

  name: On upstream released
  on:
    repository_dispatch:
      types: [upstream-released]
  jobs:
    cascade:
      uses: arthur-debert/release/.github/workflows/cascade-handler.yml@v1
      secrets:
        RELEASE_TOKEN: ${{ secrets.RELEASE_TOKEN }}

First-consumer migration (lex-fmt/tree-sitter-lex) ships as a
follow-up PR after this lands.

* fixup: address Copilot + Gemini review feedback on #64

Seven valid findings — three are gotchas from the cascade doc I
should have folded in but missed (#2 shell-injection, #7 manifest
drift, #10 UNRELEASED.md seed), plus a fourth real gap (stale
release branch on re-fire), plus three doc/log fixes.

.github/workflows/cascade-handler.yml:

- Copilot line 70 — shell-injection on `Announce trigger`. The step
  interpolated `${{ github.event.client_payload.* }}` directly into
  a `run:` body. Routed through `env:` so a compromised upstream
  can't inject command substitutions. (Gotcha #2 in the cascade
  doc — I had this on later steps but missed it on the announce.)

- Copilot line 134 — manifest-drift collision protection. The naive
  patch bump from `get-current-version` collides if the manifest
  has drifted behind real tags (tree-sitter-lex hit this exact
  case per Gotcha #7). Added max(manifest, latest GH release tag)
  via `gh release list` + `sort -V` before bumping.

- Copilot line 146 — re-fire safety on stale release branch. A
  previous handler that failed after pushing the branch but before
  admin-merge left a remote branch + open PR; a re-fired dispatch
  died at `git push` because the branch already existed. New step
  closes any stale PR (with --delete-branch) or deletes the stale
  remote branch, restoring the documented "failed pre-tag handlers
  can be re-fired cleanly" recovery contract.

- Copilot line 143 — UNRELEASED.md seed. nvim's `update-release`
  hard-fails on empty UNRELEASED.md; cascade runs have no
  human-authored notes. Added a new "Seed UNRELEASED.md if empty"
  step before `update-release` that writes
  `- Triggered by upstream release of <src>@<ver>.` (Gotcha #10.)

- Copilot line 190 — success log was tag-push-specific ("tag-push
  will fan into release.yml"). lex uses workflow_dispatch, not
  tag-push. Made the message generic.

docs/lex-release-cascade.md:

- Copilot line 224 — referred to "gotcha below" but the list is
  above in document order. Fixed wording.
- Gemini line 227 — recommended caller-shape section now lists the
  available inputs (bump-kind, git-author-name, git-author-email)
  and notes that run lookups happen under the caller's filename,
  not the reusable workflow's.
arthur-debert added a commit that referenced this pull request May 30, 2026
Both reviewers independently flagged the fork-clone bug; one fix covers it.

- _spawn_fixer: resolve the clone URL from the PR's headRepository.url
  (the base URL is wrong for fork PRs), in a single `gh pr view` call
  (drops the redundant `gh repo view`). Fail fast on isCrossRepository
  (fork) PRs — auto-fix can't push to a fork we don't own. Catch
  JSONDecodeError/TypeError (null headRepository) too, and surface
  stderr/stdout in errors. (Gemini #1/#3, Copilot)
- poll_once: wrap get_status in try/except + Sink.error — a transient
  gh/network/rate-limit failure on one PR logs and skips, the daemon
  keeps polling the rest instead of crashing. (Gemini #2)
- watch `prs` argparse: type=int (clean validation, no late traceback);
  simplify the now-redundant int() conversion. (Gemini #4/#5)
- _desktop: escape quotes in the notification title too, not just body.
  (Gemini #6)

Tests: +1 (daemon survives a per-PR status error). 67 total.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
arthur-debert added a commit that referenced this pull request May 30, 2026
* Phase 5: orc watch — detached poll-loop transport (#338)

The event/transport layer, settled as a poll-loop (not webhooks): a
long-running local process imports release_gh.state and polls a few PRs
at ~zero agent-token cost, dispatching only on a lifecycle *transition*.

- orchestrator/watch.py: pure decide() (state -> Action) + poll_once()
  with side effects behind an injected Sink, so the dispatch/dedup logic
  unit-tests with no gh, no network, no SDK. The SDK is lazy-imported,
  only when an auto-fix agent actually spawns.
- orc watch <pr>... [--auto] [--repo] [--interval]: notify-only by
  default (page the human at the moments that matter); --auto spawns a
  FRESH fixer agent on ADDRESSING/BLOCKED. Two gates are never automated:
  merge (READY flips draft->ready + pages) and a fired breaker (always
  pages, never acts).
- Auto-fix = fresh agent + /handoff, not session-resume (#338 rationale):
  runs bypassPermissions in a throwaway clone of the PR branch (blast
  radius bounded like probe), pushes fixups, never merges. Skill gains a
  "leave a /handoff note when you open the PR" step so the fresh fixer
  has the original reasoning.

Workspace: orchestrator now depends on release-gh (uv workspace source);
folded into the ruff gate (CI py-lint + lefthook) now that it's clean;
added to pytest pythonpath.

Tests: +10 (decide per state, poll dedup/transition, multi-PR, bounded
loop, fixer-prompt contract). 66 total. uv sync resolves; orc runs.

Part of #332. Closes #338.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Address review on #347: fork-safe auto-fix + daemon resilience

Both reviewers independently flagged the fork-clone bug; one fix covers it.

- _spawn_fixer: resolve the clone URL from the PR's headRepository.url
  (the base URL is wrong for fork PRs), in a single `gh pr view` call
  (drops the redundant `gh repo view`). Fail fast on isCrossRepository
  (fork) PRs — auto-fix can't push to a fork we don't own. Catch
  JSONDecodeError/TypeError (null headRepository) too, and surface
  stderr/stdout in errors. (Gemini #1/#3, Copilot)
- poll_once: wrap get_status in try/except + Sink.error — a transient
  gh/network/rate-limit failure on one PR logs and skips, the daemon
  keeps polling the rest instead of crashing. (Gemini #2)
- watch `prs` argparse: type=int (clean validation, no late traceback);
  simplify the now-redundant int() conversion. (Gemini #4/#5)
- _desktop: escape quotes in the notification title too, not just body.
  (Gemini #6)

Tests: +1 (daemon survives a per-PR status error). 67 total.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
arthur-debert added a commit that referenced this pull request Jun 12, 2026
…already done

Owner review: #569 item 1 was written before the bin→app-bin / WS7 decisions.
The reusable workflows already removed the `bin/build` fallback (#588/#590);
post-WS7 `bin/build` is an UNTRACKED ephemeral mirror, so the os.path.isfile
check read RED for every consumer forever — measuring a model the codebase left
behind. App build moved to bespoke `app-bin/*`, which has no sound mechanical
predicate (a future `release-core run` supersedes it), so no replacement row.

- Remove _p_task_verbs, TASK_VERB_KINDS, and the Condition("task-verbs", "1", …)
  row. CONDITIONS now measures 5: pull-seeded #2, resolver-vintage #3, ws4-ws7
  #4, hooks-path #5, fragment-model.
- KEEP the kind field + _detect_kind as a display-only roster label (mirrors the
  verify sweep); only the kind-gating is gone. NA constant retained for a future
  kind-gated row (renderer + removable() still handle it).
- Tests: drop the predicate-1 cases, assert the predicate/constant are gone,
  adjust the table to 5 rows. #569 body marks item 1 ALREADY COMPLETE; docs +
  changelog reflect five conditions.

Co-Authored-By: Claude Fable 5 <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