Skip to content

ci(ci-status): yield a contract-only run to the full run, not wait - #94

Merged
kyle-sexton merged 2 commits into
mainfrom
ci/yield-to-full-run
Oct 5, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
ci/yield-to-full-run

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

What

Adopts ci-workflows v0.37.0 yield-to-full-run in pr-require-checks.yml, same shape as melodic-software/medley#2115.

  • check-contract and aggregate-results composites re-pinned to d49c4915ba75bbc9602cc30980604cad0806334e # v0.37.0.
  • aggregate-results sets yield-to-full-run: 'true', rerun-contract-only-siblings: 'true', carry-forward-wait-seconds: '0' (was '240'). No record-pending.
  • ci-status: actions: write (was read), timeout-minutes: 3 (was 5).
  • Header and job comments updated for the new behavior.

Why

A contract-only ci-status (edited/labeled/unlabeled; every lane skips) that waits on an in-flight full run holds a runner for minutes. With yield-to-full-run it fails at once ("superseded by full run "), the full run's own ci-status decides the gate, and on success the full run re-runs the red contract-only siblings.

Not changed

The SYNC-MANAGED hosted workflows (pr-check-managed-files-hosted.yml, pr-review*-hosted.yml) and pr-automerge-dependabot.yml stay on v0.36.0 (reusable workflows; sync-managed files belong to standards). The repo has no .github/runner-policy.json or runner-policy standards.

Tests run

  • actionlint .github/workflows/pr-require-checks.yml: clean
  • npm test: 114 pass, 0 fail
  • npm run validate: PASS, 0 errors

🤖 Generated with Claude Code

…aiting

Pin the pr-require-checks composites to ci-workflows v0.37.0 and set
yield-to-full-run, rerun-contract-only-siblings and a zero carry-forward
wait on aggregate-results. ci-status gets actions: write and a 3-minute
timeout. Same shape as melodic-software/medley#2115.

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

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

PR body contract — issue linkage

This PR body does not yet satisfy the issue-linkage contract:

  • Missing a "## Summary" section. Describe what this PR changes and why, in a sentence or two.
  • Missing a "## Fix" section. State the concrete change and how it addresses the problem.
  • Missing a "## Verification" section. Record concrete evidence the change works (commands, gates, output).
  • Missing a "## Related" section. List related PRs, ADRs, or decision-log entries this PR does not close.
  • Missing a native closing keyword (Closes/Fixes/Resolves #N). If this PR references an issue it must not close, put "Refs: #N" (or "Relates to: #N") on its own line. If it relates to no GitHub issue at all, state "No linked issue" (or "No related issue:") in the body instead.

Edit the body and this comment updates itself on the next run.

@kyle-sexton
kyle-sexton marked this pull request as ready for review October 5, 2026 01:23
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-05T01:28:41.790574Z 7830df3 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

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


CI code review — pr-require-checks.yml (PR #94)

  • Gather context (diff vs origin/main, full file, sibling SYNC-MANAGED workflows)
  • Understand request (code-review lane via /review:code-review)
  • Review the changed workflow for correctness
  • Post findings

Scope: .github/workflows/pr-require-checks.yml (+22/-14), the only changed file.

What changed: check-contract/aggregate-results repinned v0.36.0 → v0.37.0; aggregate-results now sets yield-to-full-run: 'true', rerun-contract-only-siblings: 'true', carry-forward-wait-seconds: '0' (was '240'); job permissions.actions goes read → write; timeout-minutes goes 5 → 3. Header/job comments were rewritten to match.

Findings: none at the high-signal bar.

Walked the race/permission logic this diff touches:

  • actions: write is the correct superset for rerun-contract-only-siblings (listing runs and triggering a re-run) — there's no narrower built-in scope for "rerun only." Security implications of this grant are in scope for the separate security lane (pr-review-security-hosted.yml exists in this repo), not this one.
  • timeout-minutes: 3 is consistent with the new zero-wait, fail-fast/yield behavior described in the job comment (no step now blocks on an in-flight run).
  • The "contract-only runs are immune to eviction" invariant from the concurrency-group comment (unique ci-contract-<pr>-<run_id> group) still holds for rerun-contract-only-siblings: a re-run reuses the same run_id, so it stays in its own unique group — it doesn't fall into the hazard described further up (a full-branch stale re-run landing in the current head's shared group).
  • The "Not changed" claim in the PR body checks out: the other ci-workflows@v0.36.0 pins (pr-check-managed-files-hosted.yml, pr-review-hosted.yml, pr-review-security-hosted.yml, pr-automerge-dependabot.yml) are untouched by this diff, consistent with them being SYNC-MANAGED/reusable-workflow files owned by standards.
  • Comment-vs-code consistency: the header block (lines 52–56) and the two job-level comments were all updated together and accurately describe the new yield-to-full-run / rerun-contract-only-siblings / carry-forward-wait-seconds: '0' behavior — no stale prose left referencing the old bounded-wait design.

Not independently verified (author-claimed only): this sandbox's Bash tool requires approval for anything beyond local git read commands, so actionlint, npm test, npm run validate, and the v0.37.0 tag/SHA on melodic-software/ci-workflows could not be re-run or re-checked here. The PR body's reported results (actionlint clean, 114/114 tests, validate PASS) are taken as author-claimed, not confirmed by this review.

Out of scope for this lane: the PR-body contract (missing ## Summary/## Fix/## Verification/## Related and a closing keyword) is already flagged by the repo's own needs-issue-linkage automation on this PR — not duplicated here.
· branch

@claude

claude Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

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


Security review — no issues found

  • Gather context (PR diff, file)
  • Apply skip gate
  • Review changed workflow file for security-relevant issues
  • Check for instruction-surface deletions
  • Post findings

Scope: .github/workflows/pr-require-checks.yml (the only changed file) — current blob.

What changed: re-pins check-contract/aggregate-results to ci-workflows v0.37.0, switches the carry-forward mechanism from a bounded wait (carry-forward-wait-seconds: '240') to yield-to-full-run/rerun-contract-only-siblings ('0'), and widens the ci-status job's actions: scope from read to write so it can list and re-run sibling runs on the same head SHA.

Findings: none that meet this lane's bar.

  • The actions: read → actions: write widening is a permissions: change — that's zizmor's advisory lane per this skill's scope, not re-reported here. Worth noting for the record: the workflow's own header comment (lines ~20–22) documents that fork PRs are forced to a read-only GITHUB_TOKEN on pull_request regardless of what permissions: requests, and validate-marketplace's if: guard already routes non-same-repo contract-only paths through the full run — so the widened scope only ever takes effect for same-repository PRs on the contract-only path, not fork PRs.
  • The rerun-contract-only-siblings / yield-to-full-run logic itself lives in the external melodic-software/ci-workflows composite (pinned by SHA, not altered loosely here), outside this diff's reviewable surface — no in-diff logic flaw found in how this workflow invokes it (inputs, if: guards, and token: wiring are consistent with the stated design in the comments).
  • No secrets, credentials, or injectable ${{ github.event.* }} expressions are introduced in run:/uses: context here (out of scope for this lane per zizmor anyway).
  • This diff is CI-orchestration YAML, not a rules/CLAUDE.md/AGENTS.md/skill/hook guardrail file, so the instruction-surface-deletion lens doesn't apply; no standing instruction is removed, narrowed, or softened by this change.

No security concerns to raise for this PR.

Branch: ci/yield-to-full-run

@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

claude-review has reviewed this pull request through 01dd20d; a later push is reviewed from there.

@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: 7830df3ddd

ℹ️ 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/workflows/pr-require-checks.yml Resolved
@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

claude-security-review has reviewed this pull request through 01dd20d; a later push is reviewed from there.

@kyle-sexton kyle-sexton changed the title ci(ci-status): yield a contract-only run to the full run instead of waiting ci(ci-status): yield a contract-only run to the full run, not wait Oct 5, 2026
…ll run

A draft full run skipped the lanes, so it must not re-run siblings.

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

claude Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

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


Security review — pr-require-checks.yml (PR #94, incremental)

  • Gather context (incremental diff since 7830df3, .claude-lane/incremental.diff)
  • Apply skip gate
  • Review changed lines for security-relevant issues
  • Check for instruction-surface deletions
  • Post findings

Scope: incremental diff only — .github/workflows/pr-require-checks.yml lines 128–131, the single line changed by 01dd20d since this lane last reviewed 7830df3. (Everything else in the file was reviewed at 7830df3 and is unchanged.)

What changed:

-          rerun-contract-only-siblings: 'true'
+          rerun-contract-only-siblings: ${{ github.event.pull_request.draft && 'false' || 'true' }}

Permalink. Commit message: "A draft full run skipped the lanes, so it must not re-run siblings."

Findings: none.

  • github.event.pull_request.draft is a plain boolean from the event payload, consumed only as an action with: input (a ${{ }} boolean ternary), not interpolated into a run: shell block or any other injectable sink — no template/command-injection surface here (and that class is zizmor's lane regardless).
  • The change only narrows when rerun-contract-only-siblings fires (drops from unconditional 'true' to 'true' only on a non-draft full run). That's a strictly more conservative direction: it stops a draft run — where other required lanes (pr-review-hosted.yml, pr-review-security-hosted.yml) are gated off via their own if: github.event.pull_request.draft == false — from being treated as a trustworthy "full success" signal that promotes red contract-only sibling runs to green. No path by which this lets a contract-only run merge on a weaker signal than before; the failure mode if anything is fail-closed (siblings stay red/unrehearsed) rather than fail-open.
  • No permission, trigger, or pin changes in this incremental diff — permissions:, actions: write, the v0.37.0 pins, and timeout-minutes: 3 were all already reviewed at 7830df3 and are untouched here.
  • Not an instruction-surface deletion: nothing in CLAUDE.md/AGENTS.md/rules/hooks is touched, and the change adds a condition rather than removing or softening a check.

No security concerns to raise for this increment.
· branch

@claude

claude Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

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


Incremental CI code review — pr-require-checks.yml (PR #94)

  • Gather context (.claude-lane/incremental.diff since 7830df3, full file, sibling lane workflows)
  • Understand request (incremental /review:code-review pass — review only the diff since the last review)
  • Review the one-line change for correctness
  • Post findings

Scope: the diff since 7830df3 is one line, .github/workflows/pr-require-checks.yml#L131:

-          rerun-contract-only-siblings: 'true'
+          rerun-contract-only-siblings: ${{ github.event.pull_request.draft && 'false' || 'true' }}

Findings: none at the high-signal bar.

  • Expression correctness. github.event.pull_request.draft is a boolean in the webhook payload. When true: true && 'false' short-circuits to the string 'false' (truthy), then 'false' || 'true' returns 'false' (a non-empty string is truthy in GitHub Actions' expression semantics, same as the &&/|| idiom this file already uses in the concurrency.group expression at lines 65–70). When false: false && 'false' returns the boolean false, then false || 'true' returns the string 'true'. Both branches resolve to the string the composite's input expects (matching the existing yield-to-full-run: 'true' string-input convention) — no type-coercion bug.
  • Premise check. The commit message states "a draft full run skipped the lanes, so it must not re-run siblings." Confirmed: pr-review-hosted.yml:76 and pr-review-security-hosted.yml:73 both gate their lane job on github.event.pull_request.draft == false, and this workflow's own comment at lines 134–136 notes a skipped lane is already treated as a failure (treat-skipped-as: fail). So a full run triggered while the PR is a draft is itself red for a reason unrelated to the actual lane outcome, and re-running previously-red contract-only siblings off that red-by-construction verdict would just waste the actions: write rerun call without fixing anything. Disabling the rerun for that case is sound.
  • Fallback for non-PR events. ci-status also runs on push to main (job if: !cancelled() has no PR gate), where github.event.pull_request doesn't exist. github.event.pull_request.draft then evaluates to null (falsy either way the expression is read), so the expression falls through to 'true' — i.e. unchanged behavior for push events. Correct.

Not independently verified (author-claimed only): this review did not re-run actionlint/npm test/npm run validate against this incremental commit; no tool-running capability was exercised beyond reading the files above.
· branch

@kyle-sexton
kyle-sexton merged commit e7fba05 into main Oct 5, 2026
7 checks passed
@kyle-sexton
kyle-sexton deleted the ci/yield-to-full-run branch October 5, 2026 01:37
kyle-sexton added a commit that referenced this pull request Oct 5, 2026
Restores `rerun-contract-only-siblings: 'true'` on `aggregate-results`
(changed to a draft-conditional in #94, 01dd20d).

`ready_for_review` is not in this workflow's `pull_request.types` and no
lane is draft-gated, so a draft full run runs every lane and re-running
its red contract-only siblings is safe. Skipping the re-run on a draft
could leave a yielded red contract-only ci-status with nothing to re-run
it after ready. This repo has no refuse-a-draft step, so the other draft
change does not apply.

Tests: `actionlint .github/workflows/pr-require-checks.yml` clean.

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

Co-authored-by: Claude Opus 5.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