docs: record which re-run modes can verify a major-tag slide - #365
Conversation
Hit while verifying #359's release. After v2 was slid to c50e847, a re-run of the run that had failed still failed with the same 'ai-config@d-morrison' error, because GitHub replays the reusable-workflow SHA recorded in referenced_workflows at the original run's creation. The confusing part is that nested composite actions DO re-resolve at job-preparation time, so the same job log showed a c50e847-only echo line next to the old workflow's INPUT_PLUGINS -- which reads as the fix being live but ineffective, rather than not yet live. Documented next to the existing "can't self-verify before merge" section, since that one ends at the merge and this covers the step after it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PKotEhi2qeEQ1kZwuFES8
🤖 Jules ReviewSummaryThis PR adds useful documentation to Strengths
Findings[NIT]
VERDICT: comment Session: |
|
Thanks for the review. One finding, dispositioned below.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Code reviewReviewed the diff (new One finding, posted inline on Everything else checked out:
VerdictNeeds minor changes — one factual/precision fix to the "never a re-run" guidance (inline comment above) before merge; everything else is clean. |
|
Claude finished review — View run Review posted on PR #365. Finding (inline, on Everything else — placement, heading/citation conventions, semantic line breaks, AI-prose tells, forward references, redundancy, changelog fragment format, and Markdown well-formedness — checked out clean across all four independent review passes. Summary comment: #365 (comment) VerdictNeeds minor changes — one factual/precision fix required before merge; everything else is clean. |
Review correctly found that "the UI's re-run button" and "never a re-run"
were overbroad. Verified against GitHub's own docs -- read from the docs
source (data/reusables/actions/partial-reruns-with-reusable.md), since
docs.github.com returns 403 to this environment:
* Re-running all jobs in a workflow will use the reusable workflow from
the specified reference.
* Re-running failed jobs or a specific job in a workflow will use the
reusable workflow from the same commit SHA of the first attempt.
So only the failed-jobs/specific-job path is stale; "Re-run all jobs"
re-resolves the tag. Narrowed the heading and the closing advice, quoted
the docs inline so a later reviewer re-deriving this finds the citation in
place rather than in thread history, and added the docs' own
reference-is-not-a-SHA precondition.
Also addressed the reviewer's second point: the composite carve-out means a
failed-jobs re-run CAN surface a slide whose only change lives in a
composite, so the section now says what such a re-run cannot verify is a
change to the reusable workflow's own content.
The observed case was an API rerun_failed_jobs, which the narrowed claim
still covers exactly; the evidence did not change, only its scope.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012PKotEhi2qeEQ1kZwuFES8
|
🤖 Jules is reviewing this PR. Results will appear here shortly (typically 2–5 minutes). |
lint-markdown's MD004/ul-style requires dash markers; the block quoted GitHub's docs verbatim, which use asterisks. Only the list marker changed; the quoted wording is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PKotEhi2qeEQ1kZwuFES8
🤖 Jules ReviewSummaryThis PR adds documentation to Strengths
Findings[NIT]
VERDICT: comment Session: |
dem-extra1
changed the title
Round 2
Jules' date nit — Rebutted, holding. This is the same finding as the previous round, re-raised verbatim without engaging that rebuttal. Also fixed this round: One note on CI: Generated by Claude Code |
Code reviewFollow-up review after the prior round's finding (the "UI's re-run button" overbroad claim). The section has since been rewritten ( I also re-verified the other core mechanism claims directly against GitHub's REST API docs:
One new, non-blocking finding posted inline: the backtick-quoted Also checked: changelog fragment ( VerdictReady for merge — the prior blocking finding is resolved; the one new finding is a minor optional nit, not a blocker. |
|
Claude finished review — View run The comment has been corrected with the accurate link. SummaryReviewed PR #365 (the
Verdict: Ready for merge |
Review flagged that backticked `rerun_failed_jobs` reads as a precise API citation but matches no REST endpoint. The concern is right; the stated grounds are only partly so. `rerun_failed_jobs` IS a real identifier -- it is the GitHub MCP server's `actions_run_trigger` method value, and the literal call the anecdote describes -- but it is not the REST or CLI name, so backticking it alone invites a reader to hunt for an endpoint by that name. Rather than swap one surface's spelling for another, name all three and say that none is canonical: the REST path segment, `gh run rerun --failed`, and the MCP method. Dropped the "API" qualifier from the anecdote, which was what made the single spelling read as authoritative. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PKotEhi2qeEQ1kZwuFES8
🤖 Jules ReviewSummaryThis PR adds a section to Strengths
Findings[NIT]
VERDICT: comment Session: |
Round 3
Jules' date nit — Deferred, now tracked in #366. Third identical raise across three heads, each without engaging the round-1 rebuttal. Since it doesn't self-correct and will fire on every dated To restate once for anyone landing on this thread: No open findings from my side. Both inline threads are resolved, and the last Generated by Claude Code |
|
Code review: Confirming review, no new findings. Follow-up to the prior round's verdict at #365 (comment) (Ready for merge), which flagged one non-blocking nit: the backtick-quoted rerun_failed_jobs didn't match the REST/CLI spelling. That nit has since been addressed in commit 3dc8e83 (round 2): the section now names all three surfaces without asserting a canonical one -- REST (POST .../rerun-failed-jobs), CLI (gh run rerun --failed), and the GitHub MCP server's actions_run_trigger (method: rerun_failed_jobs). I re-verified this directly against github/github-mcp-server's source (pkg/github/actions.go): the tool is indeed named actions_run_trigger, and rerun_failed_jobs is a real constant (actionsMethodRerunFailedJobs) passed to it, so this spelling isn't fabricated, just not the REST/CLI name, exactly as the new text now says; also re-confirmed gh run rerun --failed is the real CLI invocation per GitHub CLI docs. The second occurrence at the anecdote (previously flagged as needing the same fix) no longer asserts a specific API name at all, which resolves that instance too. No other changes landed since the last round; the prior verdict stands. Verdict: Ready for merge. |
|
Claude finished review — View run Review complete and posted to PR #365. SummaryThis was a confirming re-review. Since the last review round (verdict: "Ready for merge" with one non-blocking nit), a new commit ( I independently verified the fix rather than trusting the commit message:
No new issues found. Posted a confirming comment referencing the prior verdict, since GitHub's Markdown rendering combined with this session's shell sandboxing made a multi-line/heading-formatted comment impractical to post reliably (link-bracket and heading-line patterns kept tripping static command-safety checks) — the single-line body still conveys the full finding and citation trail. VerdictReady for merge |
What
Adds a
CLAUDE.mdsection documenting that re-running failed jobs cannot verify a major-tag slide, while re-running all jobs can.Why
Hit while verifying #359's release. After
v2was slid toc50e847, the obvious way to confirm the fix had reached consumers was to re-run the run that had failed. That re-run failed with the identicalFailed to install plugin 'ai-config@d-morrison'error, which reads as "the fix didn't work".It hadn't been reached at all.
referenced_workflowsstill readsha: 6ee996bon attempt 2, hours after the tag moved.The rule, per GitHub's docs
Review round 1 correctly caught that my first draft overstated this as "never a re-run". GitHub's docs distinguish two modes:
So only the failed-jobs / specific-job path replays the stale SHA — and that is what I actually ran. The section now says exactly that, and quotes the docs inline so a later reviewer re-deriving the claim finds the citation next to it rather than in thread history.
Round 2 then flagged that backticking a single spelling of that call reads as a canonical API name. It is real (it's the GitHub MCP server's
actions_run_triggermethod value) but it isn't the REST or CLI spelling, so the section now names all three —rerun-failed-jobs(REST path segment),gh run rerun --failed(CLI),method: rerun_failed_jobs(MCP) — and says none of them is canonical.Why it's worth documenting rather than just knowing
The two layers behave differently within the stale mode, which is what makes it deceptive rather than merely surprising. Composite actions nested inside the reusable workflow re-resolve at job-preparation time. The same job log showed both at once:
INPUT_PLUGINS: ai-config@d-morrison— from the old reusable workflowdetect-review-request: match=false— an echo line that only exists atc50e847That reads as "the fix is live and ineffective" rather than "the fix isn't live yet", which points debugging in exactly the wrong direction. It also means such a re-run can surface a slide whose only substantive change lives in a composite — so what a failed-jobs re-run cannot verify is specifically a change to the reusable workflow's own content.
Placement
Directly after the existing "A PR fixing claude-code-review.yml (or claude.yml) itself can't self-verify before merge" section. That one ends at the merge; this covers the step after it, and the two are otherwise easy to conflate.
The entry gives the mechanical check (
referenced_workflows[].shaviaactions_getget_workflow_run) rather than leaving it to inference, per the repo's own "Never just theorize -- investigate empirically" guidance.Verification
npx --prefix lint-markdown markdownlint-cli2 --config lint-markdown/.markdownlint.default.jsonc CLAUDE.md->Summary: 0 error(s)NLB_BASE_REF=origin/main python3 check-new-line-breaks/check-new-line-breaks.py->No lines missing semantic breaks.(run after committing, so the diff is non-empty)A note on the docs citation:
docs.github.comreturns 403 to this environment, so the quoted text was read from the docs' own source,data/reusables/actions/partial-reruns-with-reusable.mdingithub/docs, viaraw.githubusercontent.com.Adds a
changelog.d/fragment per the repo's convention.Generated by Claude Code