docs(claude-lanes): narrow the queue/cancel-in-progress claim to its true form - #294
Conversation
…true form The code-review caller's job-level comment said `queue:` cannot share a concurrency block with cancel-in-progress. GitHub's prohibition is value-level, not key-level: only `queue: max` + `cancel-in-progress: true` is rejected. Narrow the claim to the documented pair and add the second, independent reason the blocks stay separate — this group is job-scoped and repo-wide, not per-PR — so a reader who sets `cancel-in-progress: false` does not conclude the two blocks may merge. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 16s —— View job Review: docs(claude-lanes): narrow the queue/cancel-in-progress claim to its true form
Verdict: LGTM — no issues found. What the diff doesThe comment at
That is a key-level prohibition — any
This is value-level, matching the GitHub docs citation in the PR body verbatim. Claim verification
REVIEW.md criteriaThis is a comment-only change (+5/-3, no YAML key, value, or group expression touched). The two always-check criteria in Minor observation (non-blocking)The PR body notes a deliberate non-change: the The correction is precise, the reasoning is sound, and the test plan confirms 30/30 passing. Ready to merge. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2a8228e0d
ℹ️ 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".
… bytes This component is materialized verbatim into consumer workflows, so a primary source that lives only in the commit message does not reach the maintainers who read the claim. Put the authoritative GitHub URL at the sentence that defers to it, per conventions/engineering/documentation-and-citations.md. The job-level anchor is cited rather than the workflow-level one because the comment annotates a `jobs.<id>.concurrency` block; both anchors were confirmed to resolve and to carry the rule sentence. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 32s —— View job Claude finished reviewing the updated PR —— Review: follow-up commit
|
Summary
components/claude-lanes/claude-review.ymljustified its job-level queue blockwith a key-level claim:
GitHub's prohibition is value-level, not key-level. Verbatim, from the
reusable that renders into both
#concurrencyand#jobsjob_idconcurrency:Same file, restated in the example prose:
github/docs@336b7f5data/reusables/actions/actions-group-concurrency.md(lines 20 and 126).
https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#concurrency
queue: max+cancel-in-progress: falseis legal and runs — the key-levelwording forbids a shape GitHub permits.
The correction narrows the claim to the documented pair and adds the second,
independent reason the two blocks stay separate: this group is job-scoped and
repo-wide, not per-PR. Without that clause a reader who sets
cancel-in-progress: falsecould conclude the blocks may now be merged, whichwould silently collapse the per-PR supersede group into the repo-wide queue.
The authoritative URL is cited inline, in the tracked bytes (
565559a,added after review). Because this component is materialized verbatim
downstream, a citation living only in a commit message never reaches the
maintainers who read the claim — a correct-but-uncited assertion would repeat
this PR's own failure mode at lower severity. The link sits at the sentence that
defers to it per
conventions/engineering/documentation-and-citations.md, andit is a living URL rather than a pinned snapshot because the same convention
prefers fetching at read time over storing a snapshot with no recheck trigger.
The job-level anchor (
#jobsjob_idconcurrency) is cited rather than theworkflow-level one because the comment annotates a
jobs.<id>.concurrencyblock; both anchors were confirmed live to exist on the rendered page and to
carry the rule sentence.
Why this races the open sync PRs
This component is sync-managed: its bytes are copied verbatim into every
consumer's
.github/workflows/claude-review.yml. The wording has not landedanywhere yet, and four open sync PRs are carrying it right now.
Regenerated from commands, not recalled:
Managed targets:
claude-code-plugins,dotfiles,github-iac,medley,provisioning.Each consumer's live default-branch file was fetched and counted — the file was
read, not grepped on a ref for text expected to be there:
queue: maxon default branchclaude-code-pluginsdotfilesdf035f9) — yesgithub-iac0b8124d) — yesmedleyd107862) — yesprovisioning7b9d016) — yesLanding this before those PRs merge means the correct text reaches every
consumer on first contact. Landing it after means a false statement propagates
fleet-wide and needs a second sync to retract.
Hold on the four sync PRs until this merges. The rollout-window gate is
intact —
grep -c '^ automerge: false$' distribution/sync-manifest.ymlreturns
8against8total targets — and each of the four PRs was queriedlive (
gh pr view --json autoMergeRequest): none is armed. So nothingmerges them without a human, and a human merging any of them before #294 lands
is the only thing that defeats this PR.
Merging #294 does not require closing them. Verified in the engine at the
SHA this repository pins (
ci-workflows@ac223bb,.github/workflows/standards-sync.yml:454-461): it usespeter-evans/create-pull-requestagainst a fixedbranch: chore/standards-sync,which is the head branch on all four PRs — so a subsequent real run refreshes
each existing PR in place rather than opening a new one, which is also why the
engine guards auto-merge arming on
pull-request-operation == 'created'.sync.ymlruns onpush: branches: [main], so merging this PR is itself therefresh trigger.
Sibling component: checked, no change
components/claude-lanes/claude-security-review.ymlwas read in full, notassumed to match. It makes no key-level claim. Its one queue-adjacent
statement — "a full queue CANCELS new arrivals" — is accurate:
A repo-wide grep confirms the defect had exactly one site:
This repository's own
.github/workflows/claude-review.ymlsets no caller-levelconcurrency and no
queue:, so it never carried the claim.Deliberate non-change
Independent verification surfaced a separate omission, not a falsehood: the
comment does not mention that
queue: maxcaps at 100 pending and cancelsarrivals beyond that. Left out on purpose — this PR narrows a false claim and
should stay a one-hunk diff while it races the sync PRs.
The overflow-wedge argument that makes the cap load-bearing in the security
lane does not transfer here, and that was checked rather than assumed: every
ruleset on all five managed targets was enumerated and its
required_status_checkscontexts read, and no target requires anyclaude-named context today.So overflow cancellation on this lane cannot void a required check — the same
premise that already makes this lane's deliberate
cancel-in-progress: truesafe. If a consumer later promotes the code-review context to required, the
cap becomes load-bearing and the comment should gain it. Worth a follow-up on
its own merits, not a blocker for this one.
Test plan
components/claude-lanes/claude-lanes.test.sh, counted from the run ratherthan eyeballed, and reproduced by two independent runs:
That includes
[29] a synced lane caller fails actionlint without the suppressionand[30] control run reports the suppressed message. Note foranyone re-running: the harness materializes from the git index, so the
change must be staged or every target reports
source worktree bytes differ from the indexed objectand the suite fails for that reason alone.Comment-only change: no YAML key, value, group expression, or pin is touched.
Confirmed by
git diff --stat origin/main...HEAD— 6 insertions, 3 deletions,every line inside a
#comment block.Independent verification by a fresh-context agent with the rationale withheld
(given the final file text and asked whether every claim is true, and whether
the constraint is stated at the correct level of generality). Verdict on the
corrected wording: ACCURATE — "The comment restates the constraint at
precisely the docs' generality — the docs name the literal
queue: max+cancel-in-progress: truepair and nothing wider. Neither broader nornarrower. I would not reword this clause." It also independently confirmed
queue:is valid at job level, viadata/reusables/actions/jobs/section-using-concurrency-jobs.mdincluding thesame reusable.
Repository CI on this PR: every check in the
passbucket, zero non-pass,verified by
gh pr checks 294 --json name,bucketon each pushed head(
a2a8228and565559a). That includes the Claude review lane(
review / review) — run once per head, never re-run to chase green.Related
corrected here.
melodic-software/provisioning#231, melodic-software/github-iac#244,
melodic-software/medley#1676.
ci-workflowsand are trackedthere (ci-workflows#296 for the workflow; ci-workflows#302, merged, for
PLAN.md). Deliberately untouched by this PR.No linked issue.