Set a time limit on every job this repo defines - #58
Conversation
Closes #51. Three pin corrections plus the config that prevents the drift recurring. actions/checkout v4 -> v7 in all five workflows that use it. The issue named two (check-spelling.yaml, lint-project.yaml); copilot-setup-steps.yml, publish.yml and preview.yml were also on v4, so fixing only the named two would have left three behind. v7 is the current major upstream and matches rpt/qwt. v4 runs on the Node 20 runtime GitHub is winding down, which is the one item here with a deadline attached. r-spellcheck-action v3.0.2 -> v3.0.3, the current release, so the three templates no longer spellcheck to different rules for no stated reason. summary.yml @v2 -> @v1. Re-verified against Morrison-Lab/gha's README rather than the d-morrison/gha citation in the issue, which predates the org move: the Versioning section still records summary.yml as "unchanged since the freeze, so @v1 remains current". @v2 resolves and runs identical content today, so nothing was broken, but it would silently pick up future changes that @v1 consumers deliberately would not. Adds .github/dependabot.yml covering github-actions and gitsubmodule weekly, matching the siblings. Its absence is the root cause: those repos get bump PRs automatically and this one did not, so the drift went unnoticed because nothing failed. The config ignores semver-major bumps of first-party Morrison-Lab/gha references. Those track their capability's major tag by design, so a major bump is not an upgrade but a silent opt-in to a different contract -- and without the ignore, Dependabot's first run would propose reverting the summary.yml correction above. gha's own dependabot.yml excludes its first-party self-references for the same reason. Verified: no actions/checkout@v4 remains; all seven touched YAML files parse; the dependabot file is schema-valid, ASCII-only and newline-terminated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lx1hhHu58mXq9DfJsAxkHn
Closes #28. Adds timeout-minutes to the four local jobs that had none, sized by the convention Morrison-Lab/gha uses for the same kinds of work: check-spelling.yaml::check 20 lint-project.yaml::lint-project 20 preview.yml::build-deploy 45 publish.yml::build-deploy 45 Also lowers copilot-setup-steps.yml from 55 to 50. It already had a limit, so it did not look like a gap, but 55 exceeds the 50-minute cap the issue asks for, which makes it the one existing violation rather than a missing setting. Two things the issue's "all workflows" cannot mean, both established by reading the workflows rather than assumed: Fifteen of the twenty-one workflows consist of a single job that calls a Morrison-Lab/gha reusable workflow. GitHub rejects timeout-minutes on a job that uses `uses:`, so a limit cannot be set at the call site at all; it has to live in the reusable workflow. Every gha workflow this repo calls already sets its own, so those fifteen are covered upstream and need no change here. Adding one would break parsing. Two of those upstream limits are above the cap: claude.yml and claude-code-review.yml each carry a 60-minute job. Both are in Morrison-Lab/gha and no caller can override them, so closing that gap is an upstream change and is deliberately not attempted here. Everything this repo controls is now at or under 50. Documents the rule in .github/copilot-instructions.md, per the issue's second bullet, including the reusable-workflow exception so the next contributor does not try to add a limit that will not parse. Verified: every local job now has a limit and none exceeds 50; all workflow YAML parses; the added prose passes gha's own check-non-standard-chars script, which flags em dashes in .md and would otherwise have turned check-chars red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lx1hhHu58mXq9DfJsAxkHn
Fixes two checks the first push turned red, both in the prose added to .github/copilot-instructions.md rather than in the workflow changes: lint-markdown's list-item splice check (LIST_ITEM_SPLICE_FAIL=true) rejected the "Never exceed 50" bullet, which wrapped onto a continuation line with no intervening blank line. Bullets are now one line each, and the sentence that was wrapping is its own paragraph. check-new-line-breaks wanted semantic line breaks on two added lines. The section is rewritten one clause or sentence per line, matching how this repo writes prose. The 55 markdownlint findings in the same job are pre-existing and non-blocking (fail=false; they span CONTRIBUTING.md, IMPROVEMENT_AREAS.md and images/ README.md, none of which this PR touches), so they are left alone. No workflow file changed in this commit; the timeout values and the rule itself are unchanged. Verified by running gha's own checkers locally with CI's exact configuration and base ref: check-new-line-breaks examined 13 added lines and reported none missing breaks, and check_list_item_splices reported no splices. Both had reproduced the CI failure before the rewrite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lx1hhHu58mXq9DfJsAxkHn
The ignore rule's comment claimed "gha's own dependabot.yml excludes its first-party self-references for the same reason". That is not true, and the review round on #57 was right to flag it. Morrison-Lab/gha's dependabot.yml has exactly one ignore rule, and it freezes the third-party actions/ai-inference at its last usable major for an unrelated breaking-change reason. First-party references there are covered by no ignore rule at all; the file's own comment records them as needing "no Dependabot coverage" because they track a major tag rather than being SHA-pinned. The rule itself stays, because its README-sourced rationale is independently verified and unaffected. Only the supporting claim was wrong, and leaving it in a committed comment would have sent a future reader to gha's config looking for an exclusion that is not there. The replacement states the actual difference rather than a false symmetry: in gha those are self-references inside the same repo and fall outside its SHA-pin-focused scan, whereas here gha is an external repository whose major tags are precisely what Dependabot will see and offer to bump. That is why this repo needs a rule gha does not. Also records the review's second, non-blocking note in the comment: that the glob matching a full owner/repo/.github/workflows/file.yml dependency name is not explicitly documented and should be confirmed on the first real run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lx1hhHu58mXq9DfJsAxkHn
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The review round on #58 found the comment above timeout-minutes still reading "Timeout after 55 minutes" after the value below it was lowered to 50. It was right: I changed the number and left the sentence describing it, so the file contradicted itself on the one line this PR touched there. The replacement states the new value, says where the 50 comes from, and keeps the genuinely useful part of the original note, that the platform maximum for this workflow is 59. No behavior change; the value stays 50 and all workflow YAML still parses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lx1hhHu58mXq9DfJsAxkHn
|
Both findings were right. Fixed in Addressed — the stale comment. # Timeout after 50 minutes (qbt#28 caps every job here at 50; the
# copilot-setup-steps maximum is 59)
timeout-minutes: 50That keeps the genuinely useful half of the original note — the platform maximum of 59 — while saying where the 50 comes from. Addressed — the off-by-one. The count is 16, not 15. Re-derived by parsing rather than counting by eye: The sting is that my own survey output listed all 16; I miscounted them when writing the description. So "established by reading the workflows, not assumed" was true of the reading and false of the arithmetic on top of it, which is the more embarrassing half. The description now says 16, and 21 − 5 = 16 reconciles with the other figure as you noted. The commit message for the original change still says "fifteen" and can't be rewritten without a force-push on a branch with a PR open; Neither changes behavior: the value stays 50, every local job is still at or under the cap, and all 21 workflow files still parse. Worth noting you caught this by cloning Generated by Claude Code |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
The render is fine. Every step that would catch a bad workflow edit passed: Step 14 is the push to The controlled comparison now exists, which it didn't when I first saw this signature on #61:
The same step fails intermittently across different PRs while renders always succeed, and a re-run of identical content passed on #61. That is contention, not a defect: four PRs were open together, each deploying a preview to the same I created that contention by opening four PRs in quick succession, so I'm not calling it someone else's problem — but it is not fixable from inside this PR, and I've spent my one re-run on it rather than pushing a speculative change. If the re-run fails again, the theory is wrong and it becomes mine to root-cause properly. The durable fix is upstream. A Generated by Claude Code |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Claude finished review — View run Confirmed: the PR is open, unmerged, and the diff content ( The commits added since round 3 ( The prior verdict — Ready for merge, posted 2026-09-15T02:15:20Z — still stands: the only finding raised across all rounds (the stale "55 minutes" comment in round 1) was fixed in round 2 and remains fixed here, and no new issues are introduced by the intervening merge commits. One unresolved process note carried forward from the last round: No new issues found. VerdictReady for merge — diff content is unchanged and byte-identical to the previously verified clean state; no new issues introduced by the merge commits since the last round. (Note: GitHub still reports Structured Review Data (JSON){
"schema_version": "1.1",
"reviewer": "claude",
"commit_sha": "2cbb9e05b8c99ea3ddcb39ab6b3e5ac62f67e614",
"verdict": "CLEAN",
"findings": [],
"detailed_assessment": "The six-file timeout-minutes diff across copilot-instructions.md and five workflow files is byte-identical to the previously verified round-3 content, including the corrected copilot-setup-steps.yml comment.",
"holistic_assessment": "Scope remains confined to timeout additions and documentation; intervening merge commits only bring in unrelated README and WORDLIST changes from PR 61, with no regression risk to publish or preview pipelines."
}Reviewed commit: ecab60a |
|
3 participants
Closes #28.
What changed
Four local jobs had no limit. Sized by the convention
Morrison-Lab/ghauses for the same kinds of work:check-spelling.yaml::checklint-project.yaml::lint-projectpreview.yml::build-deploypublish.yml::build-deployPlus
copilot-setup-steps.ymllowered 55 → 50. It already had a limit so it didn't read as a gap, but 55 is over the cap the issue asks for — making it the one existing violation rather than a missing setting.And the rule is now written into
.github/copilot-instructions.md, per the issue's second bullet.Two things "all workflows" can't mean
Both established by parsing the workflows.
Sixteen of the twenty-one can't have a limit set here at all. Each is a single job calling a
Morrison-Lab/ghareusable workflow, and GitHub rejectstimeout-minuteson a job that usesuses:— the limit has to live in the reusable workflow. Every gha workflow this repo calls already sets its own (20 for the check/lint family, 45 forcleanup-pr-previews, 10 forsummary), so those sixteen are already covered upstream. Adding a limit to them would break parsing, which is why the copilot note spells the exception out. That reconciles with the other half of the arithmetic: 21 total − 5 with local jobs = 16.Two upstream limits are over the cap.
claude.ymlandclaude-code-review.ymleach carry a 60-minute job inMorrison-Lab/gha, and no caller can override them. That gap is now closed upstream by gha#881, which exposestimeout-minutesas an input on both (filed as gha#879). Once that lands, this repo can passtimeout-minutes: 50to both callers and the cap holds end to end.Everything this repo actually controls is at or under 50 today.
Verification
yaml.safe_load, not grepped)gha's owncheck-non-standard-chars.pyThat last one caught a real failure before push: the checker flags em dashes in
.md, my first draft of the copilot note contained two, andcheck-charswould have gone red.Review history
e50588f): two checks red, both in my prose rather than the workflow changes — a list-item splice and two lines missing semantic breaks. Fixed ina23a266, reproduced locally againstgha's own checkers first.c7faad9):NOT_CLEANon a stale comment —copilot-setup-steps.ymlstill said "Timeout after 55 minutes" beside the new value of 50. Fixed in33b6b89. The same round caught this description saying "fifteen" where the real count is sixteen; corrected above.afb6e47): CLEAN. Diff byte-identical to round 2; the only new commits bring the mergedmainin.🤖 Generated with Claude Code
https://claude.ai/code/session_01Lx1hhHu58mXq9DfJsAxkHn