Skip to content

feat(distribution): add actionlint queue-suppression component for no-config consumers - #284

Merged
kyle-sexton merged 7 commits into
mainfrom
feat/actionlint-suppression-component
Jul 27, 2026
Merged

feat(distribution): add actionlint queue-suppression component for no-config consumers#284
kyle-sexton merged 7 commits into
mainfrom
feat/actionlint-suppression-component

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a new managed actionlint component distributing the Phase-0d-approved
.github/actionlint.yaml suppression — scoped to actionlint's
unexpected key "queue" for "concurrency" section false positive
(rhysd/actionlint#654; GitHub accepts and honors the GA concurrency.queue
key, live-probe-verified) — and materializes it into dotfiles,
provisioning, and claude-code-plugins only. The source doubles as
this repository's own live config (.github/actionlint.yaml): standards
lints its own workflows via the same pinned ci-workflows actionlint
composite and will carry a repo-local Claude review caller.

This is Phase 3c0 of the claude-review-lanes plan (ci-workflows
docs/topics/claude-review-lanes/PLAN.md): a fleet precondition. Caller
workflow components use the queue: max concurrency key; without this
suppression landing first, every caller sync PR fails the consumer's own
actionlint lane feeding its required ci-status check, and the rollout
wedges.

Probe evidence (empirical, fleet-wide)

  • All six consumers that lint their own workflows run actionlint 1.7.12
    via the ci-workflows shared composite — no version skew; the paths
    config key is accepted (shipped upstream in v1.7.4).
  • The glob-dialect config auto-discovers and suppresses correctly: with a
    queue: max workflow present, config present → exit 0; config removed →
    exit 1 (recorded in the plan's Phase 0d block).
  • A UNIFORM fleet component would break the excluded repos: overwriting
    medley's config produced 12 label errors, and ci-runner's local waivers
    would be clobbered — hence the component targets ONLY repos with no
    existing actionlint config.

Deliberate exclusions

Recorded twice, deliberately: explanatory comments at the component entry,
plus machine-readable locally-owned markings under medley, ci-runner, ci-workflows (pre-existing locally-owned copy), and
github-iac. The marking is what the manifest's ownership model prescribes
for a repo-specific opt-out on an otherwise relevant component: the
synchronizer never reads, changes, or deletes the file, and the periodic
Reconcile obligation (canonical-minimum-content check, not a byte diff)
becomes the drift control for exactly the moment queue-bearing callers land
in those repos and their local configs must carry the suppression.

  • medley — keeps its locally-owned config: repo-specific
    self-hosted-runner labels including the deliberate
    ci-runner-selection-failed sentinel, load-bearing in 12 workflows.
  • ci-runner — keeps its locally-owned config with repo-specific waivers.
  • github-iac — extends its own enumerated config in-repo; its
    GovernanceTopologyTests pin the config content.
  • standards — manifest source, not a target; carries the canonical file
    itself (this PR). No ownership label, per the README's own-originals rule.
  • .github — no workflows needing it; omission means unclassified.

Test plan

  • distribution/sync-manifest.sh validateManifest valid: 32 components, 8 targets (sources staged, matching indexed Git blobs).
  • yq -o=json distribution/sync-manifest.yml | node distribution/validate-sync-manifest.mjs — Draft 2020-12 schema-valid.
  • distribution/sync-manifest.sh plan over all six affected targets —
    exit 0; dotfiles, provisioning, and claude-code-plugins each plan exactly
    the one new 100644 .github/actionlint.yaml -> .github/actionlint.yaml
    mapping; medley, ci-runner, and github-iac show
    locally-owned actionlint (not modified) — no materialization from the
    marking.
  • components/actionlint/actionlint.test.sh (new slice, run in the actionlint
    CI job with the composite-installed engine) — 7/7 assertions with actionlint
    1.7.12: queue workflow clean with the config, unrelated violation still
    fails, configless control reproduces the suppressed message (removal
    tripwire for concurrency: Add support for queue key rhysd/actionlint#654).
  • Lefthook pre-commit (typos, editorconfig, gitleaks) passed on the staged
    files.

Related

  • Phase 3c0 of melodic-software/ci-workflows
    docs/topics/claude-review-lanes/PLAN.md — fleet actionlint precondition;
    suppression approved 2026-07-26 (Approval record item 3; fleet
    distribution rides the same approval).
  • Upstream gap: concurrency: Add support for queue key rhysd/actionlint#654. REMOVAL
    TRIGGER recorded at the suppression site: delete the ignore when the fix
    ships in the pinned actionlint version.
  • ci-workflows' own identical suppression: melodic-software/ci-workflows
    .github/actionlint.yaml (Phase 0d).

No linked issue.

🤖 Generated with Claude Code

…-config consumers

Distributes the Phase-0d-approved `.github/actionlint.yaml` suppression
(scoped to actionlint's `concurrency.queue` syntax-check false positive,
rhysd/actionlint#654) to the consumers that lint their own workflows and
carry no actionlint config today: dotfiles, provisioning, and
claude-code-plugins. The source doubles as this repository's own live
config, since standards lints its own workflows via the same pinned
ci-workflows composite.

medley, ci-runner, and github-iac are deliberately excluded (existing
repo-specific configs a whole-file sync would clobber); rationale
recorded as comments at the component entry.

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

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

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


Review

  • Gather context (CLAUDE.md, REVIEW.md)
  • Read changed files
  • Review PR diff
  • Post findings

Summary: The PR is well-scoped, thoroughly documented, and the actionlint config format is correct. One Important finding on the component source path, one Nit on a documentation gap in the manifest comment.


🔴 Important — Component source breaks the established convention and creates a silent distribution risk

Every other component in the manifest sources its files from under components/<name>/. The actionlint component sources from .github/actionlint.yaml — the repository's live config — with no canonical copy under components/actionlint/:

# All other components:
agent-orientation:
  files:
    components/agent-orientation/orientation.md: AGENTS.md

# This component (unconventional):
actionlint:
  files:
    .github/actionlint.yaml: .github/actionlint.yaml

The risk: if a future contributor adds a standards-specific suppression to .github/actionlint.yaml (e.g., silencing a false positive unique to this repo's workflows) without knowing it's a distribution source, the next sync silently pushes that repo-local ignore to dotfiles, provisioning, and claude-code-plugins. Those consumers opted into the queue suppression only — not unbounded accumulation of the standards repo's own waivers.

The conventional fix is a canonical source at components/actionlint/actionlint.yaml with the manifest referencing it, and .github/actionlint.yaml either pointing to or copying it. That maintains the "standards' live config is validated against the distributed artifact" property the PR description values while keeping the source-of-truth arrow clear.

The PR body explains this is intentional ("the source doubles as this repository's own live config"), but the risk is real enough and the convention deviation clear enough that it warrants a deliberate call from the author before merging.

Fix this →


🟡 Nit — ci-workflows exclusion undocumented in the manifest comment

The component entry comment enumerates five deliberate exclusions (medley, ci-runner, github-iac, standards, .github) — but ci-workflows is also a manifest target that doesn't receive actionlint. The rationale (ci-workflows already carries its own identical suppression at Phase 0d) is captured in the PR body but not in the manifest. A future editor scanning the exclusion list will hit an unexplained gap.

Fix this →


Everything else looks good: the glob patterns cover both .yml and .yaml extensions correctly, the error message string matches the false positive exactly, the REMOVAL TRIGGER comment is present, the component name passes the schema regex, the three target additions are alphabetically ordered, and the PR test plan (validate + schema-valid + plan dry-run) is comprehensive.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fbd618b490

ℹ️ 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".

Comment thread .github/actionlint.yaml
Comment thread distribution/sync-manifest.yml
…r, github-iac

Makes the deliberate exclusions machine-readable per the ownership model
in distribution/README.md, alongside the existing component-entry
comments. `locally-owned` is the manifest's opt-out seam for a
repo-specific config on an otherwise relevant component: the
synchronizer never touches the file, and the periodic Reconcile
obligation (canonical-minimum-content check) becomes the drift control
for when queue-bearing callers land in those repos and their local
configs must carry the suppression.

Plan output over all six affected targets confirms the marking
materializes nothing: medley, ci-runner, and github-iac each show
"locally-owned actionlint (not modified)".

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

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

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


Review

  • Gather context (CLAUDE.md, REVIEW.md)
  • Read changed files
  • Review PR diff
  • Post findings

Summary: The second commit (d104ac9) resolved the Codex P2 finding by adding locally-owned: actionlint for medley, ci-runner, and github-iac — those are now correctly classified. Two findings from the prior review remain open, and docs/component-lifecycle.md raises a third that hasn't been addressed.


🔴 Important — Verification admission evidence is missing per docs/component-lifecycle.md

docs/component-lifecycle.md lists Verification as a required admission evidence item:

Add behavioral tests for the policy boundary, fixtures for expected pass and fail cases, deterministic installation, and CI that runs the same entrypoint consumers will use. Structural validation is not behavioral proof.

The PR documents probe results from ci-workflows' docs/topics/claude-review-lanes/PLAN.md (Phase 0d), but that evidence lives in an external repository and is not testable from standards' own CI. There are no fixture files here and no CI step that runs the pinned actionlint composite against a queue: max workflow to confirm suppression works. If the actionlint diagnostic text changes or the pinned version changes its path-config behavior, manifest validation stays green while every consumer sync PR breaks — exactly the silent-failure scenario the lifecycle criterion guards against.

The minimum evidence set would be:

  1. A fixture workflow file under a test directory containing a concurrency.queue key.
  2. A CI step that runs actionlint against it with the config present (expect exit 0) and without it (expect non-zero or the error message to appear).

Fix this →


🔴 Important — Component source deviates from convention and creates a silent accumulation risk

Every other component in the manifest sources from under components/<name>/. The actionlint component sources directly from .github/actionlint.yaml — the repository's live config — with no canonical copy under components/actionlint/:

# All other components:
agent-orientation:
  files:
    components/agent-orientation/orientation.md: AGENTS.md

# This component (unconventional):
actionlint:
  files:
    .github/actionlint.yaml: .github/actionlint.yaml

Permalink: sync-manifest.yml line 20–22

The risk: a future contributor adds a standards-specific suppression to .github/actionlint.yaml without knowing it is a distribution source. The next sync silently pushes that repo-local ignore to dotfiles, provisioning, and claude-code-plugins, which opted in only to the queue suppression. The PR body acknowledges this is intentional ("the source doubles as this repository's own live config"), but the risk is real enough — and the convention deviation clear enough — that it warrants a deliberate call from the author on record before merging.

The conventional fix is a canonical source at components/actionlint/actionlint.yaml with .github/actionlint.yaml either copying or symlinking it, so the source-of-truth arrow is unambiguous.

Fix this →


🟡 Nit — ci-workflows exclusion is undocumented in the manifest comment

The component entry comment enumerates five deliberate exclusions (medley, ci-runner, github-iac, standards, .github), but ci-workflows is also a target in the manifest that receives neither managed nor locally-owned: actionlint. The rationale (ci-workflows already carries an identical suppression from Phase 0d of the claude-review-lanes plan) is captured in the PR body but not in the manifest. A future editor scanning the exclusion list will find an unexplained gap.

Permalink: sync-manifest.yml lines 7–19

Fix this →


What the second commit fixed: locally-owned: actionlint has been correctly added to medley, ci-runner, and github-iac — the ownership model is now consistent with distribution/README.md's definitions.

Everything else looks good: the suppression config format is correct, both .yml and .yaml glob patterns are covered, the error message string matches the false positive exactly, the REMOVAL TRIGGER comment is present at the suppression site, the component name passes the schema regex, the three target additions are in alphabetical order, and no secrets or security-sensitive content is introduced.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d104ac99fd

ℹ️ 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".

Comment thread distribution/sync-manifest.yml
Independent re-measurement against medley's real 28-workflow tree found
12 label errors across 12 workflows, not 24, from overwriting its
actionlint config in the fleet probe. Comment-only correction.

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

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

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


Review

  • Gather context (CLAUDE.md, REVIEW.md)
  • Read changed files
  • Review PR diff (latest commit: 32f122f)
  • Post findings

Summary: The third commit (32f122f) corrects one prose number in the manifest comment (24→12 label errors for the medley probe). It does not address the two open Important findings from prior reviews. A third Important finding surfaced on re-read: ci-workflows is missing the same locally-owned: actionlint classification that was correctly applied to medley, ci-runner, and github-iac in d104ac9.


🔴 Important — Verification admission evidence still missing

docs/component-lifecycle.md requires:

Add behavioral tests for the policy boundary, fixtures for expected pass and fail cases, deterministic installation, and CI that runs the same entrypoint consumers will use. Structural validation is not behavioral proof.

None of the three commits add fixture files or a CI step exercising the suppression. The probe evidence cited in the PR body lives in melodic-software/ci-workflows' plan document — an external repo, not verifiable from standards' own CI. distribution/sync-manifest.sh validate confirms manifest structure; it does not confirm actionlint behavior. If the diagnostic text in actionlint changes, or path-config behavior regresses in the pinned version, standards' CI stays green while every subsequent consumer sync PR breaks — the silent-failure scenario this criterion guards against.

Minimum evidence set:

  1. A fixture workflow file (e.g. components/actionlint/fixtures/queue-workflow.yml) that contains a concurrency.queue key.
  2. A CI step that runs actionlint against it with the config (expect exit 0) and without it (expect the known error message or non-zero exit).

Permalink: docs/component-lifecycle.md lines 55–58

Fix this →


🔴 Important — ci-workflows lacks locally-owned: actionlint — same gap that d104ac9 fixed for three other targets

distribution/README.md lines 38–39:

locally-owned records a deliberate repository-specific implementation or opt-out. Omission means the component is irrelevant or has not been classified.

Lines 113–115 state the rule directly: record a deliberate exception as locally-owned only when it clarifies an otherwise relevant component.

ci-workflows lints its own workflows via the same pinned actionlint composite and carries its own .github/actionlint.yaml from Phase 0d — actionlint is unambiguously relevant there. Without locally-owned: actionlint, the manifest classifies it as "irrelevant or unclassified," which is factually wrong and will mislead future reconciliation audits. d104ac9 correctly applied this label to medley, ci-runner, and github-iac for exactly the same reason; ci-workflows needs the same treatment.

Permalink: distribution/sync-manifest.yml lines 186–198

# Missing from the ci-workflows target:
melodic-software/ci-workflows:
  managed:
    - comment-hygiene-action
    # ...
  locally-owned:          # ← should be here
    - actionlint

Fix this →


🔴 Important — Component source deviates from convention and creates a silent accumulation risk

Every other component in the manifest sources from components/<name>/. The actionlint component sources directly from .github/actionlint.yaml — the repository's live operational config — with no canonical copy under components/actionlint/:

# Every other component:
agent-orientation:
  files:
    components/agent-orientation/orientation.md: AGENTS.md

# This component (unconventional):
actionlint:
  files:
    .github/actionlint.yaml: .github/actionlint.yaml

Permalink: distribution/sync-manifest.yml lines 20–22

The risk: a future contributor adds a standards-specific suppression to .github/actionlint.yaml without knowing it is a distribution source. The next sync silently pushes it to dotfiles, provisioning, and claude-code-plugins, which opted in only to the queue suppression. The conventional fix is a canonical source at components/actionlint/actionlint.yaml referenced by the manifest, with .github/actionlint.yaml being a copy — preserving the "live config validates against distributed artifact" property the PR description values while keeping the source-of-truth arrow unambiguous.

This finding has been open across two prior review rounds. The PR body acknowledges the deviation is intentional, but "intentional" is not the same as "the risk is accepted on the record" — that call belongs in the thread.

Fix this →


🟡 Nit — ci-workflows exclusion still undocumented in the manifest comment

The component comment enumerates five deliberate exclusions (medley, ci-runner, github-iac, standards, .github) but ci-workflows — a manifest target that receives neither managed nor locally-owned: actionlint — is not mentioned. Resolving the Important finding above (adding locally-owned: actionlint for ci-workflows) would make this the natural place to also add a comment bullet explaining why: ci-workflows carries its own identical suppression from Phase 0d of the claude-review-lanes plan.


What the three commits did:

  • fbd618b — added the component and materialized it into dotfiles, provisioning, claude-code-plugins
  • d104ac9 — added locally-owned: actionlint for medley, ci-runner, github-iac ✓
  • 32f122f — corrected medley probe figure in the comment from 24 to 12 label errors ✓

Everything else looks good: the actionlint config format is correct, both .yml and .yaml glob patterns are covered, the error message string matches the false positive exactly, the REMOVAL TRIGGER comment is present at the suppression site, the component name passes the schema regex, the target additions are alphabetically ordered within their lists, and no secrets or security-sensitive content is introduced.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 32f122f428

ℹ️ 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".

Comment thread distribution/sync-manifest.yml
ci-workflows is a manifest target carrying its own .github/actionlint.yaml
— the canonical origin this component's content mirrors — yet had no
actionlint classification, which the ownership model reads as "not
classified". Marks it locally-owned so the most relevant repo is
deliberately classified; the removal trigger on rhysd/actionlint#654
fires there too.

Plan output for ci-workflows confirms no materialization:
"locally-owned actionlint (not modified)".

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

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 58s —— View job


Review

  • Gather context (CLAUDE.md, REVIEW.md)
  • Read changed files (latest commit: 4e18553)
  • Verify which prior findings are resolved vs still open
  • Post findings

Summary: Commit 4e18553 resolves the last Important finding from the prior round — ci-workflows now carries locally-owned: actionlint with an inline comment explaining it is the canonical origin. That finding and the nit (previously the ci-workflows omission was unexplained) are now closed. Two Important findings remain open across all four rounds.


🔴 Important — Verification admission evidence still missing

docs/component-lifecycle.md lines 55–58:

Add behavioral tests for the policy boundary, fixtures for expected pass and fail cases, deterministic installation, and CI that runs the same entrypoint consumers will use. Structural validation is not behavioral proof.

Four commits in, no components/actionlint/ directory exists, and there are no fixture files or CI step exercising the suppression. The probe evidence cited in the PR body lives in melodic-software/ci-workflows' plan document — unverifiable from standards' own CI. distribution/sync-manifest.sh validate confirms manifest structure; it does not confirm that actionlint accepts the config, suppresses the known diagnostic, and only the known diagnostic. If the diagnostic text changes in the pinned actionlint version, standards' CI stays green while every subsequent consumer sync PR breaks — the silent-failure scenario this criterion was written to guard against.

Minimum evidence set:

  1. A fixture workflow file (e.g. components/actionlint/fixtures/queue-workflow.yml) containing a concurrency.queue key.
  2. A CI step that runs actionlint against it with the config (expect exit 0) and without it (expect the known error message or non-zero exit) — matching the entrypoint consumers use via the ci-workflows composite.

Permalink: docs/component-lifecycle.md lines 55–58

Fix this →


🔴 Important — Component source deviates from convention and creates a silent accumulation risk

Every other component in the manifest sources from components/<name>/. The actionlint component sources directly from .github/actionlint.yaml — the repository's operational live config — with no canonical copy under components/actionlint/:

# Every other component:
agent-orientation:
  files:
    components/agent-orientation/orientation.md: AGENTS.md

# This component (unconventional):
actionlint:
  files:
    .github/actionlint.yaml: .github/actionlint.yaml

Permalink: distribution/sync-manifest.yml lines 20–22

The risk: a future contributor adds a standards-specific suppression to .github/actionlint.yaml without knowing it is a distribution source. The next sync silently pushes it to dotfiles, provisioning, and claude-code-plugins, which opted in only to the queue suppression. The conventional fix is a canonical source at components/actionlint/actionlint.yaml referenced by the manifest, with .github/actionlint.yaml remaining as the live repo config — preserving the "live config validates against distributed artifact" property the PR body values while keeping the source-of-truth arrow unambiguous.

This finding has been open for four review rounds. The PR body acknowledges the deviation is intentional, but that is a description of the design decision, not a recorded acceptance of the risk. If the risk is accepted, a comment at the source entry stating so would close this finding without requiring a structural change.

Fix this →


What commit 4e18553 fixed: Added locally-owned: actionlint to melodic-software/ci-workflows with an inline comment ("Canonical origin of the actionlint config; keeps its own copy — the removal trigger fires there too."). This closes the Important finding and the nit about the unexplained gap. The component-level exclusion comment block still doesn't enumerate ci-workflows, but the target-level comment and locally-owned marking together make the classification self-explanatory in context.

What all four commits did:

  • fbd618b — added the component and materialized it into dotfiles, provisioning, claude-code-plugins
  • d104ac9 — added locally-owned: actionlint for medley, ci-runner, github-iac ✓
  • 32f122f — corrected medley probe figure in the comment from 24 to 12 label errors ✓
  • 4e18553 — added locally-owned: actionlint for ci-workflows with an inline explanation ✓

Everything else looks good: the actionlint config format is correct, both .yml and .yaml glob patterns are covered, the error message string matches the false positive exactly, the REMOVAL TRIGGER comment is present at the suppression site, the component name passes the schema regex, target additions are alphabetically ordered within their lists, and no secrets or security-sensitive content is introduced.

@kyle-sexton
kyle-sexton enabled auto-merge (squash) July 27, 2026 13:47

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4e18553455

ℹ️ 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".

Comment thread distribution/sync-manifest.yml Outdated
kyle-sexton and others added 2 commits July 27, 2026 10:18
…ly-owned

"Canonical origin" left two apparent authorities for one component: the
ownership model gives a component one normative owner, and standards'
root config is the source. The ci-workflows copy is the pre-existing
locally-owned implementation the content was derived from — say exactly
that.

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

Completes the repository-shape contract for the new root tool config:
components/actionlint/ documents and behaviorally tests the exported
.github/actionlint.yaml per docs/component-lifecycle.md's Verification
requirement — pass/fail fixtures against the pinned consumer entrypoint,
not structural validation alone.

The test builds a scratch consumer project from the root-canonical
config: the concurrency.queue workflow lints clean, an unrelated
syntax-check violation still fails alongside it (message-scope proof),
and a configless control run reproduces the suppressed diagnostic. The
control case doubles as the removal tripwire: when rhysd/actionlint#654
ships in the pinned engine it fails, firing the config's recorded
removal trigger instead of leaving a stale suppression.

CI runs the suite in the actionlint job, reusing the engine the pinned
ci-workflows composite installs on the job PATH. Verified locally with
actionlint 1.7.12: 7/7 assertions pass.

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

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 4m 20s —— View job


Review

  • Gather context (CLAUDE.md, REVIEW.md)
  • Read changed files (latest commit: 86f392b)
  • Verify which prior findings are resolved vs still open
  • Post findings

Summary: Commit 86f392b resolves the persistent Verification finding — behavioral fixtures, test script, and CI wiring are all in place and well-constructed. One Important finding carries into this round: the distribution source file carries no in-file warning that it is a distribution artifact, leaving the accumulation risk documented across five prior rounds unmitigated at the file level. One new Nit on CI actionlint PATH availability.


🔴 Important — .github/actionlint.yaml carries no in-file warning that it is the distribution source

The manifest sources the distributed artifact directly from .github/actionlint.yaml:

actionlint:
  files:
    .github/actionlint.yaml: .github/actionlint.yaml

Permalink: distribution/sync-manifest.yml lines 20–22

The component README (components/actionlint/README.md) and the manifest comment both document the relationship. But .github/actionlint.yaml itself says nothing about being a distribution source. A contributor who opens that file to add a standards-specific suppression — silencing a false positive unique to this repo's own workflows — won't encounter any warning before the next sync pushes it to dotfiles, provisioning, and claude-code-plugins. Those consumers opted into the queue suppression only.

The conventional fix (a canonical source at components/actionlint/actionlint.yaml) has been noted across every prior round. A minimal resolution that avoids the structural change: add a comment to .github/actionlint.yaml that makes the distribution relationship discoverable at the point of edit — something like:

# NOTE: This file is the distribution source for the actionlint fleet component.
# See components/actionlint/README.md before adding standards-specific ignores —
# they would be distributed to all fleet consumers on next sync.

Either the structural fix or the in-file comment closes this finding. This finding has been open five rounds; the PR body correctly describes the design as intentional, and the component README correctly documents it — the gap is that the file a future editor is most likely to open has no such documentation.

Fix this →


🟡 Nit — CI actionlint PATH availability is unverified; a skip is silent success

The test step runs bash harness/shell/run-tests.sh components/actionlint/actionlint.test.sh immediately after the actionlint composite:

- name: Lint workflows
  uses: melodic-software/ci-workflows/.github/actions/actionlint@f2d5e06...
- name: Test actionlint component
  run: bash harness/shell/run-tests.sh components/actionlint/actionlint.test.sh

Permalink: .github/workflows/ci.yml diff

The test script gracefully skips the entire suite when actionlint is not in PATH (skip_suite 'actionlint not installed'), and run-tests.sh counts a skip as exit 0. If the composite downloads and runs actionlint but does not leave it in PATH for subsequent steps, the CI step exits 0 with "SKIP" — providing no behavioral verification at all, while appearing green. The step placement suggests the composite is expected to make actionlint available in PATH, but that dependency is implicit. If it does, a single line in the job comment or step name would make the assumption explicit. If it doesn't, an explicit actionlint install step is needed before the test.


What commit 86f392b resolved — Verification admission evidence (was open all five rounds):

The component now has complete behavioral verification:

The test logic is sound. The harness functions (skip_suite, require_min_version, make_repo, assert_exit, assert_silent, assert_nonzero, assert_contains, assert_not_contains) are all defined in harness/shell/lib.sh and used correctly. The scratch-repo isolation via make_repo is the right approach — it mirrors a consumer checkout and avoids the standards repo's own .github/ config interfering with the test's control case. The version guard (require_min_version actionlint ... 1.7.4) correctly skips rather than fails on older engines that reject the paths config key.

Full commit history for this PR:

  • fbd618b — added the component, materialized into dotfiles, provisioning, claude-code-plugins
  • d104ac9 — added locally-owned: actionlint for medley, ci-runner, github-iac ✓
  • 32f122f — corrected medley probe figure in comment (24→12 label errors) ✓
  • 4e18553 — added locally-owned: actionlint for ci-workflows with explanation ✓
  • 86f392b — added component slice with behavioral fixtures, test script, CI step, and README ✓ (Verification criterion resolved)

Everything else looks good: the actionlint config format is correct, both .yml and .yaml glob patterns are covered, the error message string matches the false positive exactly, the REMOVAL TRIGGER comment is present at the suppression site, the component name passes the schema regex, target additions are alphabetically ordered within their lists, and no secrets or security-sensitive content is introduced.

…rigger

The control-case comment embedded a tracker reference, which the
comment-hygiene gate forbids in code comments. The config file owns that
citation as its recorded removal trigger; the comment now points there
instead of repeating it. Verified with the gate's own scan-tree.sh and
patterns: clean; component test still 7/7.

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

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

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


Review

  • Gather context (CLAUDE.md, REVIEW.md)
  • Read changed files (latest commits: 86f392b, b081468)
  • Verify which prior findings are resolved vs still open
  • Post findings

Summary: Commits 86f392b and b081468 resolve the Verification admission evidence finding — the behavioral suite is comprehensive and correctly wired into CI. One Important finding remains open across all five review rounds: the component source path convention.


✅ Verification admission evidence — RESOLVED

Commit 86f392b adds a complete components/actionlint/ behavioral suite that satisfies docs/component-lifecycle.md's Verification requirement:

Commit b081468 fixes a comment hygiene gate failure introduced in 86f392b: the control-case comment embedded rhysd/actionlint#654 directly, which the comment-hygiene gate forbids. It now reads "the upstream fix shipped in the pinned engine" and points to the config's REMOVAL TRIGGER instead. Clean and correct.


🔴 Important — Component source still deviates from convention (fifth round)

Every other component sources its files from components/<name>/. The actionlint component sources directly from .github/actionlint.yaml — the repository's operational live config — with no canonical file under components/actionlint/:

# Every other component:
agent-orientation:
  files:
    components/agent-orientation/orientation.md: AGENTS.md

# This component (unconventional):
actionlint:
  files:
    .github/actionlint.yaml: .github/actionlint.yaml

Permalink: distribution/sync-manifest.yml lines 20–22

The risk: a future contributor adds a standards-specific suppression to .github/actionlint.yaml without knowing it is a distribution source. The next sync silently pushes it to dotfiles, provisioning, and claude-code-plugins, which opted in only to the queue suppression. The behavioral suite confirms that the config currently suppresses only the intended diagnostic — but it does not guard against a future accumulation: adding a new ignore entry would not break any of the 7 test assertions.

The manifest comment says "the source doubles as this repository's own live config," which describes the design but does not explicitly record the risk as accepted. The prior review round noted that a comment at the source entry explicitly acknowledging the risk would close this finding without a structural change. That comment has not been added.

Two paths to resolution (either is sufficient):

Option A — explicit risk-acceptance comment at the actionlint: entry:

  # Source is the repo's own live config (.github/actionlint.yaml), not
  # components/actionlint/. Risk accepted: any future repo-local ignore added
  # to that file will distribute to managed targets. Maintainers adding an
  # ignore must assess distribution scope before committing.
  actionlint:
    files:
      .github/actionlint.yaml: .github/actionlint.yaml

Option B — canonical copy at components/actionlint/actionlint.yaml referenced by the manifest, with .github/actionlint.yaml remaining as the live config (preserving the "live config is the same content" property).

Fix this →


What commits 86f392b and b081468 fixed:

  • Added the full behavioral test suite (fixtures, test script, README) ✓
  • Wired the test into the actionlint CI job ✓
  • Fixed comment hygiene gate failure (tracker reference in code comment) ✓

What all seven commits did:

  • fbd618b — added the component; materialized into dotfiles, provisioning, claude-code-plugins
  • d104ac9 — added locally-owned: actionlint for medley, ci-runner, github-iac ✓
  • 32f122f — corrected medley probe figure (24→12 label errors) ✓
  • 4e18553 — added locally-owned: actionlint for ci-workflows ✓
  • fde84a6 — updated ci-workflows comment to "pre-existing locally-owned copy" (removes "canonical" claim) ✓
  • 86f392b — added component slice: README, behavioral test suite, fixtures, CI step ✓
  • b081468 — fixed comment hygiene (removed tracker reference from test comment) ✓

Everything else looks good: the actionlint config format is correct, both .yml and .yaml glob patterns are covered, the error message string matches the false positive exactly, the REMOVAL TRIGGER comment is in the config, the behavioral test tripwire will fire if the diagnostic text changes in the pinned engine, the component name passes the schema regex, target additions are alphabetically ordered within their lists, and no secrets or security-sensitive content is introduced.

@kyle-sexton
kyle-sexton merged commit ae33708 into main Jul 27, 2026
43 checks passed
@kyle-sexton
kyle-sexton deleted the feat/actionlint-suppression-component branch July 27, 2026 14:24

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b0814688b4

ℹ️ 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".


queue_message='unexpected key "queue" for "concurrency" section'

out="$(cd "$project" && actionlint -no-color 2>&1)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Exercise the composite entrypoint in the fixture test

The fresh suite added after the earlier thread still invokes the actionlint binary directly rather than the ci-workflows composite that consumers use. In the checked CI ordering (.github/workflows/ci.yml:463-466), the composite runs before the scratch project exists, and the repository itself has no queue-bearing workflow, so a wrapper change that stops discovering this config could leave both steps green while every queue-bearing consumer fails. docs/component-lifecycle.md:55-58 specifically requires CI to exercise the consumer entrypoint; make the composite see the queue fixture (or expose and test the exact script it invokes) rather than validating only the underlying binary.

AGENTS.md reference: AGENTS.md:L3-L8

Useful? React with 👍 / 👎.

kyle-sexton added a commit that referenced this pull request Jul 29, 2026
## Summary

Re-pins the `sync` job's engine from ci-workflows `0b45b9f` (2026-07-18)
to
`ac223bb` (2026-07-28). `0b45b9f` predates the sync job's
`permission-workflows: write` token-mint request, so the
melodic-standards-sync
App's Workflows grant is inert at the old pin and the sync cannot
materialize a
target repository's workflow files.

The runner-policy gate admits a reusable call only at a reviewed
`path@SHA`, so
the new contract is registered alongside the pin in the same change, per
the
lockstep in `components/runner-policy/README.md`. The entry is
byte-identical to
the reviewed `0b45b9fb` one — same `runner-input` routing, same five
allowed
inputs, same two-secret map — because the engine's delta widens nothing
a caller
may pass.

The `0b45b9fb` entry is kept, but not for the usual mid-rollout reason:
an
org-wide code search finds exactly ONE caller of `standards-sync.yml`
(`melodic-software/standards/.github/workflows/sync.yml` — the five
downstream
`.github/standards/runner-policy/policy.json` hits are synced copies of
the
allowlist, not callers), and this PR migrates it. After merge, zero
workflows
pin `0b45b9fb`. It is retained only as the rollback target while
`ac223bb` is
unproven in a real run; the component's own step 4 says to drop
superseded
entries once every consumer has migrated, so it should be removed in the
Phase
3g sweep.

The grant this pin activates is present on installation 144867070,
verified live:

```console
$ gh api orgs/melodic-software/installations \
    --jq '.installations[] | select(.id==144867070) | {app_slug, permissions}'
{"app_slug":"melodic-standards-sync","permissions":{"contents":"write",
"issues":"write","metadata":"read","pull_requests":"write","workflows":"write"}}
```

## Delta audit

The engine calls no local composite action and no ci-workflows-side
script — the
manifest interpreter (`distribution/sync-manifest.sh`) is owned by this
repository and travels with `standards-ref`, not with the pin.
Third-party
actions are pinned inline inside the engine file. So
`standards-sync.yml` is the
entire delta surface, and three commits in `0b45b9f..ac223bb` touch it.

Behavior was compared under YAML normalization rather than by reading
the raw
patch, so anchor/alias expansion and formatting collapse out and only
semantic
change survives:

```bash
git show 0b45b9f:.github/workflows/standards-sync.yml | yq -o=json '.' > old.json
git show ac223bb:.github/workflows/standards-sync.yml | yq -o=json '.' > new.json
diff -u old.json new.json
```

| Commit | Change | Changes behavior for an existing target? |
| --- | --- | --- |
| `e8e5f3f` (#205) | Dedupes the two identical checksum-verified yq
install steps via a YAML anchor/alias (`&install-yq` / `*install-yq`) |
**No.** The step vanishes entirely from the normalized diff — proof the
alias expands to the byte-identical step, not an eyeball judgement. Both
jobs still read the same workflow-level `YQ_VERSION` /
`YQ_LINUX_AMD64_SHA256`. |
| `dd45dac` (#213) | Adds `id: cpr` to the create-pull-request step and
a new step that arms auto-merge (squash) on the PR | **Yes** — see
below. |
| `ac223bb` (#284) | Adds `permission-workflows: write` to the
per-target App token mint | **Yes** — see below. |

The normalized diff isolates exactly three additions:
`permission-workflows:
write` on the mint, `id: cpr` on the create-PR step, and the auto-merge
arming
step. Nothing was removed or reworded.

### `ac223bb` — `permission-workflows: write` on the mint

Intended effect: the per-target token can write files under
`.github/workflows/`,
which is what Phase 3a's caller components require. Stated precisely,
this is
capability, not current behavior — **no manifest component maps under
`.github/workflows/` today**, so the grant is unexercised until Phase 3a
lands
the first such mapping.

Failure mode if the premise were wrong: `create-github-app-token`
**fails the
mint** when the installation lacks a requested permission, and the mint
runs
before any target work — so a missing grant would fail the apply job for
*every*
target, not degrade gracefully. The live installation read above is the
evidence
that the premise holds. No other permission was widened; the mint stays
`repositories: <one target>`, `contents: write`, `pull-requests: write`.

### `dd45dac` — auto-merge armed at sync-PR creation

This is the one genuine behavior change for existing targets. After this
pin, a
sync PR is armed for auto-merge (squash) at creation, so it merges into
the
target's default branch once required checks go green — with no human
review
step. Two guards bound it:

- It fires only when `steps.cpr.outputs.pull-request-operation ==
'created'`, so
a refresh of an already-open sync PR does **not** arm (and does not
re-arm a PR
  a reviewer deliberately disarmed).
- It is skipped when the target's manifest record sets `automerge:
false`. That
  key is already supported by this repository's interpreter and schema
(`distribution/sync-manifest.sh`, `sync-manifest.schema.json`),
absent-means-true.

Blast radius today — **no target in `distribution/sync-manifest.yml`
sets
`automerge: false`**, so arming is live for all eight, and the
`created`-only
guard is what actually splits them:

| Target | Open `standards-sync` PR | Next sync |
| --- | --- | --- |
| `melodic-software/.github` | none | **created → armed** |
| `melodic-software/ci-runner` | none | **created → armed** |
| `melodic-software/ci-workflows` | none | **created → armed** |
| `melodic-software/claude-code-plugins` | none | **created → armed** |
| `melodic-software/dotfiles` | none | **created → armed** |
| `melodic-software/provisioning` | none | **created → armed** |
| `melodic-software/github-iac` | #234 | updated → not armed |
| `melodic-software/medley` | #1665 | updated → not armed |

The "updated -> not armed" rows are a coincidence with a shelf life, not
a
property of those two targets: all eight repositories set
`delete_branch_on_merge: true`, so once #234 and #1665 close, the next
sync
creates a fresh PR and arms it like everyone else.

Arming never fails the run: the GraphQL mutation is wrapped and a
rejection
downgrades to `core.warning`. That cuts both ways — an arming FAILURE
yields an
unarmed PR, which is invisible to the stuck-armed-PR watchdog and
indistinguishable from today's behavior. The first real run is the only
test of
whether an App installation token may call `enablePullRequestAutoMerge`
at all;
if it may not, the sync silently reverts to manual merging behind a
warning. A
watchdog arm flagging UNARMED App-authored sync PRs would close that gap
and
does not exist. Not fixed here: it is a ci-workflows-side change and
this PR is
scoped to the standards caller.

The watchdog half of #213 is already wired here —
`.github/workflows/standards-sync-stuck-automerge-alert.yml` exists and
is
pinned at `43bc8d0` — so an armed-but-blocked sync PR is not silent.

### Verdict for existing targets

**Safe to re-pin; one sequencing hazard to schedule around, not a
defect.**

`e8e5f3f` is provably inert. `ac223bb` is safe given the verified grant,
and its
failure mode is loud and immediate rather than silent. `dd45dac` does
change
outcomes for the six targets without an open sync PR: merging this PR is
itself
the activation event (`on: push: branches: [main]` with
`dry-run: ${{ inputs.dry-run || false }}`, so a push-triggered run is a
real
run), and the next sync will open and arm PRs against those six.

Two things the rollout owner should not get wrong:

- **Merge order is not the control.** The caller passes no
`standards-ref`, so
  the engine's plan job checks out `melodic-software/standards@main` at
  job-execution time and pins every downstream job to whatever SHA that
resolves to. Two PRs merged minutes apart are both carried by the FIRST
push-triggered run. Sequencing on "merge this one first" is a race; the
real
control is the `automerge: false` window Phase 3d already plans, or
holding
  the caller-component merge until a sync cycle has completed.
- **This PR's own merge is the activation event.** There is no staging
step
  between merging and the six targets getting armed PRs.

Nothing here breaks a target — the targets' own required checks still
gate the
merge. This PR does not merge itself.

### The two changes compose

Worth naming because neither commit describes it alone. After this pin,
the
per-target token may write `.github/workflows/`, sync PRs arm themselves
at
creation, and every target's `base` ruleset requires zero approving
reviews
(verified live). Human checkpoints on the path from a standards merge to
workflow content landing in a target therefore go from two (merge the
standards
PR, merge each target's sync PR) to one. `ci-workflows`, which hosts the
engine,
is itself a sync target, so the loop closes on itself.

This is inert today only because no component maps under
`.github/workflows/` —
and Phase 3a exists to change exactly that. Recorded in
`components/runner-policy/README.md` so the next person to add such a
mapping
meets it. Not a blocker for this PR; a decision the rollout owner should
make
consciously rather than inherit.

### What merging this PR sets off

Merging is a push to `main`, which fires a REAL non-dry-run sync
(`inputs` is
empty on push, so `dry-run` evaluates false). Two consequences the
merger should
expect:

- **The `attest` job is the re-proof of App/manifest set equality, and
that
  proof is currently stale.** The last successful non-dry-run attest was
`2026-07-27T15:16Z` (run 30279157140); installation 144867070's
`updated_at`
is `2026-07-29T01:56Z` — roughly 34 hours later, with no read-only way
to
re-verify in between. Most likely benign: a permissions-only grant,
consistent
with ci-workflows#284 merging minutes afterward. If the post-merge
attest goes
red, that is **fail-closed behavior working** — attestation runs before
any
mutation, so nothing is corrupted, and it self-clears once the grant and
the
  manifest target set agree. Do not patch around it.
- **The six targets without an open sync PR get created-and-armed PRs on
that
  same run.** There is no staging step.

### Deferred, not fixed here (out of scope)

`.github/workflows/standards-sync-stuck-automerge-alert.yml` pins
`43bc8d0`,
which predates `42329ef` (#234) — the fix that splits the stuck-PR scan
into a
cheap page fetch plus a per-candidate merge-state probe with retry. The
watchdog
functions at its current pin; it is simply one reliability fix behind.
Trigger to
revisit: the next time the watchdog misses or flakes on a stuck armed
PR, or the
Phase 3g fleet re-pin sweep.

## Test plan

- `actionlint .github/workflows/sync.yml` — clean.
- Input/secret conformance proved against the engine blob **at
`ac223bb`**, not
against ci-workflows `main`: the caller passes `runner`, `dry-run`,
`targets`,
`app-client-id`, `app-private-key`; all five are declared at that SHA,
both
secrets remain `required: false`, and no input gained `required: true`.
(A
caller passing an undeclared input hard-fails `workflow_call` before the
job
  starts.)
- `gh workflow run sync.yml --ref ci/repin-standards-sync-engine -f
dry-run=true`
  — **run

[30419339590](https://github.com/melodic-software/standards/actions/runs/30419339590),
`sync / plan` success.** This resolves the `workflow_call` against
`ac223bb`
for real and exercises the plan job, which is the direct test of the
input
contract. Limit stated honestly: a dry-run skips the apply job (`sync /
sync`
  and `sync / attest` both `skipped`), so it does **not** exercise the
  `permission-workflows: write` mint or the auto-merge arming. The live
installation read in the Summary is the evidence for the mint half. It
also
  gives **zero attestation signal** — `attest` and `sync` both carry
`if: ${{ !inputs.dry-run }}`, so neither runs under dispatch. What it
does
prove, beyond the input contract: the engine file PARSES at `ac223bb`,
which
is what clears the anchor/alias refactor — an unresolvable `*install-yq`
would
  be a `startup_failure` with no job running at all.
- `npm run test:runner-policy` (238 pass, 0 fail) and `npm run
lint:runner-policy`
  ("Runner policy passed.") after the allowlist entry was added.
- Repository CI (`ci-status` and the required gates), including the
  `pin-comment-convention` component, which mechanically validates the
`# <short-sha> <date>` provenance comment against the pin it annotates.

## Related

- ci-workflows#284 — the engine change this pin adopts (`ac223bb`).
- ci-workflows#213 — the auto-merge arming this pin also adopts
(`dd45dac`).
- ci-workflows#205 — the yq anchor/alias refactor (`e8e5f3f`),
behavior-neutral.

No linked issue.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Jul 29, 2026
…ents (#286)

## 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 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 (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. **#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. **#289 (re-pin the sync engine at `ac223bb`) — OPEN.** #290's stated
merge
order is #290#289#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 #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 #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: #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](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 14, 2026
)

## Summary

`approvedReusableWorkflowContracts` could waive an *exact* caller
permission set
but could not require a **minimum** one, so a consumer repinning to a
reusable
that newly requests a read scope passed `Runner policy` while granting
less than
the callee needs. This adds the missing term,
`minimumCallerPermissions`, and
backfills it onto the three `7107b34` gate contracts.

`allowedCallerPermissions` is unchanged — still an exact-match waiver
the
validator refuses unless it carries at least one `write`. No existing
contract
entry, validator branch, or check is relaxed or removed.

The floor may only require `read`. That is the resolution of the Codex
P2 on
this PR, and it is what makes the two fields exactly complementary
rather than
overlapping — reasoning below.

### The term, and why it is separate rather than a relaxation of the
waiver

The two fields answer different questions and neither implies the other.

| | `allowedCallerPermissions` | `minimumCallerPermissions` |
|---|---|---|
| Direction | ceiling — the exact set a caller may present | floor — the
least a caller must grant |
| What it does | **waives** the ordinary read-only caller boundary so a
reviewed workflow may hold a privileged grant | **grants nothing, waives
nothing**; records what the callee's own `permissions:` block requests |
| Values | must contain at least one `write` | `read` only — a write
floor is rejected at policy load |
| Match semantics | exact, per scope | ordered, per scope; more passes |

The two are exactly complementary, and that is not incidental — see the
Codex P2
resolution below. Every **write** obligation already has a home: the
waiver,
reviewed against the calling job, which is the route #384 just used for
`zizmor.yml@7107b34`'s `security-events: write`. Every **read**
obligation now
has one too. Merging them into a single field was considered and
rejected: the
write requirement on the waiver is load-bearing — it is what makes the
field a
*privilege* waiver rather than a mirror of the callee's declared
permissions —
and the obligation #383 was filed about is entirely read, exactly the
shape that
requirement makes inexpressible.

### Codex P2, resolved by construction: no write floor can exist

A floor that could require `write` would be unsound. GitHub downgrades a
caller's write grants to read — and write-only scopes to none — on
**forked**
and **Dependabot** pull requests unless repository settings permit
otherwise, so
a caller's declared `write` is not the access the callee receives. A
write floor
compared against the YAML declaration would pass exactly the callers it
exists
to catch.

Rather than model event-time downgrades — the policy can read neither
repository
settings nor fork/Dependabot context at validation time, so such a check
would be
a guess dressed as a check — the term is **restricted to `read`
values**. That
disposes of the finding by construction rather than by argument: no
write floor
can be declared, so none can be silently downgraded.

The rule lives in the validator rather than the schema so the author of
a
rejected contract is told *why*. That is the split
`allowedCallerPermissions` already uses in the opposite direction — the
schema
permits an all-read waiver, and the validator rejects it with "must
include at
least one write permission". Verified behavior, not intended: running
`validatePolicy` over the real `policy.json` with the floor on
`semantic-pr@7107b34` mutated gives

```text
REJECTED {"actions":"write"}
  reusable workflow contract <ref>.minimumCallerPermissions must require read access only
  (actions); GitHub downgrades caller write grants on forked and Dependabot pull requests,
  so a write floor cannot be proven from the caller's declaration — use
  allowedCallerPermissions for a write obligation
REJECTED {"id-token":"write"}                  … must require read access only (id-token); …
REJECTED {"pull-requests":"read","contents":"write"}  … must require read access only (contents); …
REJECTED {"actions":"none"}     <ref>.minimumCallerPermissions.actions must NOT be valid
REJECTED {"id-token":"read"}    <ref>.minimumCallerPermissions.id-token must be equal to one of the allowed values
ACCEPTED {"actions":"read"}
```

The last two are the schema's own value domain: a floor value must name
a real
grant, and `id-token` is a write-only scope with no `read` level to
require.

### How permission comparison is ordered

The restriction is on what a contract may **require**, not on how grants
compare — the ordered comparison is unchanged. GitHub access is ordered
`none` < `read` < `write`, and a called workflow can only *downgrade*
the
caller's `GITHUB_TOKEN`, never elevate it ([reusable-workflow
docs][rw]), so the
check remains a floor, not a match:

- a caller granting `write` where the contract requires `read`
**passes**;
- extra scopes the contract does not name **pass** — the floor says
nothing
  about them;
- `read-all` and `write-all` both clear a read floor — and each still
clears it
after an event-time downgrade lands at `read`, which is precisely why a
  read-only floor is sound where a write floor is not;
- a scope the caller does not name is granted nothing and **fails**;
- effective job permissions that are **omitted** fail closed — they
resolve to
repository- or organization-defined defaults this policy cannot read, so
they
  can never *prove* the floor.

Job-level permissions override workflow-level, so the comparison runs
against
`effectivePermissions(workflow, job)`, the same surface the existing
waiver
check uses.

`write-all` clearing the floor is arithmetic, not absolution: the floor
waives
nothing, so a `write-all` caller still meets the ordinary
`privileged-control-plane` rules. A regression test asserts exactly
that.

A contract naming **both** fields is checked for satisfiability when the
policy
loads: because the waiver is the only mapping such a caller may present,
a
waiver falling short of its own floor would admit nothing at all, so it
is
rejected as a configuration error rather than left to fail silently at
every
call site.

### Auto-approval: deliberately *not* a decline category

`selectorResultInput`, `allowedCallerPermissions`, and a nonempty
`allowedSecrets` each decline Dependabot auto-approval unconditionally,
because
each is trusted for something the surface diff never inspects — what the
callee's *steps* do. `minimumCallerPermissions` is the opposite kind of
term: it
says nothing about steps, only what the callee's `permissions:` block
requests,
and that block is already part of the compared surface. A bump that
changes it
is declined by the diff; a bump that does not carries the same floor. So
the
term is **added to `reviewedContractSurface`** — two surface-matching
bases
holding different floors must still be caught as ambiguous, and there is
a test
for that — but **not** to the decline list.

(Moot for the three entries here: all three carry `selectorResultInput`
and are
already declined unconditionally.)

### Correction to the backfill list I was given

The task brief said `semantic-pr.yml` and `do-not-merge-gate.yml`
declare only
`actions: read`. **They do not.** Fetched from
`melodic-software/ci-workflows`
at `7107b34832a7b6db5d08d3b132621c599fbe5e50`, each of the three
declares
exactly one workflow-level `permissions:` block, with no job-level
override
anywhere in the file:

| Reusable | `permissions:` at `7107b34` | Backfilled floor |
|---|---|---|
| `semantic-pr.yml` (L93) | `pull-requests: read`, `actions: read` |
both |
| `do-not-merge-gate.yml` (L45) | `pull-requests: read`, `actions: read`
| both |
| `pr-issue-linkage.yml` (L61) | `pull-requests: read`, `actions: read`
| both |

This matches the table already recorded in #382's own body, so the
brief's list
was the outlier. Each floor is the callee's whole declared set: the
callee
narrows to that set, so a caller granting any less starves it.

## Blast radius

**No consumer breaks today. Nothing in the fleet needs a change before
or after
this merges.** Verified against the **live default branches** via `gh
api`, not
local clones.

The floor is keyed to `path@SHA`, so only callers pinned at `7107b34`
for these
three paths are governed at all.

| Repo | Runs `runner-policy`? | Pins at `7107b34` for these three? |
Effect |
|---|---|---|---|
| `provisioning` | **yes** — `.github/runner-policy.json` and the
managed materialization both present | **yes**, all three (converged by
melodic-software/provisioning#284) | **governed and compliant** — each
calling job already grants `pull-requests: read` **and** `actions: read`
|
| `ci-runner` | **no** — neither `.github/runner-policy.json` nor
`.github/standards/runner-policy/` exists (HTTP 404 on both) | yes, all
three | none; and its caller jobs already grant both grants too, so it
would pass if it adopted the component |

`provisioning` was re-verified after #284 merged mid-flight, against its
**live
default branch**, in two ways: `gh api` on all three caller files,
confirming the
`7107b34` pin and both `read` grants on each calling job; and the
component from
**this branch** run over a `git archive origin/main` export of that tree
with
real owner evidence —

```text
GITHUB_REPOSITORY=melodic-software/provisioning CI_REPOSITORY_VISIBILITY=private \
  node components/runner-policy/runner-policy.mjs --root <provisioning@origin/main> \
    --policy <this branch>/components/runner-policy/policy.json
Runner policy passed.
```

Identical result under `origin/main`'s policy, so this PR changes
nothing for
it.

**What the gate actually buys, then:** every caller still on an older
SHA —
`standards` itself, `.github`, `dotfiles`, `medley`, `github-iac`,
`claude-code-plugins`, `codex-plugins` — grants `permissions: {}` or
`pull-requests: read` on its gate jobs. Each of those now fails
**pre-merge, in
the repin PR**, instead of at workflow startup or with a runtime 403 in
the
cancelled-prerequisite resolver. `provisioning` reaching the same end
state by
hand, in #284, is the case for the gate rather than against it: nothing
forced
that convergence to include the grants, and nothing would have caught it
had it
not. That is why the change lands with zero present-day breakage — it
catches
the *next* repin, not the current state.

Delivery is gated too: `components/runner-policy/policy.json` is a
`managed`
component in `distribution/sync-manifest.yml`, so the tightened policy
reaches
each consumer through a reviewed sync PR, never at this PR's merge.

## Test plan

Real results, run on this branch, rebased onto `origin/main` at
`0fb6464`
(post-#384).

- `node --test components/runner-policy/runner-policy.test.mjs` — **272
pass,
0 fail**. `origin/main` measured the same way (`git archive origin/main`
into
  a clean tree) is **264 pass, 0 fail**; +8 net tests, three of them
  table-driven case sets.
- `npm run lint:runner-policy` — **Runner policy passed.**
- **Schema:** `validatePolicy` compiles `policy.schema.json` with Ajv
2020
  (`strict: true`) on every run above, so the lint and test runs are
schema-validating runs. `githubMinimumPermissionMap` was additionally
probed
directly against the component's own Ajv 8.20.0 with its real options
before
  being adopted: it compiles clean under `strict: true`, accepts
  `{"actions":"read"}`, and rejects `{"actions":"none"}`, `{}`,
`{"id-token":"read"}`, `{"models":"write"}`, and unknown scopes —
inheriting
the whole 17-scope table and its per-scope constraints through `$ref`
rather
than duplicating them. The read-only rule sits in the validator, not
here, for
  the message-quality reason given above.
- `npm run lint:md` — 0 issues, 112 files. `lefthook run pre-commit` —
typos,
  editorconfig, gitleaks, markdownlint, biome all pass.
- Neighbouring components, to show nothing cross-broke: `test:packages`
14/14,
  `test:concurrency-policy` 24/24, `test:dependabot-policy` 35/35,
  `test:pr-convention-policy` 10/10, `test:lefthook-dotnet` 12/12,
  `lint:hooks` "All good", `lint:concurrency-policy` and
`lint:dependabot-policy` pass. (`lint:pr-convention-policy` fails
identically
on `origin/main` in this environment — its npm script self-checks with a
`$(cat …)` substitution Windows `cmd` does not expand. Untouched here.)

### Proof the new validation bites

The unit tests cover the semantics; this is the end-to-end proof against
the
**real** backfilled `policy.json` and this repository's **own real
callers**.
`standards`' `.github/` at `origin/main` was exported to a scratch root,
the
three gate callers repinned to `7107b34`, only the caller job's
`permissions:`
varied, and `auditRepository` run with auto-approval disabled and
`fetch`
stubbed to throw so nothing could pass by network.

Against **this branch's** `policy.json`:

```text
### today's grant, repinned to 7107b34            (permissions: pull-requests: read)
  .github/workflows/do-not-merge.yml [runner-target-contract] reusable workflow caller permissions.actions must grant at least "read"; the grant is "none"
  .github/workflows/pr-issue-linkage.yml [runner-target-contract] reusable workflow caller permissions.actions must grant at least "read"; the grant is "none"
  .github/workflows/pr-title.yml [runner-target-contract] reusable workflow caller permissions.actions must grant at least "read"; the grant is "none"

### exactly the reviewed minimum                  (pull-requests: read, actions: read)
  (no findings)

### more scopes than the minimum                  (read-all)
  (no findings)

### higher access than the minimum                (pull-requests: write, actions: read)
  (no findings)

### no grant at all                               (permissions: {})
  .github/workflows/do-not-merge.yml [runner-target-contract] reusable workflow caller permissions.actions must grant at least "read"; the grant is "none"
  .github/workflows/pr-issue-linkage.yml [runner-target-contract] reusable workflow caller permissions.actions must grant at least "read"; the grant is "none"
  .github/workflows/pr-title.yml [runner-target-contract] reusable workflow caller permissions.actions must grant at least "read"; the grant is "none"
```

Against **`origin/main`'s** `policy.json`, those same five scenarios
produce
`(no findings)` every time — including both under-granted ones. That is
the gap
this PR closes, reproduced rather than asserted.

The check reports the **first** shortfall in sorted scope order,
matching
`exactCanonicalMap`'s existing first-failure style, which is why the
`permissions: {}` scenario names `actions` and stops rather than also
listing
the equally-missing `pull-requests`.

### New tests

- `an all-read minimum admits a caller granting exactly it` — the case
that is
  inexpressible on `main`.
- `a caller granting more than the minimum clears it` — a `write` grant
against
  a `read` floor, on a contract carrying **both** terms. This is the
  load-bearing proof that the ordered comparison survived the read-only
restriction: only what a contract may *require* narrowed, not how grants
are
  compared.
- `a caller granting less than the minimum is rejected` — omitted scope,
  explicit `{}`, and an unrelated scope granted instead.
- `a minimum caller permission floor cannot require write access` —
`write` on
a read/write scope, `id-token: write`, and a mixed map with one write
value;
  all rejected at config-validation time. **New for the P2.**
- `read-all and write-all callers both clear a read floor`, asserting
that
  `write-all` is nonetheless still caught by the privileged rules.
- `a caller with no explicit permissions cannot prove a minimum`.
- `a minimum caller permission scope must name a real read grant` —
`none`,
  unknown scopes, `{}`, and `models: write`.
- `a contract naming both caller-permission terms must be satisfiable`,
re-targeted to a read floor the waiver omits entirely (the previous
`write`
  floor is no longer a legal contract).

One row was added to the existing `Dependabot SHA bump declines
ambiguous
surface-matching reviewed contracts` table so two bases differing
**only** in
`minimumCallerPermissions` are proven to be caught.

### Not done here, deliberately

- **`hosted-only` contracts do not get the term.** Not an oversight and
not
scope-trimming: `reusableWorkflowStatus` returns approved for
`hosted-only`
before any permission check runs, so extending the floor there means
extending
that path — a behavior change to a routing mode with no consumer in this
  issue. It belongs in its own change with its own review.
- **No consumer repins.** Nothing downstream needs one; see Blast
radius.
- **`ci-runner`'s callers pass no `runner` input**, so they would fail
the
`runner-input` contract for an unrelated, pre-existing reason if
`ci-runner`
ever adopts the component. Already flagged in #382; unchanged by this
PR.

### Proposal, not implemented: should the term be required?

Raised rather than built, per the brief. **Recommendation: yes
eventually, as a
registration-time check with a migration first — not now.**

The honest shape of the rule, after the read-only restriction, is "a
`runner-input` contract must record the **read** scopes of its callee's
`permissions:` block as its floor" — the callee's write scopes stay the
waiver's
business, as `link-check`'s `issues: write` and `zizmor`'s
`security-events: write` already are. Requiring it today would
invalidate every
existing entry at once, including several this PR does not touch, and
the check
cannot derive the callee's block itself without a network fetch at
policy-load time, which the module deliberately does not do outside the
auto-approval path. The workable sequence is the one #382 proposed for
untagged
SHAs: backfill the floor onto the remaining entries first, then add the
requirement as a registration-time check on
`approvedReusableWorkflowContracts`, so the failure lands on whoever
adds an
entry rather than on every downstream consumer simultaneously. Worth its
own
issue once the backfill is complete.

Closes #383

## Related

- #382 — registered the `7107b34` entries and recorded this gap in its
own body
  as a known limitation; this PR closes it
- #384 — waived `zizmor.yml@7107b34`'s `security-events: write` through
`allowedCallerPermissions`; this branch is rebased on it, and it is the
worked
  example of the write half of the ceiling/floor split
- #381 — the convergence effort those entries exist to unblock
- melodic-software/provisioning#284 — converged provisioning's three
gate
callers to `7107b34` with both `read` grants while this PR was open; the
  blast-radius table reflects the post-merge state
- melodic-software/ci-runner#250 — the adjacent, worse-behaved
caller-contract
failure mode: rejected at startup, with the required check emitting no
context
  at all
- ci-workflows#458 — the cancelled-vs-`timed_out` prerequisite resolver
behind
  the `actions: read` additions

[rw]:
https://docs.github.com/en/actions/reference/workflows-and-actions/reusable-workflows

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_013yvHrEronHPoznT1b3HtN5

---------

Co-authored-by: Claude Opus 5 (1M context) <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.

1 participant