Repository navigation
fix(ci): bound every job and step in this repo's workflows, and keep the bound enforced - #184
Conversation
…the bound enforced (#162) The deploy workflow carried no `timeout-minutes` on any of its 16 steps and no total deadline on its job; the tag-on-merge dispatcher carried none on its 2 jobs or its 1 step. A step with no bound cannot fail - it can only hang, until the runner's own six-hour limit, and the failure it produces names the JOB, never the step. That is the control this issue's remedy names: "a total deadline and a named step boundary, so a stall becomes a time-bounded failure that says where it died". Every job and every step now carries a deadline, sized from the measured runs (55 successful runs of this workflow, 2026-09-22..2026-10-24: 6.0-13.9 min total, median 10.6; the Regenerate step, which is the whole cost of the job, 3.3-10.8 min, median 7.1). A dependency-free checker in .github/scripts/ asserts the rule on this repo's workflows and runs in CI, and its --self-test proves it rejects an unbounded step - including a `timeout-minutes:` written inside a `run:` body, which is text to the shell and satisfies nothing. The over-an-hour stall the issue quotes is NOT reproduced here: the runs it names have since completed, each inside 14 minutes, and the deploy's Regenerate step never exceeded 10.8 minutes across all 55 successful runs. What the issue measured was two runs in flight at once. This change is the missing CONTROL, not a reproduction of the stall - and it is the control that makes the next occurrence a named, bounded failure. The deadlock's other half was already bounded in opencharly/.github since 2026-10-23 (the validator's `arm` job, 75 min, and its head-settle step, 70 min), so the two interlock: this job's total is 60 minutes and the validator's wait is 70, so a stall here fails - and says which step - before the validator gives up waiting for the head to settle. Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (analysed on a live system)
validator INCONCLUSIVE — no review verdict was produced (not a BLOCK; no code finding)The gate could not obtain a review verdict on this run, so it produced no code finding. This is not a BLOCK and the PR is not being rejected — but the required check stays RED on purpose: unreviewed code must never merge.
Root cause (measured in this run). The engine classified the run ITSELF, and its terminal line is quoted below. For a marker this workflow has no bespoke narrative for, the gate does not re-derive a cause from a signature: the engine's own line IS the diagnosis, and the instruction that line carries (every engine class names the knob to change) is the remedy. A signature heuristic must never overwrite what the engine stated. Measured in this run (the engine's own lines): Durable remedy.
Thread policy. A thread gets up to A run that produced no verdict keeps this required check RED: unreviewed code must never merge. Full diagnostics: this run's evidence artifact + job log (not embedded here — embedding a runaway log would feed it into the next review and deepen the collapse). |
|
STATUS — the round's findings are fixed at head Agent: docs-step-deadlines Head: What the validator actually did on the pushed headThe only validator event on The mechanism of the INCONCLUSIVE, from that run's own lines: The gate's own parse step is Finding 1 (blocking) — the body miscounted its own evidence
It is real, and it was false without the fix. Fixed. The new body states the distribution the evidence shows, and the counts it states are now read by an independent YAML reader off the committed file (pasted in the body). RCA.
Finding 2 (the round's second block) — the
|
The round-1 review found a real defect in that PR's own body, and the fresh RCA the defect forced found a deeper one: the tag-on-merge caller in this repo is a DISTRIBUTED COPY of opencharly/.github's canonical template. That repo's bootstrap installs the template into every repo, and its reconcile script binds the copies back to it (opencharly/.github#192) - measured today, read only, with that repo's own tool: 418 repos examined, 415 of them drifted, 3 canonical, and this repo among the drifted. So the deadline added to the COPY in the previous commit was in the wrong place twice over. It duplicates the template's rule (R2), and it would be ERASED by the org's own reconcile pass - a bound that can be silently reverted by the org's tooling is not a bound. The copy is therefore restored to its pre-change content here, and the deadline belongs to the template, which the org-wide sweep already owns (opencharly/.github#256). The checker keeps the rule, on the surface this repo owns, and REPORTS the org-distributed copy instead of asserting it - with the owner and both tracking issues named, because a skip that says nothing is a gap nobody can see. A path named explicitly on the command line is still asserted, so the copy's three unbounded surfaces - two jobs and one step - stay one command away. The mutation harness is committed rather than kept outside the repo, and is wired into the workflow, so the proof that the self-test can fail is reproducible from this repository: five mutants, each removing one mechanism, each asserted to turn the self-test red on the fixture that covers it. The measured figures in the bounds comments are corrected here as well. The previous commit sized them from the LAST 100 runs, a window that slides as new runs land and that excluded the slowest run in the history; measured over a FIXED window (2026-09-22..2026-10-24, 56 successful runs) the longest this job has ever run is 15.15 min, on 2026-09-22, and that is also the longest of all 583 successful runs in the retained history. The 60-minute bound is 4.0x that maximum, and the Regenerate step's 30-minute bound is 2.8x the 10.8 min that step has ever taken across the same 56 runs. Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (analysed on a live system)
Review — PASSI re-derived every material claim in the body and the one comment against the current head ( Correctness against the diff
Honesty of the bodyThe body is unusually candid about its limits and they check out: it explicitly declines to claim the original stall was reproduced, separates the queue-wait limit from the work bound, and labels the PyYAML cross-check and the org-wide sweep as author-side (with the script pasted and the file set re-gatherable). The measurement arithmetic is self-consistent (15.15 × 4.0 ≈ 60; 10.82 × 2.8 ≈ 30; 16 original steps + 1 job = 17 findings on TestsThe self-test and the five-mutant harness are committed and wired into the workflow as the first three steps (after checkout, before the expensive work). The coverage claim — that the checker fails on the unbounded Security & cleanlinessNo secrets, no shell interpolation of untrusted input ( Comment dispositionThe single comment ( Non-blocking observations (not blocks)
No correctness, security, test, or scope defect warrants blocking. Verdict: PASS ollama-cloud/deepseek-v4.1-flash — action-review. |
|
RESOLVED — this PR landed, and the validator's verdict on the merged head is PASS (no code finding). Its two non-blocking observations are dispositioned below, with the evidence each one needs. Agent: docs-step-deadlines The landing
The two non-blocking observations, dispositioned1. "The comment references head The head moved because I amended the unpushed tip's commit MESSAGE before the push, to correct one phrase in it ("the copy's three unbounded jobs" → "the copy's three unbounded surfaces — two jobs and one step", which is what the checker reports). The amend changed the message and nothing else, and the two commits carry the same tree: So every substantive claim in my 07:19 comment on this PR — 3 files, +970 -0, the checker's behaviour, the measurement figures, the scope decision — holds verbatim at the merged head; only the SHA it names was superseded, and this comment supersedes it. The PR's own body was written for 2. "The mutation harness names skip The mutant identifiers are labels, not ordinals, and nothing in the repository refers to a The set is an explicit list in the harness, the docstring's table enumerates the same five, and the summary's count is derived from that list rather than written by hand — so a gap in the alphabet cannot desynchronize anything the harness asserts: print(
f"test_check_step_timeouts: OK ({passed} mutants; every one of them turns "
f"the self-test red)"
)
What this PR leaves behind
Agent: docs-step-deadlines Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (analysed on a live system) |
Summary
Every step and every job in this repo's own deploy workflow now carries a deadline, and a dependency-free control keeps it that way. Head
21b923dc2b7b8bc91f3829566739aa121037a6ca— 3 files changed, +970 -0.The defect, in one sentence: the
deployworkflow carried notimeout-minuteson any of its 16 steps and no total deadline on its one job. A step with no bound cannot fail — it can only hang, until the runner's own six-hour limit, and the failure it produces names the JOB, never the step. That is exactly the remedyopencharly/docs#162names: "a total deadline and a named step boundary, so a stall becomes a time-bounded failure that says where it died."What this change is. Three files, one idea:
.github/scripts/check_step_timeouts.py(new, 718 lines) — the control. It reads this repo's workflow files with a dependency-free indentation reader (the repo carries no YAML parser:package.jsonholds astro, starlight, sharp and wrangler) and asserts that every job has a total deadline and every step has a step deadline shorter than its job's — a step bound at or above its job's is dead code, because the job's deadline fires first and the step never gets to name itself. It ships with a--self-testmode, and both are wired intodeploy.ymlas its first three steps: the check before the expensive work, then two proofs that the check can fail..github/scripts/test_check_step_timeouts.py(new, 178 lines) — the mutation harness. It removes each of the checker's five mechanisms in turn and asserts that the self-test then turns RED on the fixture that covers exactly that mechanism; a mutant that still passes is a mechanism the self-test does not reach, and the harness fails on it. Each mutation is an asserted single-string replacement — the anchor must occur exactly once in the committed source, so a moved anchor fails loudly instead of masquerading as a passing mutation..github/workflows/deploy.yml— thedeployjob's total deadline (60 min) and a deadline on each of its 19 steps.The scope decision, and why the copy is not edited here. One workflow file in this repo is not this repo's to fix:
.github/workflows/tag-on-merge.ymlis a distributed copy of opencharly/.github's canonical template (.github/workflows/tag-on-merge-dispatcher.yml), which that repo's bootstrap installs into every repo and whose reconcile script binds the copies back to it. A deadline added to the copy would duplicate the template's rule (R2) and would be erased by the org's own reconcile pass — a bound that the org's tooling can silently revert is not a bound. The copy is therefore restored to its pre-change content, and the checker REPORTS it — naming the owner and both tracking issues — instead of asserting it, because a skip that says nothing is a gap nobody can see. A path named explicitly on the command line is still asserted, so the copy's three unbounded surfaces — two jobs and one step — stay one command away (pasted in How tested below).The bounds are sized from the measured runs, not chosen. The measurement window is FIXED and dated, because a sliding window moves under the numbers written next to it: over 2026-09-22..2026-10-24 (56 successful runs) the job's median is 10.62 min and its longest is 15.15 min, on 2026-09-22 — and that 15.15 min is also the longest of all 583 successful runs in the retained history, back to 2026-08-03. The job's 60-minute total is 4.0x that maximum; the
Regeneratestep's 30-minute bound is 2.8x the 10.82 min that step has ever taken across the same 56 runs. Real work does not trip either bound, and both fire long before the runner's own six-hour limit.The interlock with the org validator. The
deploycheck is one of the checks the org validator'sarmjob waits for, and that job has been bounded since 2026-10-23 (opencharly/.github#236): thearmjob at 75 minutes, its head-settle step at 70. This job's total is 60 — under the validator's wait — so a stall in this workflow now fails, and says WHICH step, before the validator gives up waiting for the head to settle. The deadlock's other half is therefore already landed; this PR is the docs-repo half of it.What this PR does NOT claim. It does not claim to have reproduced the over-an-hour stall the issue describes. The runs that issue names have since completed, and each of them finished well inside an hour:
So the stall's own record is two checks in flight at once, each of which concluded inside 14 minutes — and across all 56 successful runs in the fixed window the
Regeneratestep never exceeded 10.82 minutes. What the issue measured is the shape the bound has to answer, not a single step that never returned. The claim this PR makes is the missing CONTROL, not a reproduction of the stall: every one of those steps ran with no deadline, so the next occurrence of that shape has nothing to fail against until this lands.And one honest limit of the mechanism, stated rather than papered over. A job's
timeout-minutesstarts when the job STARTS RUNNING; it does not bound the time the job spends queued for a runner. The retained history shows that shape on the validator's own side — its run 48 was created at 06:57:53Z and its job did not start until 08:18:53Z, 81 minutes later:So the bound this PR adds is a bound on the WORK, and the queue wait that produced the 81-minute stall is a different thing — owned by the runner, not by this workflow, and already bounded on the validator's side by the 70-minute head-settle step.
How tested
Every block below is a verbatim slice of the run that gated the committed tree,
21b923dc2b7b8bc91f3829566739aa121037a6ca, in this repository's worktree.The control, on the committed tree — exit 0, and the org-distributed copy is reported rather than asserted:
The checker's own self-test — exit 0:
The mutation harness: five mechanisms removed, one at a time, and the self-test turns red on each — exit 0:
The coverage, which fails without the change. The same checker, run against
origin/main'sdeploy.yml— 17 findings (1 job, 16 steps), exit 1:The org-distributed copy, asserted EXPLICITLY rather than skipped — its three unbounded surfaces (two jobs and one step), exit 1:
An independent reading of the same files. The checker parses YAML with its own indentation reader; PyYAML is a second, independent reading of the same bytes, and the two must agree on the IDENTITY of every finding (kind, job, step), not merely on the count. Over 15 workflow files — this repo's two, and the opencharly/.github repo's own 13, fetched fresh over HTTPS by the first line of the block — 0 mismatches, exit 0. This cross-check is an author-side verification and NOT a committed CI step: this repository carries no YAML dependency, and adding PyYAML to the workflow to prove a dependency-free checker would put the dependency back in. The tool that produced the block is author-side and is not committed to this repository, so its full source is pasted here — the reader can save it and re-run the command, and the file set is re-gatherable by the first line:
The cross-check's negative control. With the checker's cross-job step bound removed (the
mutBmutant, emitted from the committed harness), the same cross-check finds a mismatch — in the org's ownpr-validator.yml, whosevalidatejob's twelfth step the mutant reads as unbounded and PyYAML does not — and exits 1, so a clean cross-check means something. The paths are scratch paths: the harness writes its mutants into a temporary tree that MIRRORS this repo's layout, because the checker derives its repository root from its own path, and a mutant written anywhere else would compute a different root and turn the checks red for the wrong reason:This repo's own existing tests, unchanged by this PR — exit 0:
The artifact check: no stray files, clean tree, and the org-distributed copy byte-identical to
origin/mainagain.Every count this body states, read by an INDEPENDENT YAML reader from the committed file (the checker has its own indentation reader; PyYAML is a second reading of the same bytes):
The measurements the bounds are sized from — the
deployjob's total over a FIXED window, the same over the WHOLE retained history, theRegeneratestep over the same 56 runs, and this repo's own 141 successful runs of the caller that invokes the org's reusable workflow (whosetagandwait-for-mergejobs are the ones the TEMPLATE must bound). The fixed window ends on 2026-10-24, so the runs that land after it cannot move the numbers written next to them; the whole-history figure (583 runs, first successful run 2026-08-03) includes them, and its maximum is the same run:The org-wide exposure, re-measured at this head with this PR's own committed checker. The 8 repos of the filed issue, their 27 workflow files fetched fresh over HTTPS: the same 174 findings — 139 unbounded steps, 35 unbounded jobs — and still no dead bound (a step bound at or above its job's) anywhere in them:
The org's own read-only reconcile tool, run over the org, which is what makes the copy's owner the right place for the bound — one template edit reaches every copy (418 repos, 415 of them drifted). It was run from a reconstruction of opencharly/.github's
scripts/and its canonical template. Both files are byte-identical to the org repo's own: the template's blob sha is375cb1edcff9(the org repo's owncontentssha for that path) and the script's sha256 isb9ce1b918c8bf1d1c3a96996bb0477ce79ace540d5f0872056579157e39e1055, the sha256 ofscripts/reconcile-tag-on-merge-callers.shfetched from the org repo'smain. So the reconstruction is the same file the tool reads in place, and the run below is the org's own tool over the org. Its report block, verbatim:The same run continued — the 56 repos
discover_reposcould not see (archived, fork, or a non-maindefault), then the per-repopin-mutableandprose-driftedlists, and finally its closing note:The change itself, and the commit the evidence belongs to:
Rulebook compliance
/charly-internals:git-workflow(every git andghaction here),/charly-internals:strict-policyand/charly-internals:root-cause-analyzer(the review round's findings and the RCA in the round's PR comment),/charly-tools:docs-siteand/charly-internals:skills. The harness cannot resolve/charly-build:docs("Unknown skill"), so it was read by path from the corpus instead —marketplace/build/skills/docs/SKILL.md— which is the fallback the rulebook states for a harness that cannot resolve the/family:skillform. It was read during this round, before this round's first edit, and it confirmed the scope of the change: the docs repo'sdeploy.ymlis the sole owner of generation, build and publish, and no content or generated page is touched here.## Filed issues..github/scripts/check_step_timeouts.pyand is not re-implemented in the workflow, in the copy, or in a second checker; the org-distributed copy is deliberately NOT given its own copy of the rule, which is the duplication R2 forbids.origin/mainagain — proved by the claim-keyed diff pasted in the artifact check above (git diff --stat origin/main -- .github/workflows/tag-on-merge.ymlprints nothing). No shim, alias or stale deadline survives in the tree.git statuswas stated before every mutation; the branch isfeat/workflow-step-timeouts-162in a session worktree at.worktrees/docs162/docs; no force-push, no bypass, no push tomain, and no file outside this repo touched.origin/main'sdeploy.ymlreports 17 findings and exits 1 (pasted above), and the mutation harness proves the self-test can fail. The changed paths were executed live against the real workflow files, with the output and the exit codes retained above.charly/pr-validator; this repo carries no per-repo candy gate and nodisposable: truetarget. The part of that workflow this change adds — the deadline check, its self-test and the mutation harness — was run on the committed tree above, with its output and exit codes retained, as were this repo's two existing test suites. The workflow's other half (the pinned charly fetch, the regeneration, the site build, the deploy) is not reproducible outside CI: it clones and builds the pinned toolchain, and it is proven by this PR's owndeploycheck, which is PENDING at this head.The change is exactly these three files, and nothing else:
Change classification
analysed on a live system— the changed paths (the checker, the self-test, the mutation harness) ran live against this repository's real workflow files with their output and exit codes retained above, and the numbers the bounds are sized from come from this repository's real run history. The tier is notfully tested and validatedbecause the workflow's own live CI run at this head is the PR's pendingdeploycheck, not a result this body can paste. The round-1 head'sdeploycheck ran the whole workflow green in CI, but it did not include the mutation-harness step this round added, so it is evidence about the round-1 head and not about this one.deployworkflow and the org-widecharly/pr-validator. Of the workflow, the part this change adds — the deadline check, its self-test and the mutation harness — ran on the committed tree with its output and exit codes retained above, as did this repo's two existing test suites; the workflow's other half (the pinned charly fetch, the regeneration, the site build, the deploy) is not reproducible outside CI, and it is proven by this PR's owndeploycheck, which is PENDING at this head. The org-widecharly/pr-validatoris the gate that is also PENDING at this head.Filed issues
wait-for-mergejob and its one step, and thetagjob, carry no deadline. The bound belongs in the template, and the template is read by 418 repos (415 of them drifted from it, per the org's own reconcile report above). This repo's own 141 successful runs of the caller are the measurement pasted above: the whole run's maximum is 5.55 min, thetag / tag-and-changelogjob's maximum is 1.88 min, and thewait-for-mergejob's maximum is 5.23 min over the 124 runs that have one — so a bound in the template can be sized from data rather than guessed. That is this repo's population, not the org's; the org-wide population is that issue's own sweep to gather. The issue's per-repo table reproduces exactly at this head: 174 findings across 27 files in those 8 repos, 139 unbounded steps and 35 unbounded jobs, and no dead bound anywhere (pasted above).deploy.yml" and pasted the output that shows one job — the body's own evidence disproved its sentence, and only the live validator caught it. That issue's proposed rule targets a count declared INSIDE a fence; the variant measured here declares the count in PROSE and enumerates it in a fence, so the same root cause ("a count and its enumeration are two representations of one fact") needs a second arm on the same rule. The evidence is added to that issue rather than filed as a new one, so the rule has one home.Agent: docs-step-deadlines
Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (analysed on a live system)