Skip to content

docs(ci): state the observed contract-only ci-status re-run behavior - #5592

Merged
kyle-sexton merged 4 commits into
mainfrom
fix/4670-ci-status-stale-rerun
Sep 30, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
fix/4670-ci-status-stale-rerun

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Refs #4670

Summary

The Contract-only ci-status section of docs/ci-runner-routing.md told operators to "try re-running" a red contract-only run and to push a commit if it stayed red. A throwaway-PR reproduction on #4670 showed what actually happens, so the section now says so.

Fix

  • Remedy: re-running a red contract-only run after ci-lanes is success passes the composite and replaces the red check run. No new commit is needed.
  • Defect (a): a body edit while a failed full run is being re-run reads the stale ci-lanes failure at once and goes red. The fix is upstream: ci-status: a stale ci-lanes failure ends the carry-forward wait while its full run is re-running ci-workflows#646.
  • The ruleset behavior for two same-name ci-status check runs stays marked unverified. The probe could not observe it (the do-not-merge label kept every run red), and no stale-check-clearing step is added.
  • ci.yml is not repinned here.

Verification

Related

🤖 Generated with Claude Code

Refs #4670

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review September 30, 2026 13:23
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 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-09-30T13:28:02.663225Z 62c1945 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 Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 34s —— View job


Security review

  • Gather context (diff, changed files)
  • Determine request type: /review:security-review per repo custom instructions
  • Apply skip gate
  • Report result

Skip gate hit: this PR changes only docs/ci-runner-routing.md (11 additions / 8 deletions), editing prose in the "Contract-only ci-status" section to describe the observed re-run behavior (confirmed via a throwaway-PR reproduction on #4670) instead of the previously-assumed behavior. No code, workflow, or config file is touched.

  • Not a standing instruction/guardrail file (not CLAUDE.md/AGENTS.md/a rules file/a skill or agent body/a hook allowlist) — it's operator-facing documentation of CI behavior, so the instruction-surface-deletion lens doesn't apply. The edited clause narrows a description of remedy steps, it doesn't remove an enforced control: ci-status/ci-lanes gating logic lives in the pinned ci-workflows composite, unchanged here.
  • No injection, auth, secrets, or trust-boundary surface in the diff — it's pure markdown text plus one new link to melodic-software/ci-workflows#646.
  • No GitHub Actions files changed (zizmor's lane doesn't apply).

No security issues found. Nothing to fix.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 57s —— View job


Code review complete

  • Gather context (PR diff, CLAUDE.md/AGENTS.md, skip gate)
  • Review changed file for correctness/maintainability
  • Post findings

Scope: docs/ci-runner-routing.md is the only changed file (+11/-8), all within the "Contract-only ci-status" section.

Review: This is a prose-only documentation update describing observed CI re-run behavior (per the linked issue comment for #4670). I checked it for internal consistency against the rest of the doc and for factual/logical contradictions:

  • The new remedy text ("re-run the red contract-only ci run… Carried forward: ci-lanes is success… pushing a new commit is not needed") is consistent with the unchanged "Contract-only ci-status" intro paragraph above it (composite waits on an in-flight run, then ends on a settled status).
  • The new defect description (body edit during a full-run re-run reads the stale ci-lanes failure immediately and goes red) is consistent with the "waits … for an in-flight full run, then ends on a settled success/failure/error" behavior described earlier in the same section — the added "wait loop only holds while no settled ci-lanes status exists" sentence restates that correctly rather than contradicting it.
  • The ci-workflows#646 link is well-formed, and the scope note ("this repository pins the composite… until that pin moves") matches the PR's stated non-goal of repinning ci.yml.
  • No broken markdown, no stale terminology introduced, no contradiction with unchanged sections (Configuration contract, Routing and failure behavior).

This is not something I could independently verify against live CI run data (that evidence lives in the linked #4670 comment, which I did not fetch), so I'm reviewing it as a documentation-consistency/correctness pass rather than confirming the underlying CI behavior itself. The author's markdownlint-cli2 result is author-claimed and I did not re-run it.

Findings: none. No correctness or maintainability issues in this diff that meet this lane's high-signal bar.

@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: 62c1945c00

ℹ️ 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 docs/ci-runner-routing.md Outdated
@github-actions

Copy link
Copy Markdown
Contributor

PR body contract — issue linkage

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

  • 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.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 30s —— View job


Security review complete

  • Gather context (diff, changed files)
  • Invoke /review:security-review per custom instructions
  • Report result

Skip gate hit: the diff between origin/main and HEAD (7eefaac0) touches only docs/ci-runner-routing.md (+11/-8), entirely within the "Contract-only ci-status" section. It rewords the operator remedy for a red contract-only ci-status run and adds one paragraph describing the stale-ci-lanes-carry-forward defect, with a link to ci-workflows#646.

  • Not a standing instruction/guardrail surface — docs/ci-runner-routing.md is operator-facing documentation describing observed CI behavior, not CLAUDE.md/AGENTS.md/a rules file/a skill or agent body/a hook allowlist. No enforced control is removed or softened here: the actual ci-status/ci-lanes gating logic lives in the pinned ci-workflows composite (.github/workflows/ci.yml), which this PR does not touch.
  • No injection, auth, secrets, or trust-boundary surface in the diff — pure prose plus one new external link to a public GitHub issue.
  • No GitHub Actions workflow files changed, so zizmor's advisory lane doesn't apply here.

Note: Codex's inline comment on this PR (docs/ci-runner-routing.md:76) raises a documentation-correctness concern — that the remedy reads as unconditional but doesn't hold when the contract-only run is red due to an invalid title or the do-not-merge label rather than stale ci-lanes carry-forward. That's a correctness/scope concern for /review:code-review, not a security issue, so it's out of scope for this lane.

No security issues found. Nothing to fix.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review complete

  • Gather context (diff, changed files)
  • Invoke /review:code-review and follow it
  • Post findings (inline comments + summary)

Scope: docs/ci-runner-routing.md only (+11/-8), confirmed against git diff origin/main...HEAD — the diff matches what's reported in the PR (only the "Contract-only ci-status" section changed).

Finding (posted inline)

docs/ci-runner-routing.md:74-76 — Correctness. The new remedy bullet states unconditionally that re-running a red contract-only ci-status passes once ci-lanes is success. But the doc's own line 73–74 says the same ci-status composite also carries a live evaluation of the PR contract (title, do-not-merge label, issue linkage), independent of the carried-forward ci-lanes verdict. Since a labeled/unlabeled event is itself one of the three contract-only triggers (line 66), the concrete failing case is: do-not-merge gets applied while ci-lanes is already success from an earlier full run — the contract-only run goes red because of the label, and re-running per this bullet's promise will not turn it green (same for an invalid title). The previous wording ("try re-running... if it stays red, push a new commit") was hedged for exactly this reason; this PR drops the hedge and states an outcome that doesn't hold for contract-driven reds. I independently verified this against .github/workflows/ci.yml's own inline comments (lines ~71-74: "carries the pull-request contract... inside the same ci-status job"), not just Codex's earlier comment on the same lines, which raised the same concern.

Full detail and a suggested wording fix are in the inline comment.

Everything else in the diff (the defect-(a) paragraph, the ci-workflows#646 link, the "unverified" close) checked out as internally consistent with the rest of the document and with ci.yml's comments. Author-claimed markdownlint-cli2 output and the linked #4670 run-id evidence are author-claimed and not independently re-verified here.

Comment thread docs/ci-runner-routing.md Outdated
A re-run turns green only when ci-lanes is success and the title and
do-not-merge label pass. Add the invalid-title and label case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 34s —— View job


Security review complete

  • Gather context (diff, changed files)
  • Invoke /review:security-review and follow it exactly
  • Report result

Skip gate hit: the diff (gh pr diff 5592, current head 55dabf3) touches only docs/ci-runner-routing.md (+14/-8), entirely within the "Contract-only ci-status" section. It rewords the operator remedy to split the passing-contract case from the invalid-title/do-not-merge case, and expands the stale-ci-lanes-carry-forward defect paragraph with a link to ci-workflows#646.

  • Not a standing-instruction/guardrail surface: docs/ci-runner-routing.md is operator-facing documentation of observed CI behavior, not CLAUDE.md/AGENTS.md/a rules file/a skill or agent body/a hook allowlist, and it isn't one of the on-demand convention surfaces listed in this repo's AGENTS.md. No enforced control is removed or softened by this diff — the actual ci-status/ci-lanes gating and pr-contract logic live in the pinned ci-workflows composite (.github/workflows/ci.yml), which this PR does not touch. The new wording also correctly separates the "contract passes" remedy from the "title/label" remedy, so it doesn't newly describe an unsafe bypass either.
  • No injection, auth, secrets, or trust-boundary surface in the diff — pure prose plus one external link to a public GitHub issue.
  • No GitHub Actions workflow files changed, so zizmor's advisory lane doesn't apply here.

No security issues found. Nothing to fix.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review complete

  • Gather context (PR diff, CLAUDE.md/AGENTS.md, skip gate)
  • Review changed file for correctness/maintainability
  • Post findings (inline comment + summary)

Scope: docs/ci-runner-routing.md only, both commits in this PR (62c1945 and 55dabf3), diffed against origin/main. 55dabf3 already resolves the earlier Codex/Claude finding on this PR: bullet 1 of the operator remedy now conditions the "re-run passes" promise on the contract also passing (valid title, no do-not-merge), and a new bullet 2 covers the invalid-title/do-not-merge case.

Finding (posted inline)

docs/ci-runner-routing.md:78-79 — Correctness. The new bullet 2 ("Fix the title or remove the label, then re-run") states the fix unconditionally, but it's the mirror image of the gap already fixed in bullet 1: the ci-status job runs the pr-contract step and the ci-lanes carry-forward aggregate step independently (.github/workflows/ci.yml:2554-2571), and either one failing turns the check run red. Concrete case: a SHA with an invalid title and ci-lanes already failure/missing — fixing the title and re-running per this bullet only satisfies the contract step; the aggregate step still reads the failing/missing ci-lanes status and the check stays red. Bullet 1 states the ci-lanes-success precondition explicitly; bullet 2 should carry the same precondition (or an "also re-run the full workflow if ci-lanes isn't success" clause) so it doesn't repeat the exact unconditional-promise issue this PR just corrected elsewhere.

Everything else checked out: the reworded bullet 1, the defect-(a) paragraph, and the ci-workflows#646 link (all from commit 62c1945, unchanged by 55dabf3) are internally consistent with the rest of the document and with ci.yml's inline comments, and were already reviewed by prior passes on this PR with no unresolved issues. Author-claimed markdownlint-cli2 output and the linked #4670 run-id evidence remain author-claimed and were not independently re-verified here.
· branch

Comment thread docs/ci-runner-routing.md Outdated
Fixing the title or label does not turn the check green when ci-lanes is
also failed or missing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 28s —— View job


Security review complete

  • Gather context (diff, changed files)
  • Apply skip gate
  • Report result

Skip gate hit: the diff since the last security pass on this PR (commit 47e4e33,

**Operator remedy.** When a contract-only `ci-status` is red:
- If `ci-lanes` on that SHA is `success` and the pull-request contract passes
(valid title, no `do-not-merge` label), re-run the red contract-only `ci`
run. The composite logs `Carried forward: ci-lanes is success`, passes, and
the re-run replaces the red check run. Pushing a new commit is not needed.
- If the title is invalid or `do-not-merge` is applied, the contract check
stays red on a re-run. Fix the title or remove the label first, then follow
the other bullets for `ci-lanes`.
- If `ci-lanes` is `failure` or missing, re-run the full workflow.
) only edits the second operator-remedy bullet in docs/ci-runner-routing.md's "Contract-only ci-status" section — the invalid-title/do-not-merge case now says "then follow the other bullets for ci-lanes" instead of promising the re-run turns green on its own. This resolves the correctness finding earlier reviews raised on that bullet; it does not change scope.

  • Not a standing-instruction/guardrail surface (not CLAUDE.md/AGENTS.md/a rules file/a skill or agent body/a hook allowlist) — this is operator-facing documentation of observed CI behavior. No enforced control is removed or softened: the actual ci-status/ci-lanes gating and pr-contract logic live in the pinned ci-workflows composite (.github/workflows/ci.yml), unchanged here.
  • No injection, auth, secrets, or trust-boundary surface in the diff — pure prose, no new links or code in this latest commit.
  • No GitHub Actions workflow files changed, so zizmor's advisory lane doesn't apply.

No security issues found. Nothing to fix.
· branch

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review complete

  • Gather context (PR diff, CLAUDE.md/AGENTS.md, skip gate)
  • Invoke /review:code-review and follow it exactly
  • Post findings

Scope: docs/ci-runner-routing.md only, full PR diff (62c1945..47e4e33) against origin/main, with focus on the newest commit 47e4e33 since the prior two review rounds on this PR already flagged and the author already fixed two correctness gaps in 55dabf3.

What changed in 47e4e33: bullet 2 of the operator remedy ("If the title is invalid or do-not-merge is applied...") now reads "Fix the title or remove the label first, then follow the other bullets for ci-lanes" instead of promising a bare re-run turns the check green.

Verification against .github/workflows/ci.yml:2554-2581: the ci-status job runs Check the pull-request contract (pr-contract) and Aggregate lane results (the ci-lanes carry-forward read, if: !cancelled()) as independent steps in the same job; either failing turns the job/check run red. 47e4e33's rewording closes exactly the gap raised on the prior commit's review threads (the Claude/Codex inline comments on 55dabf3, which the author marked fixed): a title/label fix alone no longer implies the check turns green — the text now correctly routes back to the ci-lanes bullets (bullet 1 for success, bullet 3 for failure/missing) to determine the actual remedy after the contract is fixed.

I also checked that bullet 1's parenthetical ("valid title, no do-not-merge label") correctly omits issue-linkage: per pr-contract and this repo's .claude/rules/pr-body-contract.md, a missing closing keyword only produces the advisory needs-issue-linkage label and comment, it does not turn ci-status red, so it's correctly left out of what "the contract passes" requires here.

No remaining correctness issues found in the diff. markdownlint-cli2 could not be independently re-run in this sandbox (network-restricted), so that verification claim remains author-claimed, as in prior passes on this PR.
· branch

@kyle-sexton
kyle-sexton merged commit 6b5fdde into main Sep 30, 2026
17 checks passed
@kyle-sexton
kyle-sexton deleted the fix/4670-ci-status-stale-rerun branch September 30, 2026 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant