feat(concurrency-policy): add workflow concurrency analyzer component

[kyle-sexton]

Summary

Adds concurrency-policy, a synced analyzer component that enforces the canonical top-level concurrency block on every pull-request-triggered workflow, so a superseding push cancels the in-flight run before it consumes a runner slot while pushes to the default branch and scheduled runs are never cancelled. Part of the volume-control workstream (melodic-software/github-iac#78).

concurrency:
  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
  cancel-in-progress: true

Corrected canonical. This encodes pull_request.number || run_id, not the head_ref || run_id form in the issue text. The rationale — verified against official GitHub docs — is recorded as a comment on the issue (#151 (comment)) rather than a silent body edit: head_ref is a fork-controllable branch name that collides across same-named branches from different head repositories on pull_request_target; the pull-request number is unique and trusted; both keep the identical never-cancel-push/schedule invariant (head_ref is empty on those events). The fleet already uses this variant on 14 workflows.

A concurrency block is an embedded workflow fragment, not a byte-syncable file, so this mirrors runner-policy: a byte-identical analyzer + schema materialize into consumers while each repository owns its exceptions locally.

Shared-component admission evidence

Per docs/component-lifecycle.md:

• Live consumer — standards itself. The new concurrency-policy CI job runs the analyzer (node components/concurrency-policy/concurrency-policy.mjs --root .) against this repository's own workflows and is aggregated into the required ci-status gate.
• Owner and outcome — Owner: standards maintainers. Problem: superseded pull-request runs fan out and exhaust the 2-machine fleet (a compound cause of the 2026-07-16 backlog, github-iac#78). Acceptance: every pull-request-triggered workflow carries the canonical block or a reasoned exception, and the gate blocks drift. Rollback: remove the component entry and the CI job.
• Delivery boundary — Exact materialization of a byte-identical analyzer through this repository; the config it checks stays locally-owned per consumer. A concurrency block cannot be byte-synced (embedded fragment), a reusable workflow cannot set a caller's top-level concurrency, and there is no organization control-plane setting for it — the analyzer is the only boundary that keeps one source of truth for the pattern.
• Alternatives and overlap — A prose convention was rejected: the convention tier is adopt-by-copy, outside the sync loop, so it cannot satisfy the issue's "roll out via sync" and provides no gate. Folding into runner-policy was rejected for cohesion: that component's purpose is runner selection and it never inspects concurrency, so there is no finding overlap.
• Operational fit — Static file parse; no network, credentials, or privileged access. Runs in under a second against this repository's six workflows. Deterministic pass/fail, fail-closed on parse errors and unknown config. OS-independent on Node >= 24. False-positive surface is bounded to one canonical pattern and one exception reason.
• Upstream health — Runtime dependencies ajv@8.20.0 and yaml@2.9.0, the exact pins already vetted for runner-policy (byte-identical lockfile tree, 6 packages). No other dependencies.
• Legal and security — ajv and yaml are MIT-licensed. The analyzer is read-only with no data or credential access. Boundaries, fail-closed behavior, and review triggers are in components/concurrency-policy/THREAT-MODEL.md.
• Update path — The new npm dependency root is registered in .github/dependabot.yml (/components/concurrency-policy: weekly, cooldown 7, limit 5, grouped), itself conforming to the organization Dependabot policy.
• Verification — 21 behavioral cases (concurrency-policy.test.mjs) covering canonical pass, group and cancel drift, the head_ref variant, exception consumption and inventory drift, fail-closed schema, and YAML edge cases (quoted/unquoted, array/mapping on:, symlinks, duplicate keys). CI runs the same --root . entrypoint consumers use.

Enforcement rollout. Blocking from the start: the sole live consumer (standards) is already clean — one recorded delegated-job-level exception for claude-review.yml, whose reusable enforces concurrency at job level — so no observation period is needed. Downstream consumers move to blocking in their own integration PRs after a clean run.

Concurrency drift (org-wide audit, informational)

Audited 86 workflow files across 10 repos; 34 are pull-request-scoped (the target set). Zero workflows use the dangerous head_ref-with-no-fallback pattern. 20 already conform to intent (14 via the pull_request.number variant + 6 claude-review job-level delegations). Drift the standard will catch:

• 6 github.ref-based groups on workflows that also run on push/schedule, so default-branch or scheduled runs can supersede-cancel each other: provisioning/ci.yml, dotfiles/ci.yml, medley/ci-status.yml, medley/onboard-drift.yml, medley/actions-lint.yml, medley/osv-scanner.yml. Highest concern: medley/ci-status.yml (the aggregating required-check gateway — a second push to main cancels the in-flight one) and medley/onboard-drift.yml (a main push and the scheduled run share …-refs/heads/main).
• 1 missing entirely: medley/comment-review-gate.yml (multi-event; needs a scoped block).
• 7 cosmetic pull-request-only deviations with no push/schedule trigger (safe; no default-branch run to cancel).

Per the workstream split, this PR delivers the standard and the gate only. The medley and consolidation-lane workflow remediations are not patched here — they are owned by ci-workflows#122.

Per-consumer rollout (follow-up integration work)

The 9 requires-ci repositories (each has an active required_status_checks ruleset rule). standards is wired in this PR; each of the other eight needs a separate integration PR: manifest enrollment (materializes the analyzer), a locally-owned .github/concurrency-policy.json if it needs any exception, the CI job, and a dependabot entry for the analyzer lockfile.

• melodic-software/standards — dogfooded in this PR
• melodic-software/.github
• melodic-software/ci-runner
• melodic-software/ci-workflows
• melodic-software/claude-code-plugins
• melodic-software/dotfiles
• melodic-software/github-iac
• melodic-software/medley
• melodic-software/provisioning

The component is registered in distribution/sync-manifest.yml but enrolled in no target yet, so nothing materializes until each integration PR adds it to that target's managed set. knowledge-corpus, songwriting, and ci-runner-canary have no required CI and no pull-request workflows, so they are out of scope.

Test plan

• npm run test:concurrency-policy — 21/21 passing
• npm run lint:concurrency-policy — passes against this repository (with the one claude-review.yml exception)
• npm run lint:runner-policy — still passes with the new CI job present
• biome, markdownlint, and sync-manifest.sh validate — clean

Closes #151.

[claude]

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

---

Code Review

• Gather context and understand the request
• Read changed files
• Review analyzer implementation (concurrency-policy.mjs)
• Review test coverage (concurrency-policy.test.mjs)
• Review schema, config, and manifest changes
• Review CI integration and dependabot changes
• Review documentation (README, THREAT-MODEL)
• Final review posted

---

Overall this is a well-constructed, security-aware component. No correctness bugs, no security issues. The fail-closed design, strict schema, dual-parse duplicate-key preflight, and sym-link rejection all hold up. A few things below worth discussing, ranging from a genuine gap to nits.

---

1. Unreachable deduplication block — dead code

concurrency-policy.mjs L339–350

const uniqueFindings = new Map();
for (const item of findings) {
  const key = [item.file, item.rule, item.message].join("\0");
  if (!uniqueFindings.has(key)) {
    uniqueFindings.set(key, item);
  }
}

In the current code paths, duplicates cannot occur:

• The records Map has one entry per workflow path, so the first loop fires at most once per file.
• concurrencyFindings returns at most two distinct (file, rule) tuples per call.
• The exception loop iterates Object.keys(exceptions) — unique JSON property names (guaranteed by uniqueKeys: true preflight) — so each exception key fires at most once.
• Duplicate exception keys in the config JSON throw a ConfigurationError before any findings accumulate.

The dedup block is defensive-for-future-proofing but adds cognitive overhead: a reader seeing it will wonder "when does this fire?" and won't find an answer. Either document the invariant that keeps it from firing and keep the code as an explicit guarantee, or remove it and add a brief comment explaining why duplicates can't occur. If the intent is resilience against future logic changes, a terse comment stating that would clarify the purpose.

---

2. Missing test coverage: CLI argument parsing and --json mode

The parseArguments function and the --json output path have no test coverage. The behavioral audit of auditRepository is thorough, but if someone changes argument parsing (say, swapping the pre-loop argv.includes("--json") to something else), there's no test to catch a regression.

Key uncovered scenarios:

• --json produces { findings, ok } on stdout (not stderr) when there are findings
• --root missing its value throws ConfigurationError with a useful message
• An unknown flag throws rather than silently ignoring
• The "unparsable workflow that also has an exception" path in auditRepository (L323–326) — the code says the exception is intentionally left unconsumed without a second report; there's no test exercising that combined state

None of these block the PR, but adding CLI tests to concurrency-policy.test.mjs would complete the behavioral coverage and make the parseArguments contract explicit.

---

3. workflow-unparsable rule used for a fundamentally different failure (symlinks)

concurrency-policy.mjs L238–242
concurrency-policy.mjs L279–285

A symlink detection stores { file, error: "must be a regular file; workflow symlinks are forbidden" }, then the main loop emits it under the rule workflow-unparsable with the prefix "workflow could not be parsed, so its concurrency block cannot be verified: …". The resulting CI output reads:

.github/workflows/link.yml: workflow-unparsable: workflow could not be parsed, so its concurrency block cannot be verified: .github/workflows/link.yml must be a regular file; workflow symlinks are forbidden

Two issues: the file path appears twice, and "could not be parsed" is factually inaccurate when the real problem is "this is a symlink." A distinct rule (workflow-symlink) and a direct message would make the output actionable without the redundancy. This is a UX concern, not correctness — the finding fires and the gate blocks, which is correct.

---

4. Redundant --json pre-check in parseArguments (nit)

concurrency-policy.mjs L357
concurrency-policy.mjs L363–366

const options = {
  json: argv.includes("--json"),   // ← sets it once upfront
  ...
};
for (...) {
  if (argument === "--json") {
    options.json = true;           // ← sets it again in the loop
    continue;
  }

The argv.includes call is a redundant pre-check; the loop assignment is what matters. Harmless, but the duplication would be confusing if someone added logic between them. Removing the upfront initializer and letting the loop set it (or vice-versa) would be cleaner.

---

5. Exception schema allows only delegated-job-level — schema is future-hostile by design (observation, not a finding)

concurrency-policy.schema.json L31

The reason enum is intentionally narrow: { "enum": ["delegated-job-level"] }. This is the right posture (unknown reasons fail closed at schema time, as the test at L283–293 in the test file verifies). The THREAT-MODEL explicitly lists "adding a new exception reason" as a review trigger. Just confirming this is intentional — it is, and it's correct.

---

Confirmed correct

• Double-parse preflight (parseUniqueJson): YAML strict-mode catches duplicate keys per RFC 8259 §4, then JSON.parse is the authoritative parse. This is the correct layering and is well-documented in the threat model.
• Canonical regex: whitespace tolerance inside ${{ }} and around || is correct; the - between the two expressions is treated as literal (no whitespace tolerance there), which matches GitHub's expression evaluation.
• conformsToCanonical vs concurrencyFindings: their apparent redundancy is justified — the former is a predicate for the exception check; the latter generates specific per-property findings. They serve different call sites.
• CI integration: concurrency-policy job correctly follows the select-runner pattern; correctly enrolled in ci-status needs and RESULTS aggregation.
• Sync manifest: only ships mjs, schema.json, package.json, and package-lock.json — no test file or docs materialized, matching runner-policy precedent.
• Dependabot entry: matches organizational policy (weekly, 7-day cooldown, limit 5, grouped minor/patch).
• .github/concurrency-policy.json exception: justification is substantive and technically correct — the reusable enforces concurrency at job level; a caller-level group of the same name would deadlock the called job.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 31c3d48f9e

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

[kyle-sexton]

Addressed the Codex P2 in 28979c8: the delegated-job-level exception now waives only the missing-block case. If an excepted workflow carries a present top-level block, that block is validated regardless of the exception (so a non-canonical or unsafe block is still flagged), and the now-unnecessary exception is reported as exception-inventory-drift. Added a regression test; suite is 22/22.

[claude]

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

---

Code Review (follow-up — 28979c8)

• Gather context
• Read diff of 28979c8
• Review updated concurrency-policy.mjs
• Review updated test file
• Post findings

---

Codex P2 fix — confirmed correct

The fix is semantically sound. Here is the full behavioral matrix after the change:

Excepted workflow state	Before	After
No top-level block	Pass (exception waives)	Pass (exception waives) — unchanged
Canonical block	exception-inventory-drift	exception-inventory-drift + no concurrency findings — unchanged
Present but non-canonical block	Pass (silent bypass — the bug)	exception-inventory-drift + specific concurrency finding(s) — fixed
Malformed block	Pass (silent bypass)	exception-inventory-drift + concurrency-malformed — fixed

The condition that drives the fix — topLevelConcurrency(record.workflow).present — is the right predicate. It lets the exception waive exactly what it should waive (absent block) while guaranteeing that any block that is present is held to the same standard as non-excepted workflows.

conformsToCanonical removal is clean. The grep confirms no remaining references; its only call site was the branch now replaced by the .present check. The predicate function's responsibility has dissolved into the call to concurrencyFindings, which already computes canonical conformance.

Test correctness. The new test ("an exception does not license a present non-canonical block") uses github.ref as the stale group. It expects concurrency-group-drift + exception-inventory-drift via rules() (which sorts alphabetically — c before e, so concurrency-group-drift comes first). The push order in the code is exception-inventory-drift first, then concurrencyFindings; the .sort() in rules() reconciles this. The assertion is correct.

Existing test preserved. The "already-conformant" case (ci.yml with canonical block + exception) continues to produce exactly [exception-inventory-drift] and nothing else, because concurrencyFindings returns an empty array for a canonical block. That test is still valid and passes.

---

One minor observation (not blocking)

The updated exception-inventory-drift message — "declares a concurrency-policy exception but carries a top-level concurrency block; remove the exception or the block" — is now shared between two distinct situations: (a) a canonical block that no longer needs the exception, and (b) a non-canonical block that also has an exception. For case (a), "remove the exception or the block" is slightly odd because removing the block is clearly wrong; the old message ("remove the unnecessary exception") was more precise. This is a copy nit — the finding fires correctly in both cases and the accompanying concurrency findings in case (b) make the action clear.

---

Status on original review findings

• feat(powershell): PowerShell static-analysis module #1 (dedup dead code), feat(markdown): Markdown (GFM) static-analysis module #2 (CLI/--json test coverage), feat(hygiene): add editorconfig base + checker module #3 (symlink rule naming), feat(typos): add typos spell-check module #4 (redundant --json pre-check) — still open, unchanged by this commit. All were marked non-blocking in the original review and remain so.
• test(typos): cover all four inline ignore directive forms #5 (schema enum observation) — confirmed intentional, no action needed.

The P2 fix is correct and complete. The PR is in good shape.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 28979c866e

ℹ️ About Codex in GitHub

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

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

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

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

[claude]

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

---

Code Review (follow-up — 62433b1)

• Gather context
• Read diff of 62433b1
• Review updated concurrency-policy.mjs
• Review updated test file
• Post findings

---

Codex P2 fix — confirmed correct

The fix is sound. Walking the full behavioral matrix:

Concurrency form	keys value	extra-key check
Absent (undefined)	[]	Never reached (present: false → early return)
String shorthand	[]	Empty filter — no finding (correct: a YAML scalar has no keys)
Mapping with extra key	["group", "cancel-in-progress", "queue"]	extraKeys = ["queue"] → concurrency-extra-keys
Array / null (malformed)	[]	Never reached (malformed: true → early return)

The keys propagation is minimal and placed in exactly the right spot in each of topLevelConcurrency's four return branches.

Extra-key check fires independently of group/cancel drift. All three checks (L207–241) are sequential if blocks, not else if, so a mapping with a bad group, cancel-in-progress: false, and an extra key produces all three findings. That is the correct behavior.

Test correctness. CANONICAL + " queue: max\n" produces a block with canonical group and cancel-in-progress: true plus queue: max. The expected result is exactly [".github/workflows/ci.yml:concurrency-extra-keys"] — no group-drift or cancel-missing because those values are canonical. The assertion is correct.

README is updated to document concurrency-extra-keys, consistent with the existing rule entries. The example queue is the best illustration (it is the key GitHub specifically supports, making the "why we still reject it" reasoning directly actionable for a reader who would be confused).

---

Status on prior review findings

• feat(powershell): PowerShell static-analysis module #1 (dedup dead code), feat(markdown): Markdown (GFM) static-analysis module #2 (CLI/--json test coverage), feat(hygiene): add editorconfig base + checker module #3 (symlink rule naming), feat(typos): add typos spell-check module #4 (redundant --json pre-check) — unchanged by this commit. All remain non-blocking.
• Codex P2 (extra keys) — fixed and correct.
• Codex P2 (exception bypass, 28979c8) — fixed and correct (confirmed in prior review).
• test(typos): cover all four inline ignore directive forms #5 (schema enum — observation) — confirmed intentional, no action needed.

The PR is in good shape. All blocking issues have been addressed.