feat(distribution): add sync-managed claude review-lane caller components

[kyle-sexton]

Summary

Adds two components — claude-review-caller and
claude-security-review-caller — sourcing thin workflow callers for the
ci-workflows reusable Claude review lanes at
.github/workflows/claude-review.yml and
.github/workflows/claude-security-review.yml, pinned at the v0.9.1
release SHA (c136b27f404dd32ce3873f39a6f3443891d1c16e). This is Phase 3a of
the ci-workflows claude-review-lanes plan
(docs/topics/claude-review-lanes/PLAN.md).

Sections below marked "REVISED" supersede the original text. Four
commits landed after this body was first written; where they conflict, the
REVISED text is authoritative.

REVISED — pin is v0.9.1, not v0.9.0

v0.9.0 (cf666f67) was tagged BEFORE paths-file merged. The security caller
passes paths-file, which v0.9.0's reusable does not declare — Actions
hard-fails a workflow_call that passes an undeclared input, and on the
security lane that wedges a required check permanently. Verified by reading
each reusable's workflow_call block at the pinned SHA (git show <sha>:<path>, not main, not the tag) and diffing the caller's with: /
secrets: keys against it in both directions, plus every
needs.<job>.outputs.<name> read against the reusable's declared outputs.
Result at v0.9.1: 0 undeclared inputs, 0 undeclared secrets, 0 omitted
required inputs, 0 undeclared output reads, across all four uses: including
both select-runner.yml calls. Negative control at v0.9.0 reproduces the
defect.

REVISED — these components are PRIVATE-ONLY; the security caller is PARKED

The callers resolve the runner through the governed select-runner
indirection, and runner-policy admits that selector only for a private
self-hosted consumer (routingEnabled = visibility === "private" && selfHostedCi). The !routingEnabled branch consults neither exceptions nor
localRoutingGrants, so a PUBLIC target has no configuration escape.

Auditing each component against a public consumer config using the shipped
policy.json yields four public-self-hosted-routing findings; the same
audit passes clean for a private self-hosted consumer. Consequences:

• claude-review-caller is managed for the four PRIVATE targets that run
  the lane: dotfiles, github-iac, medley, provisioning.
• melodic-software/claude-code-plugins is PUBLIC and is now locally-owned
  for both callers, keeping its hand-written hosted-only callers. Had the
  original targeting shipped, its own runner-policy lane — and with it
  ci-status — would have gone red, wedging the one repo whose ruleset
  requires security-review / security-review.
• claude-security-review-caller therefore has no managed target and is
  recorded as PARKED, not scoped: both repos running a security lane today
  (claude-code-plugins, ci-workflows) are public. It is retained rather than
  deleted because its bytes are the reviewed shape for the one lane whose
  check can be a required context. It unparks when a private repo adopts the
  security lane, or when the runner indirection moves inside the ci-workflows
  reusable so one component serves both visibilities.

Three tests in components/runner-policy/runner-policy.test.mjs hold this and
were each proven non-vacuous by reintroducing the defect: a selector-routed
caller may not be managed for a public target; every caller component must
audit clean for a private self-hosted consumer; a selector-routed caller is
expected to be rejected outright on a public one.

REVISED — accepted loss: synchronize

All four managed targets' live callers carry synchronize; the component
drops it. That is the plan's deliberate cadence cut (review on
open/ready/reopen; re-run the job for a fresh review), not drift — but it is
the largest behavior change the component makes and is now recorded as an
accepted loss for every target, alongside medley's paths-ignore. The
security lane KEEPS synchronize: its check certifies execution against the
latest head.

REVISED — validation coverage gap closed

components/claude-lanes/ holds workflow bytes, but every workflow-shaped
lane in this repo discovers files under .github/workflows — so actionlint,
zizmor, concurrency-policy, and pin-comment-convention were all blind to
these components, and lint:runner-policy scans the repo's own workflows,
not components/. Nothing validated them.

Three lanes now cover them in CI, each proven non-vacuous:

• pin-comment-convention takes an explicit file list and scans
  components/claude-lanes/*.yml (a mismatched SHA claim fails it).

• zizmor names both components alongside .; its paths input is
  whitespace-separated. Verified that a bare . never reached them, that they
  are audited now, and that the finding total, severity breakdown, and exit
  code are identical to baseline — so what the lane gates on is unchanged.

• actionlint, through a new materialization contract test
  (components/claude-lanes/claude-lanes.test.sh, wired into the actionlint
  job). It runs the entrypoint a consumer runs rather than linting the
  component in place: sync-manifest.sh apply into a scratch checkout carrying
  the target's origin identity, then actionlint over the result. Every managed
  target is covered, plus each component's bytes standalone at their
  destination path so the PARKED security caller — which no target manages, so
  no target loop reaches it — is linted too. A control run with the suppression
  config removed still reports the concurrency.queue message, so the case
  cannot go vacuous unnoticed when concurrency: Add support for queue key rhysd/actionlint#654 ships upstream.

  Scope of that coverage, stated precisely: the test is hermetic and makes
  no network call, so it lints each target's materialized bytes under THIS
  repository's canonical actionlint config — not under the target's own. For
  github-iac and medley, which own actionlint locally and so receive no
  config from the manifest, the canonical config always substitutes. If either
  deleted its own queue suppression, this test would still pass. What the test
  owns is that the shipped bytes lint clean under a conforming config at the
  destination path the manifest maps. That the four live configs actually
  conform is gate item 1 below — a live check, re-verified independently, and
  it is what covers this gap.

Still blind, deliberately: concurrency-policy fails both components on three
rules whose values are load-bearing (lane-scoped group names; the security
caller's deliberate cancel-in-progress: false). No target repo runs
concurrency-policy, so nothing fails today; the conflict is documented at the
component sources so a future adopter exempts rather than normalizes.

Caller shape derives from the reusables' canonical-caller headers (the SSOT):

• Job ids review / security-review — required-check name continuity
  (security-review / security-review on claude-code-plugins' ruleset).
• Review triggers [opened, ready_for_review, reopened]; security triggers
  add synchronize; no workflow-level path filtering on the security caller
  (a non-triggering required check wedges forever).
• Permissions per the canonical headers (contents: read,
  pull-requests: write, id-token: write on the lane job).
• Governed select-runner indirection (selector job, needs, runner input)
  with the runner-policy recovery fallback 'ubuntu-24.04' — never a
  self-hosted label, never vars.CI_HOSTED_RUNNER.
• Security caller passes paths-file: .github/claude-security-paths
  (shipped in ci-workflows#282).
• CLAUDE_CODE_OAUTH_TOKEN passed explicitly; never secrets: inherit.
• Targets are the repos that run each lane today — SUPERSEDED by the
  private-only revision above; the shipped targeting is claude-review-caller
  in dotfiles, github-iac, medley, provisioning, with claude-code-plugins
  locally-owned for both callers and claude-security-review-caller
  parked. .github and ci-runner stay exempt (plan approval record item 1);
  knowledge-corpus and songwriting are commented follow-ups gated on the sync
  App access grant.

distribution/README.md's three workflow-caller-exclusion statements gain
carve-outs pointing at a new authoritative section: hand-written lane
callers empirically drifted (medley's missing reopened trigger, divergent
skip-actors lists, pin skew v0.6.1 / e295107). AGENTS.md and
governance-process.md were grepped for restatements of the exclusion — none
exist. Cross-doc reconciliation self-review performed per
distribution/governance-process.md: the three README statements plus the
new section are the complete reconciliation surface, and no other normative
doc was left contradicting the change.

DO NOT MERGE — gate status (REVISED)

1. standards#284 (Phase 3c0 actionlint suppression) — SATISFIED. Merged
   2026-07-27T14:24Z. Every consumer lints its own workflows with pinned
   actionlint 1.7.12, which rejects the review caller's concurrency.queue
   key (concurrency: Add support for queue key rhysd/actionlint#654); without the distributed suppression, each
   caller sync PR would fail its consumer's required ci-status check.
   Verified live: all four managed targets' .github/actionlint.yaml
   suppress the message for claude-review.yml (dotfiles and provisioning by
   glob, github-iac and medley by explicit path).
2. standards-sync App workflows: write (Phase 3a0) — SATISFIED. Verified
   live: gh api orgs/melodic-software/installations reports installation
   144867070 (melodic-standards-sync) holding
   {contents: write, issues: write, metadata: read, pull_requests: write, workflows: write}. Writing .github/workflows/ files in target repos needs
   that permission; without it every sync PR from this component would fail.
3. ci(distribution): disarm fleet automerge for the Phase 3d rollout window #290 (automerge: false rollout window) — SATISFIED. Merged
   2026-07-29T13:05Z and merged into this branch, resolving the one conflict at
   the claude-code-plugins target.
4. ci: re-pin the standards-sync engine at ac223bb #289 (re-pin the sync engine at ac223bb) — OPEN. ci(distribution): disarm fleet automerge for the Phase 3d rollout window #290's stated merge
   order is ci(distribution): disarm fleet automerge for the Phase 3d rollout window #290 → ci: re-pin the standards-sync engine at ac223bb #289 → feat(distribution): add sync-managed claude review-lane caller components #286. The App grant above is inert until the engine
   re-pin lands, because main still pins ci-workflows@0b45b9f, which
   predates the permission-workflows: write mint.

Beyond that ordering, the only failing check on this PR is
do-not-merge / do-not-merge — the intentional label gate, which the label
owner lifts.

Concurrency decision record

Shipped the 2e-documented shape from the reusables' headers: review caller =
workflow-level per-PR cancel group plus a separate job-level queue: max
repo-wide group; security caller = per-PR group with
cancel-in-progress: false and no queue. github-iac's live caller
deliberately omits caller-level concurrency, claiming (i) caller-level
cancel reintroduces skipped-actor cancellation and (ii) a group-name
collision with the reusable's job group. Inspection at v0.9.0:

• Collision claim: disproven. The inner job group is
  claude-review-<PR>-<headSHA> (claude-review.yml:261 at v0.9.0); the
  caller groups are claude-review-<PR> (cancel) and
  claude-review-<owner/repo> (queue). No name equality, so the historical
  caller/inner deadlock — real when the inner group was
  claude-review-<pr-number> exactly; provisioning's caller comment
  documents the observed "deadlock was detected" error from that era —
  cannot recur. github-iac's and provisioning's comments describe a
  pre-v0.9.0 inner-group shape.
• Skipped-actor cancellation: real but bounded, accepted. The caller
  workflow-level group does evaluate before any job if, so a skip-actor
  event on the same PR cancels an in-flight review. With no synchronize
  trigger the same-PR event surface is opened / ready_for_review /
  reopened — rare and human-driven. The reusable's 2e header documents
  this exact caller value as canonical and per-lane deliberate.

The component sources record both rationales inline; the 3c smoke exercises
this exact shape.

Paths-file seeding disposition

The manifest has no seed-once mechanism (schema v2: components are
unconditional source-to-dest maps), and .github/claude-security-paths is
repo-owned tuning that must NOT become managed bytes. Disposition:

• Recorded the gap in the manifest comment and README section: a new
  adopter commits its starter list via a repo-local PR at adoption time.
• claude-code-plugins migration ordering: MOOT. That repo is now
  locally-owned for both callers and receives no sync PR, so no migration
  ordering applies. (Its .github/claude-security-paths already exists —
  733 bytes, live — and its hand-written caller keeps its inline paths:
  list.)
• Proposed mechanism if seeding is wanted later: a seed file class in
  schema v3 — materialized only when absent at the target, never
  reconciled — which preserves repo ownership after first sync.

Deviations / notes for the 3c smoke

• skip-actors is not passed by the callers: the reusable's default at the
  pinned c136b27 is already the normalized self-trigger-ban list
  (dependabot[bot],claude[bot],melodic-ai[bot],melodic-standards-sync[bot],
  claude-review.yml:150); existing callers passed it only because the
  pre-v0.9.0 default was narrower. Ownership therefore moves from the caller
  to the reusable, and
  conventions/review/ai-review-bot-composition.md is reconciled to say so —
  it previously attributed the melodic-standards-sync[bot] exclusion to the
  caller's skip-actors input, wiring this component no longer has.
• Runner fallback normalized to 'ubuntu-24.04' (dotfiles and github-iac
  currently use melodic-ubuntu-24.04-x64, which violates the runner-policy
  recovery contract in distribution/README.md and would queue forever on a
  public repo).
• Selector-failure surfacing (class=runner marker) is a comment, not a
  TODO(#issue): no dedicated issue exists; the incident-aggregator
  acceptance test (ci-workflows#238) exercises it in Phase 4.
• The 3c smoke must confirm the required-check shape when the security
  caller's selector job fails or skips (a caller-job skip is a new state for
  the security-review / security-review required context on
  claude-code-plugins; on that public repo the selector routes hosted-only,
  so the exposure is infra-failure only).
• medley's paths-ignore tuning is an accepted loss (plan approval record
  item 10), noted at its manifest target entry.

Test plan

REVISED — as run at a4d7ee6:

• npm run test:runner-policy — 242/242 pass, including four new gates (each
  proven non-vacuous by reintroducing its defect in a scratch copy): a
  selector-routed caller may not be managed for a public target; every
  caller component must audit clean for a private self-hosted consumer; a
  selector-routed caller is rejected outright on a public one; and every
  managed target of any caller must be one runner-policy admits (written as a
  property, so legitimate unparking via a private consumer passes).
• npm run lint:runner-policy, npm run lint:md,
  bash distribution/sync-manifest.sh validate
  (Manifest valid: 34 components, 8 targets) all pass.
• Scripted input-conformance proof, both directions, all four uses: read at
  the pinned SHA: 0 failures, 0 warnings. Negative control at v0.9.0
  reproduces the paths-file defect.
• Governance simulation with the shipped policy.json: private self-hosted
  consumer passes clean; public consumer produces four
  public-self-hosted-routing findings. Negative control against the
  pre-fa2e7a5 policy produces eight findings, proving the new policy
  entries load-bearing.
• Selector byte-identity claim confirmed by object hash: c136b27 and
  e77f0126 both resolve select-runner.yml to blob 6ba7d60c.
• zizmor 1.26.1 over both components: no findings; totals identical to
  baseline. Extended pin-comment-convention scan: exit 0.
• End-to-end regression test against each managed target's REAL live state —
  its .github/runner-policy.json and its full live workflow set fetched via
  the API, then the synced caller dropped in: all four (dotfiles, github-iac,
  medley, provisioning) report Runner policy passed. both before and after,
  so the caller introduces no finding at any target.
• CI on the tip: 42 of 43 checks SUCCESS, including ci-status,
  Runner policy, distribution, actionlint, zizmor,
  concurrency-policy, and pin-comment-convention. The sole failure is
  do-not-merge / do-not-merge, which is the intentional label gate.

REVISED — after merging main (#290) and closing the five review threads:

• Conflict with ci(distribution): disarm fleet automerge for the Phase 3d rollout window #290 resolved at the claude-code-plugins target, the one
  place both parents insert after - typos. Purely additive over each parent:
  git diff origin/main -- distribution/sync-manifest.yml | grep '^-' and the
  same against af85ae0 each emit only the --- header, zero deletion lines,
  so ci(distribution): disarm fleet automerge for the Phase 3d rollout window #290's comment above targets: and every component list survive.
• yq: 8 of 8 targets carry automerge as a !!bool false; the count of
  targets whose value is not boolean false is 0. sync-manifest.sh matrix
  emits it as a JSON boolean for all 8 (a quoted string would be truthy).
• Per-target managed / locally-owned lists byte-identical to af85ae0
  (95 entries); components block unchanged.
• bash distribution/sync-manifest.sh validate →
  Manifest valid: 34 components, 8 targets, exit 0.
• bash distribution/sync-manifest.test.sh → 147 passed, 0 failed.
• npm run test:runner-policy → 242/242 against the merged manifest;
  npm run lint:runner-policy → Runner policy passed.
• bash harness/shell/run-tests.sh components/claude-lanes/claude-lanes.test.sh components/actionlint/actionlint.test.sh → 2 files, 2 passed, 0 failed.
• The lane-caller suite was mutation-tested rather than trusted for being
  green. Two independent ways it could have shrunk silently — the managed-list
  query returning nothing, and the per-component membership test ceasing to
  match — were each reintroduced in throwaway copies outside the worktree and
  both now surface as FAIL with a non-zero exit, where the second previously
  dropped four assertions and still exited 0. Per-target assertion counts are
  asserted, not assumed.
• shellcheck (--rcfile .shellcheckrc -x), actionlint 1.7.12 over the repo,
  zizmor 1.26.1 over the repo plus both components (No findings to report),
  markdownlint, typos, and editorconfig-checker all clean on the changed set.

Original pre-revision plan:

• bash distribution/sync-manifest.sh validate reported
  Manifest valid: 33 components, 8 targets.
• yq eval -o=json distribution/sync-manifest.yml | node distribution/validate-sync-manifest.mjs
  passes.
• sync-manifest.sh plan over all five affected targets shows exactly the
  intended additions (claude-review-caller in all five; the security caller
  in claude-code-plugins only; automerge unchanged).
• actionlint 1.7.12 over both callers materialized at their destination
  layout WITH ci-workflows' approved queue-suppression config exits 0; the
  negative control without the config reports exactly the one expected
  concurrency.queue syntax-check finding on the review caller.
• markdownlint plus lefthook pre-commit gates (typos, editorconfig,
  gitleaks, markdownlint) green on commit.

Related

• Phase 3a of melodic-software/ci-workflows
docs/topics/claude-review-lanes/PLAN.md (approval record items 1, 9, 10).
• Gate 1: feat(distribution): add actionlint queue-suppression component for no-config consumers #284 (3c0 actionlint suppression).
• Gate 2: Phase 3a0 sync-App workflows: write grant (org-owner action,
  lands via github-iac).
• Canonical caller headers: ci-workflows claude-review.yml /
  claude-security-review.yml at v0.9.1
  (c136b27f404dd32ce3873f39a6f3443891d1c16e).
• ci-workflows#282 (paths-file input), ci-workflows#278 (empty selector
  output — motivates the hosted fallback), ci-workflows#238 (Phase 4
  acceptance test covering selector-failure surfacing).

No linked issue.

🤖 Generated with Claude Code

[claude]

Claude finished @kyle-sexton's task in 3m 25s —— View job

---

Code Review

• Load tools
• Read REVIEW.md and CLAUDE.md for review criteria
• Read changed files
• Analyze components and manifest
• Post review findings

---

Review: feat(distribution): add sync-managed claude review-lane caller components

The PR is well-structured and the reasoning is thoroughly documented. The concurrency decisions, permission grants, selector-failure guards, and exception justification are all sound. A few items worth resolving before merge:

---

🔴 Important — Workflow name: vs. ruleset required-check context

components/claude-lanes/claude-security-review.yml#L1

The component file declares name: claude-security-review, which means the GitHub check context it produces is claude-security-review / security-review. The PR body states the required-check on claude-code-plugins' ruleset is security-review / security-review, which implies the current live caller uses name: security-review as the workflow display name.

If the sync PR replaces that file with name: claude-security-review, the check context changes and the existing ruleset requirement (security-review / security-review) will never be satisfied — every PR on claude-code-plugins will be permanently wedged until the ruleset context is updated. This would be a Gate 3 that isn't called out in the "DO NOT MERGE" block.

Please verify: does the live caller at claude-code-plugins currently have name: security-review or name: claude-security-review? If it's the former, the ruleset must be updated atomically with the sync PR landing, and that coordination should be an explicit gate in the PR body.

The same question applies to the review caller — if existing targets have name: claude-review already, this is a non-issue; if they have name: review, the same wedge risk exists.

---

🟡 Nit — Migration ordering for paths-file seeding not surfaced in the README section

distribution/README.md#L248-L256

The new "Claude review-lane caller components" section correctly documents that an absent .github/claude-security-paths file fails open. However, it says "a new adopter commits its starter list in a repo-local PR alongside (or before) its caller materialization PR" — it doesn't surface the specific migration-ordering requirement for the existing claude-code-plugins adopter, whose inline paths: list must be committed to .github/claude-security-paths before (not just alongside) the caller sync PR merges. That ordering matters because the synced caller drops the inline list entirely: until the file exists, "fails open" isn't just a note, it's a silent loss of the tuned relevance gate.

The PR body covers this well, but the README section (which survives as the long-lived consumer-facing doc) doesn't make the ordering constraint as explicit. A one-sentence addition like "Existing callers that pass an inline paths: list must commit it to .github/claude-security-paths before the caller sync PR merges, not alongside — the synced caller passes only paths-file, and until the file exists the tuning is gone" would close that gap for future operators.

---

🟡 Nit — select-review / select-security job-name symmetry with name: Select runner

components/claude-lanes/claude-review.yml#L53, components/claude-lanes/claude-security-review.yml#L68

Both selector jobs carry name: Select runner. Since these live in separate workflows, there's no naming collision. Just confirming this is intentional — a reader landing on one file in isolation might wonder why the name doesn't reflect which lane it's selecting for. No change needed if this matches the canonical-caller headers in the reusable; noting it for awareness.

---

Confirmed correct — concurrency shape, permission ceiling, and selector-guard pattern

• The workflow-level permissions: contents: read is correctly narrowed; both review and security-review jobs override it with explicit grants. Job-level permissions in GitHub Actions supersede the workflow-level default, so pull-requests: write and id-token: write are available to the called reusable.
• if: ${{ !cancelled() && needs.<selector>.result == 'success' }} on both review jobs is the correct guard against skip propagation running a privileged job when the selector fails with no runner output.
• cancel-in-progress: false on the security caller is the correct defensive choice for a required execution-evidence check.
• The queue: max on the code-review job-level concurrency is correctly separated into its own block (GitHub doesn't allow cancel-in-progress and queue in the same block).
• No workflow-level paths: on the security caller — correct; a required check that never triggers stays Pending and wedges PRs.
• secrets: CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} (no secrets: inherit) — correct least-privilege forwarding.
• Three README carve-outs plus the new section cover the full cross-doc reconciliation surface; the cross-references are consistent.

[claude]

Claude finished @kyle-sexton's task in 3m 5s —— View job

---

Code Review

• Load tools
• Read REVIEW.md and project guidelines
• Read changed files
• Analyze components and manifest
• Post review findings

---

Review: feat(distribution): add sync-managed claude review-lane caller components

The PR is well-structured with thorough inline rationale. Most of the mechanics are correct and well-reasoned. One open question from the prior review remains unresolved and needs an answer before merge.

---

🔴 Important — Workflow name: vs. required-check context (unresolved from prior review)

components/claude-lanes/claude-security-review.yml#L1

The component declares name: claude-security-review. GitHub's check context is <workflow name> / <job id>, so the check this file produces is claude-security-review / security-review.

The PR body states the required-check on claude-code-plugins' ruleset is security-review / security-review — which would require the workflow name: to be security-review, not claude-security-review.

If the live caller at claude-code-plugins currently has name: security-review, replacing it with this component silently changes the check context and the ruleset required-check is never satisfied. Every PR on claude-code-plugins would be permanently wedged on the old context until the ruleset is updated. This is Gate 3 that isn't documented.

Two possible resolutions:

1. The live caller already has name: claude-security-review (or the ruleset already matches that context) — in which case the PR body's parenthetical is a documentation error, not a code error, and just needs a correction.
2. The live caller has name: security-review — in which case the component name needs to match, or the ruleset update must be a coordinated explicit gate.

Please confirm which state the live caller is in. The same question applies to the review caller (name: claude-review → context claude-review / review; if existing targets had name: review, same wedge risk).

---

🟡 Nit — README "alongside (or before)" understates the ordering constraint for existing adopters

distribution/README.md#L252

The new section reads: "a new adopter commits its starter list in a repo-local PR alongside (or before) its caller materialization PR." For a genuinely new adopter, "alongside" is fine — absent file fails open, meaning over-review, not under-review.

But for an existing adopter (specifically claude-code-plugins, which has a 26-entry inline paths: list in its current caller), the constraint is strictly "before," not "alongside." The synced caller drops the inline paths: entirely and only passes paths-file. Until the file exists, the tuned relevance gate is silently gone — a different operational outcome than the over-review of a new adopter.

The section doesn't distinguish the two cases. A one-sentence note for existing adopters would close the gap for future operators of this migration: "An existing adopter whose current caller passes an inline paths: list must commit it to .github/claude-security-paths before the caller sync PR merges — the synced caller replaces the inline list entirely, and until the file exists the tuning is lost."

Fix this →

---

Confirmed correct — no changes needed

The following were independently verified on this pass:

• Permission ceiling: Workflow-level permissions: contents: read correctly constrains the default; both review and security-review job blocks explicitly override with pull-requests: write and id-token: write. Job-level permissions supersede the workflow default in GitHub Actions. ✓
• Selector guard: if: ${{ !cancelled() && needs.<selector>.result == 'success' }} on both review jobs correctly prevents the fallback runner path from executing a privileged job when the selector hard-fails with no output. ✓
• cancel-in-progress: false on security caller: Correct. A cancelled required-check run does not satisfy the ruleset context — it must complete. ✓
• Separate concurrency blocks on review caller: queue: max in its own block is required; GitHub does not allow cancel-in-progress and queue in the same concurrency block. ✓
• No workflow-level paths: on security caller: Correct per the inline comment. A required check whose workflow never triggers stays Pending and wedges every out-of-scope PR. ✓
• secrets: CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}: Least-privilege forwarding — correct. secrets: inherit is explicitly avoided. ✓
• SHA pin + comment: Both callers pin at cf666f672e0ec81cf876303840a3af1ffd4cb966 # v0.9.0, which follows the pin-comment convention. ✓
• permissions: {} on selector jobs: Correct; the selector needs no GITHUB_TOKEN scope. ✓
• Fallback runner 'ubuntu-24.04' (not vars.CI_HOSTED_RUNNER): Correct per runner-policy recovery contract; vars.CI_HOSTED_RUNNER could be a self-hosted label and would queue forever on a public repo during infra failure. ✓
• Manifest target assignments: claude-review-caller in five targets (claude-code-plugins, dotfiles, github-iac, medley, provisioning); claude-security-review-caller in claude-code-plugins only. Matches PR body. ✓
• Component count: 33 components across 8 targets matches PR body test-plan validation output. ✓
• Three README carve-outs + new section: Cross-doc reconciliation surface is complete; no other normative doc contradicts the change. ✓

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 029aea03f2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 6m 2s —— View job

---

Code Review

• Load tools
• Read REVIEW.md and project guidelines
• Read changed files (PR diff against origin/main)
• Verify prior open findings + check for new issues from merge
• Post review findings

---

Review: feat(distribution): add sync-managed claude review-lane caller components

This pass reviews the synchronize event (the merge of origin/main into the branch). The feature content is unchanged since the prior two reviews. Two prior findings remain open; the merge from main resolves Gate 1 and surfaces a new blocking issue.

---

Gate 1 status: RESOLVED

The merge of origin/main brought in .github/actionlint.yaml with the approved concurrency.queue suppression (and the corresponding components/actionlint/fixtures/good/queue-concurrency.yml positive-control fixture). Gate 1 is cleared — actionlint 1.7.12 running at consumers will accept the queue: max key in the synced review caller once the suppression component is also materialized.

Gate 2 (sync App workflows: write grant) remains pending — org-owner action, no code change here.

---

🔴 Important — v0.9.0 SHA not registered in runner-policy

components/runner-policy/policy.json

Both callers pin all three reusable references at cf666f672e0ec81cf876303840a3af1ffd4cb966 (v0.9.0). None of those three are registered in policy.json:

• select-runner.yml@cf666f... is absent from both approvedSelectorReferences and approvedSelectorReferencesByRepositoryOwner.
• claude-review.yml@cf666f... is absent from approvedReusableWorkflowContracts.
• claude-security-review.yml@cf666f... is absent from approvedReusableWorkflowContracts.

The runner-policy gate (runner-policy.mjs) runs on every consumer's materialization PR. Without the approved entries, the gate will reject both callers and every caller sync PR across all five targets will fail its required ci-status check before it can merge — a fleet-wide rollout wedge that isn't listed in the "DO NOT MERGE" block.

The v0.9.0 contracts also need to reflect the changed input shapes:

• claude-review.yml@cf666f...: runner input only (no skip-actors, consistent with the existing runner-input routing entries at df54d0e... and 4dbb0df...).
• claude-security-review.yml@cf666f...: runner and paths-file inputs (v0.9.0 adds paths-file; the existing entries at e295107... and 66073e... list paths, which is the pre-v0.9.0 shape).

This is a code gap in the PR — components/runner-policy/policy.json needs new entries for all three v0.9.0 references before or alongside the component manifest additions.

Fix this →

---

🔴 Important — Workflow name: vs. required-check context (third pass, still unresolved)

components/claude-lanes/claude-security-review.yml#L1

Unchanged since the prior two reviews. The component file declares name: claude-security-review, which means GitHub's check context is claude-security-review / security-review. The PR body states the required-check on claude-code-plugins' ruleset is security-review / security-review — which would require name: security-review.

If the live caller at claude-code-plugins has name: security-review, replacing it with this component silently changes the check context and the existing required-check context is never satisfied. This is Gate 3 that isn't in the "DO NOT MERGE" block.

Two pass resolutions, same as before: (1) the live caller and/or ruleset already use claude-security-review — a documentation error, not a code error; (2) the live caller uses security-review — the component name or the ruleset must be updated as a coordinated explicit gate. Please confirm which state the live caller is in.

The same question applies to the review caller (name: claude-review → context claude-review / review).

---

🟡 Nit — README ordering constraint for existing adopters still understated

distribution/README.md#L254

Unchanged since the prior two reviews. The section reads "a new adopter commits its starter list in a repo-local PR alongside (or before) its caller materialization PR." For claude-code-plugins (an existing adopter with a 26-entry inline paths: list), "alongside" is wrong — the synced caller drops the inline paths: entirely and only passes paths-file, so until the file exists the tuned relevance gate is gone. The constraint for existing adopters is strictly "before."

Fix this →

---

🟡 Nit — ai-review-bot-composition.md skip-actors statement is stale

conventions/review/ai-review-bot-composition.md#L9

The governed-class description states the melodic-standards-sync[bot] exclusion is wired "via the caller's skip-actors input." The new managed callers deliberately omit skip-actors — the v0.9.0 reusable's default already includes the normalized self-trigger-ban list, so the PR body notes the input was only passed before because the pre-v0.9.0 default was narrower. The file is accurate about the outcome (the exclusion still holds), but incorrect about how it's implemented after this change. This is the kind of implementation-wiring detail that drifts if left unreferenced. A phrase like "via the reusable's built-in default or the caller's skip-actors input" or just removing the implementation detail from the normative statement would keep the doc accurate post-merge.

Fix this →

---

Summary of open items

#	Severity	Finding	Status
1	🔴	v0.9.0 SHA not in runner-policy/policy.json (3 entries needed)	New this pass
2	🔴	Workflow name: vs required-check context — needs confirmation	Open 3rd pass
3	🟡	README ordering "alongside (or before)" understates existing-adopter constraint	Open 3rd pass
4	🟡	ai-review-bot-composition.md skip-actors wiring statement is stale	New this pass

Gate 1 (actionlint suppression) resolved by the merge from main. Gate 2 (App workflows: write) still pending external action.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3e10a4062f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 1s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 1s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude finished @kyle-sexton's task in 5m 6s —— View job

---

Code Review (Pass 4)

• Load tools
• Read REVIEW.md and project guidelines
• Read changed files at HEAD (c674496)
• Verify prior open findings are addressed
• Check new commits for new issues
• Post review findings

---

Review: feat(distribution): add sync-managed claude review-lane caller components

Four commits landed since Pass 3. All four prior open findings are resolved. No new blocking issues.

---

Prior findings — all resolved

#	Prior severity	Finding	Resolution
1	🔴	v0.9.0 SHA missing from runner-policy/policy.json	Resolved. Callers re-pinned to v0.9.1 (c136b27f). All three v0.9.1 entries are present in policy.json: select-runner.yml@c136b27f in approvedSelectorReferencesByRepositoryOwner, claude-review.yml@c136b27f with allowedInputs: ["runner"], claude-security-review.yml@c136b27f with allowedInputs: ["runner", "paths-file"]. Inputs match what the callers pass.
2	🔴	Workflow name: vs required-check context	Resolved by scope change. claude-code-plugins (the only repo with a security-review / security-review ruleset requirement) is now locally-owned for both callers and keeps its hand-written caller. The security caller is parked with no managed target. No sync PR will touch claude-code-plugins.
3	🟡	README "alongside (or before)" understates existing-adopter constraint	Resolved by scope change. No existing adopter receives a sync PR — the constraint was specific to claude-code-plugins, which is now locally-owned. "Alongside (or before)" is correct for a new adopter where an absent file fails open (over-review, not under-review).
4	🟡	ai-review-bot-composition.md skip-actors wiring statement stale	Resolved by 1ec21e3. The file now correctly attributes the exclusion to "the reusable lane's own skip-actors default, which the sync-managed caller inherits rather than overriding."

---

New commits — verified correct

af85ae0 test(runner-policy): fail closed on unrecorded target visibility

Adds TARGET_VISIBILITY map and four new tests at runner-policy.test.mjs#L7941.

• The fail-closed design is correct: isPublicTarget() calls assert.ok(visibility, "... add it to TARGET_VISIBILITY") — a target added to the manifest without a corresponding TARGET_VISIBILITY entry fails the test immediately with a diagnostic, rather than silently passing. ✓
• Tests call auditRepository directly (not the audit() fixture wrapper), so they exercise the real policy.json rather than the synthetic fixture policy. ✓
• consumerCarrying pushes into temporaryRoots for cleanup — consistent with the existing cleanup pattern. ✓
• Property-based assertion on "every managed target admits its caller" means a future legitimate unpark (adding a private target) passes automatically, while adding a public one fails — exactly the right invariant. ✓
• PATHS_FILE_LANE_SHA is added to the "production selector allowlist contains only independently reviewed commits" allowlist, which verifies the v0.9.1 SHA is in the approved selector set. ✓

1ec21e3 docs(review): attribute the sync-bot exclusion to the reusable lane — Correct. See finding #4 above.

eee0d9c Merge branch 'main' into feat/claude-lane-caller-components — The merge brings in the Gate 1 content (actionlint suppression from #284). No conflicts or regressions from the merge.

c674496 test(claude-lanes): lint the caller components at the materialized shape

Adds claude-lanes.test.sh at components/claude-lanes/claude-lanes.test.sh. Several properties verified:

• Derives lanes from manifest, not hardcoded — the test reads component/target information from sync-manifest.yml dynamically, so a new lane caller or new managed target is covered without editing the test. The comment explicitly notes that the security caller has no managed target today and a fixed count would require edits. ✓
• Lints parked bytes too — the first loop (lines 64–75) lints every lane caller at its destination path regardless of whether any target manages it today. This ensures the parked security caller's bytes remain lintable even while it has no managed target. ✓
• Materializes via the real sync-manifest.sh apply engine — the second loop materializes each managed target through the distribution engine, then runs actionlint over the result. The consumer stand-in correctly sets a remote to bypass apply's origin check. ✓
• Control case is load-bearing — removing the config on one consumer and asserting the specific queue: max message appears (lines 107–114) proves the suppression is genuinely needed. ✓
• CI wiring — the actionlint job gains the yq install step (needed by the distribution engine) and the Test claude lane caller components step; zizmor now names both component files explicitly alongside .; pin-comment-convention scan is extended to components/claude-lanes/*.yml. The PR body's claim that the finding total and exit code are identical to baseline is consistent with the callers having no zizmor findings. ✓

---

Confirmed correct — unchanged from prior passes

• SHA pin + comment format at v0.9.1 on all three uses: lines (select-runner.yml, claude-review.yml, claude-security-review.yml). ✓
• permissions: {} on both selector jobs. ✓
• if: ${{ !cancelled() && needs.<selector>.result == 'success' }} guard on both review jobs. ✓
• Separate concurrency blocks on the review caller (cancel-in-progress and queue: in separate blocks). ✓
• cancel-in-progress: false on the security caller. ✓
• No workflow-level paths: on the security caller. ✓
• secrets: CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} (no secrets: inherit). ✓
• Fallback runner 'ubuntu-24.04' (quoted, never vars.CI_HOSTED_RUNNER). ✓
• Gate 1 cleared — af85ae0 and the merge bring the actionlint suppression in.
• Gate 2 (sync App workflows: write grant) still pending external org-owner action.

[claude]

Claude finished @kyle-sexton's task in 2m 39s —— View job

---

Code Review (Pass 5)

• Load tools
• Read REVIEW.md and project guidelines
• Read the new commit (d0ef74b) and changed files
• Verify pass 4 findings still hold
• Post review findings

---

Review: feat(distribution): add sync-managed claude review-lane caller components

One commit landed since Pass 4. All pass 4 findings remain resolved. No new blocking issues.

---

Pass 4 findings — still resolved

All four findings from Pass 4 were addressed before that review posted. This commit does not touch policy.json, sync-manifest.yml, ai-review-bot-composition.md, or distribution/README.md, so their resolutions are stable.

---

New commit d0ef74b — verified correct

test(claude-lanes): fail on an unreadable managed list instead of skipping

components/claude-lanes/claude-lanes.test.sh#L94-L99

The vacuousness hazard was real. The original inner loop queried the manifest on each iteration and the || continue correctly skipped non-managed components — but it also swallowed a failed or empty yq result, silently dropping all four assert_file_exists calls per target while the suite reported green. The commit message's proof is reproducible: a bad yq path (managedX instead of managed) drops 4 assertions without changing the exit code.

The fix is correct on all relevant axes:

• Read-once and fail-empty: mapfile -t managed < <(yq -r ".targets.\"$target\".managed[]" "$manifest") captures the list once; assert_nonzero "$target has a readable managed list" "${#managed[@]}" catches the empty case before any membership test runs. This is the same assert_nonzero-as-count-check pattern already established at lines 49 and 57.
• Membership test unchanged in semantics: printf '%s\n' "${managed[@]}" | grep -qxF "$component" || continue is a faithful rewrite of the original yq | grep pipeline; -F (fixed string) and -x (exact match) eliminate any regex concerns.
• set -uo pipefail compatibility: mapfile always initializes the array (avoiding the set -u unset-variable trap even when yq returns nothing). The || continue on the grep pipeline correctly handles the non-managed case without becoming an unhandled pipeline failure.
• No interaction with the control case: The bottom-of-test control run (lines 114–121) uses lane_targets[0], which is populated before this function and unaffected.
• Alignment with Pass 3 and Pass 4 coverage claims: Pass 4 confirmed the test "derives lanes from manifest, not hardcoded" and "materializes via the real sync-manifest.sh apply engine." Those properties still hold; this commit tightens the assertion density of the same loop rather than changing its structure.

No issues found.

---

Overall status

Gate	Status
Gate 1 — actionlint suppression (#284)	✅ Resolved (merged 2026-07-27)
Gate 2 — sync App workflows: write	⏳ Pending external org-owner action
Gate 3 — workflow name: / ruleset context	✅ Resolved by scope change (claude-code-plugins → locally-owned)
Gate 4 — #289 re-pin sync engine at ac223bb	⏳ Still open per PR body; #290 → #289 → #286 ordering

The only failing check is do-not-merge / do-not-merge — the intentional label gate.

[claude]

Claude finished @kyle-sexton's task in 2m 17s —— View job

---

Code Review (Pass 6)

• Load tools
• Read REVIEW.md and project guidelines
• Read the new commit (9c02de8) and changed files
• Verify pass 5 findings still hold
• Post review findings

---

Review: feat(distribution): add sync-managed claude review-lane caller components

One commit landed since Pass 5. All Pass 5 findings remain resolved. No new blocking issues.

---

Pass 5 findings — still resolved

This commit touches only components/claude-lanes/claude-lanes.test.sh. No changes to policy.json, sync-manifest.yml, ai-review-bot-composition.md, distribution/README.md, or the component YML files — all prior resolutions are stable.

---

New commit 9c02de8 — verified correct

test(claude-lanes): assert each target ran a destination assertion

The commit closes a second vacuousness hazard, distinct from the one d0ef74b addressed. The commit message's proof is internally consistent and the mutation test counts match the implementation.

The asymmetry that triggered this. Target admission (outer loop, selecting which targets to iterate) used a substring match (grep -F over \t<component>), while the per-target membership check (inner loop, selecting which lane files to assert) used a whole-line match (grep -qxF). A future component named as an extension of an existing one — claude-review-caller-v2 — would be admitted by the substring test and rejected by the exact test, so that target's iteration would produce zero destination assertions while the suite reported green.

The two changes:

1. Both membership tests are now exact associative-array lookups. is_lane_component is built once before the outer loop; manages is built fresh per target inside the inner loop. Both use ${assoc[$key]-} to provide an empty default under set -u, which is the same correct pattern as the existing test harness. declare -A manages=() inside the script-level for loop correctly reinitializes the array on each iteration — this is idiomatic bash. ✓

2. destinations counter with assert_nonzero on the count. destinations=$((destinations + 1)) increments before each assert_file_exists, and assert_nonzero "$target ran a destination assertion" "$destinations" follows after the inner loop. This is the correct invariant: it fails closed regardless of the failure mode — broken query, renamed component, or membership test that stops matching — because the symptom in all cases is destinations=0. ✓

3. Standalone loop assert_file_exists added at line 85. Previously the loop relied on actionlint's exit 3 for an empty workflows directory as an indirect signal; the explicit assertion documents the intent directly. ✓

Mutation test count verification. The claim is 30 PASS / 0 FAIL intact and 22 PASS / 4 FAIL with broken membership. Working through the assertion counts:

• Preamble: 2 (assert_nonzero ×2)
• Standalone loop (2 lane callers × 3 assertions each): 6
• Target loop (4 targets × 5 assertions each): 20 — one assert_exit (materialize) + one assert_file_exists (destination) + one assert_nonzero (count) + one assert_exit (lint) + one assert_silent (findings)
• Control case: 2

Total: 30. With broken membership: assert_file_exists (destination, line 118) is skipped and assert_nonzero (count, line 120) fails, so each target loses 1 PASS and gains 1 FAIL → 30 - 4 skipped + 4 FAIL = 22 PASS / 4 FAIL. Consistent. ✓

No issues found.

---

Overall gate status

Gate	Status
Gate 1 — actionlint suppression (#284)	✅ Resolved (merged 2026-07-27)
Gate 2 — sync App workflows: write	⏳ Pending external org-owner action
Gate 3 — workflow name: / ruleset context	✅ Resolved by scope change (claude-code-plugins → locally-owned)
Gate 4 — #289 re-pin sync engine at ac223bb	⏳ Open per PR body (#290 → #289 → #286 ordering)

The only failing check is do-not-merge / do-not-merge — the intentional label gate.