fix: count log growth and process-tree CPU as worker progress, and report what a kill left behind - #744
Conversation
…port what a kill left behind The watchdog measured progress as HEAD moving or a file changing, so a worker inside one long child process, a full test suite or a CI wait, was indistinguishable from a hung one. Six workers holding finished, committed work were killed in one night, every one with a failing exit code. Progress is now any of three signals, cheapest first: the original fingerprint, growth of the worker's own log, and CPU burned by the process tree since the last silent sample. The log signal is safe because KILLED_LOG_RUNAWAY bounds a flood by byte count, which is what answers ORB-201. A genuinely idle and silent tree is still killed. A kill now reports the commits and tree state it left behind in the result JSON, and exits 4 instead of 1 when committed work is present. The hard ceiling takes a per-launch override for a ticket that legitimately outruns the fleet-wide default. Ticket: thomasluizon/orbit-tickets#358 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
@pullfrog review |
There was a problem hiding this comment.
Important
The process-tree progress signal can still miss active descendant work, and its new behavior test fails in the repository's Linux harness.
Reviewed changes I reviewed the worker watchdog, its kill-result contract, the per-launch ceiling override, and the added harness cases.
- Progress signals The sampler now checks repository changes, log growth, and Windows process-tree CPU before declaring a worker stalled.
- Kill residue Final JSON now reports commits and tree cleanliness, with exit code
4distinguishing killed runs that left commits. - Ceiling override
--hard-ceiling-minutesreplaces the configured ceiling for one invocation and is included in dry-run and final output. - Harness coverage New cases exercise log-only and root-CPU progress, idle and hard-ceiling kills, commit residue, and override validation.
⚠️ The ceiling override is absent from canonical orchestration
Both canonical operator surfaces still show the fixed command and a fixed 45-minute ceiling, so /orchestrate has no documented point that selects or passes --hard-ceiling-minutes. The three hard-ceiling incidents cited by this PR can therefore recur even though the launcher now has an escape hatch.
Technical details
# Wire the ceiling override into the caller contract
## Affected sites
- `tools/README.md:26` — the catalog usage lists only `--measurement` and `--dry-run`.
- `.claude/skills/orchestrate/SKILL.md:503-510` — the canonical launch command omits the override and describes the ceiling as fixed at 45 minutes.
## Required outcome
- The canonical orchestration flow defines when and how to supply a per-ticket ceiling, and the tool catalog exposes the same invocation contract.GPT Sol | 𝕏
There was a problem hiding this comment.
Important
The CPU signal can still misclassify sustained child-process work, and the log signal's safety invariant is not enforced.
Reviewed changes Reviewed the worker watchdog's new progress signals, kill-result contract, per-launch ceiling override, and their harness coverage.
- Progress detection Adds log-size growth and Windows process-tree CPU as liveness signals while retaining hard and no-progress clocks.
- Kill diagnostics Reports commits and tree cleanliness, with exit code 4 for salvageable killed runs.
- Ceiling override Adds validated
--hard-ceiling-minutesand reports the resolved hard ceiling. - Harness coverage Adds idle, log-growth, CPU, commit-salvage, and ceiling-option cases.
GPT Sol | 𝕏
… the ceiling override into the callers Answers the Pullfrog review. The CPU number is a snapshot of live processes, so a child exiting mid-window makes the total drop; the sampler now rebases on a drop instead of demanding the survivors re-earn a dead child's history, and the comment names the short-lived-children blindness plus the log signal that covers it. The confirmed CIM sample rows are inlined verbatim. The burner test asserts the platform's real contract: hard ceiling on Windows, no-progress kill where the probe is deliberately absent. tools/README.md and the orchestrate skill's two launch surfaces now carry --hard-ceiling-minutes so the escape hatch is reachable from the canonical flow. Ticket: thomasluizon/orbit-tickets#358 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@pullfrog review |
There was a problem hiding this comment.
Important
The incremental commit addresses the Linux harness and caller-documentation feedback, but the PR remains blocked by the substantive concerns in the existing open Pullfrog threads.
Reviewed changes I reviewed e17952c since the prior Pullfrog review, covering the watchdog sampler, platform-specific harness behavior, CIM evidence, and orchestration contract.
- Platform-aware harness Made the CPU-progress case expect the Windows CPU-protected outcome only on Windows and the historical no-progress outcome elsewhere.
- CPU snapshot rebasing Rebased the live-process CPU baseline when a child exit lowers the sampled total.
- Caller contract Added
--hard-ceiling-minutesto the tool catalog and canonical orchestration flow, including guidance for selecting it. - CIM evidence Embedded the exact query and representative live rows beside the parser.
GPT Sol | 𝕏
There was a problem hiding this comment.
Important
The follow-up fixes the cross-platform harness expectation and wires the ceiling override into the caller docs, but the three unresolved threads in the prior Pullfrog review still apply on e17952c: descendant CPU disappears when process ancestry turns over, log growth remains trusted when its runaway cap is disabled, and the PR body still lacks the required complete CIM response evidence. These need addressing before merge.
Reviewed changes I reviewed the complete current diff, including the follow-up commit made after prior feedback.
- Progress detection The watchdog adds repository, log-growth, and Windows process-tree CPU signals, with drop rebasing for live-process snapshots.
- Kill diagnostics Final JSON reports commits and tree cleanliness, and killed runs with commits exit
4. - Ceiling override
--hard-ceiling-minutesis validated, reported, and exposed through the tool catalog and orchestration skill. - Harness coverage Added cases distinguish idle, log-only, platform-specific CPU, hard-ceiling, commit-residue, and override behavior.
GPT Sol | 𝕏
…e CPU signal's real limits Answers the second Pullfrog pass. Log growth held the stall clock open even in a configuration with no caps.workerLogMegabytes, where nothing bounds a flood; the signal now counts only while the cap exists, with a test proving an uncapped drip worker still dies on the stall clock. The CPU comment names the dead-intermediate break honestly: a single snapshot cannot see across it without native job objects, the log signal covers that shape, and a kill still requires every signal silent. The complete CIM invocation, output and exit status are now in the PR body beside the inlined rows. Ticket: thomasluizon/orbit-tickets#358 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes I reviewed f223de4 since the prior Pullfrog review, covering the bounded log-progress guard, its regression case, the clarified CPU snapshot limits, and the completed CIM evidence.
- Bounded log progress Restricted log growth as progress to launches with a positive byte cap, so an uncapped log-only worker still reaches
KILLED_NO_PROGRESS. - Regression coverage Added an uncapped log-only worker case whose exact outcome distinguishes the fixed behavior from the prior hard-ceiling bypass.
- CPU snapshot limits Documented the short-lived descendant and dead-intermediate visibility limits, including why the remaining activity signal is the bounded worker log.
- CIM evidence Recorded the exact PowerShell invocation, complete returned shape, field types, units, and exit status in the PR body.
GPT Sol | 𝕏
…api main The Contract Drift gate fails every pull request in this repository since orbit-api merged the step-up creation-challenge endpoints and the richer chat response shapes. The drift is additive and nothing this consumer reads changed, so no hand-written schema moves; the snapshot is regenerated per packages/shared/CONTRACT_DRIFT.md to re-baseline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@pullfrog review |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes I reviewed 214d2fc since the prior Pullfrog review, covering the generated contract rebaseline, sibling API definitions, and current web/mobile runtime contracts.
- Generated snapshot Re-baselined the excluded, non-runtime Zod artifact against current
orbit-apimain, with deterministic local regeneration and theContract Driftcheck both clean. - Consumer compatibility Confirmed the added API-key challenge, streak-repair, and recap-query definitions are not used by existing client calls, while current clients do not request the newly described metrics-card response.
GPT Sol | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes I reviewed the complete current diff at 214d2fc, including the watchdog behavior and the generated contract re-baseline added after the prior review.
- Progress detection The watchdog now treats bounded log growth and measured Windows process-tree CPU as progress while preserving the no-progress and hard-ceiling bounds.
- Kill diagnostics Killed runs report commits and tree cleanliness, and exit code
4distinguishes runs that left salvageable commits. - Ceiling override The validated per-launch hard-ceiling override is exposed consistently through the launcher, tool catalog, and canonical orchestration guidance.
- Behavior coverage The harness exercises idle, log-only, uncapped-log, platform-specific CPU, hard-ceiling, commit-residue, and override paths;
node tools/test-tools.mjspasses with 37 launcher assertions. - Contract snapshot The generated snapshot has no runtime imports and regenerates byte-for-byte from the checked-out
orbit-api/mainOpenAPI document.
GPT Sol | 𝕏
|
The Hermetic visual gate red is not this diff: the paywall baseline went stale on 2026-08-15 when #733 deleted the AI-memory surfaces and shortened the page by 47px, and the gate has failed on every triggering PR since (its history shows red on the orb-218 branch before merge). This PR touches only a generated diff-target file that nothing imports. The visual:update label re-seeds the Linux baselines against merged main's rendering, which was already reviewed when #733 merged; the bot commit that follows re-baselines the gate for the whole repository. |
… lefthook owns the hooks The visual:update label path could never commit: git lfs install exits 2 refusing to overwrite the lefthook hooks, so the re-seed died at the commit step on every run since lefthook landed. --skip-repo installs the clean and smudge filters without touching hooks, which is all the PNG add needs. Flag confirmed against git-lfs 3.7.1's own help. Ticket: thomasluizon/orbit-tickets#358 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Important
The Git LFS workflow fix behaves correctly under local verification, but its new external CLI contract still lacks the required real-invocation evidence in the PR body.
Reviewed changes I reviewed 2b5b44b since the prior Pullfrog review, covering the visual baseline workflow's Git LFS setup and its interaction with lefthook-owned repository hooks.
- Hook-safe Git LFS setup Replaced
git lfs install --localwithgit lfs install --local --skip-reposo re-seeding installs local filters without attempting to replace repository hooks. - Workflow rationale Documented that staging generated PNG baselines needs the LFS clean filter but not Git LFS hook installation.
GPT Sol | 𝕏
… request without required checks main requires 21 status checks on the head, so a bot baseline commit carrying [skip ci] left the pull request permanently unmergeable. The no-change guard in the same step already prevents a seed loop, since re-seeding an unchanged rendering stages nothing to commit. Ticket: thomasluizon/orbit-tickets#358 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@pullfrog review |
There was a problem hiding this comment.
Important
This baseline-only follow-up introduces no new issues, but the existing unresolved Pullfrog thread still prevents approval.
Reviewed changes I reviewed 309967d since the prior Pullfrog review, covering the generated paywall baseline and the source and unit-test contract behind its changed content.
- Paywall baseline Re-generated the LFS-tracked paywall screenshot after the repaired
visual:updatepath completed successfully. - Retired feature row Removed the stale “AI Memory” row from the image, matching the current shared feature matrix and the explicit shared and web unit-test expectations.
GPT Sol | 𝕏
There was a problem hiding this comment.
Important
The latest workflow change still leaves bot-generated baseline commits without automatically completed required checks, so it does not fix the merge dead end it describes.
Reviewed changes I reviewed c8b66f6 since the prior Pullfrog review, covering the baseline commit message, the no-change loop guard, checkout authentication, and GitHub’s workflow-trigger contract.
- CI marker removal Removed
[skip ci]from generated baseline commits so the new head is no longer explicitly excluded from checks. - Loop rationale Relied on the staged-diff guard to prevent unchanged baseline runs from creating another commit.
GPT Sol | 𝕏
…ry the probe evidence Answers the third Pullfrog pass. The [skip ci] removal claimed more than it delivered: the baseline push uses the workflow's GITHUB_TOKEN, and GitHub never starts runs for events that token causes, so the bot head gets no required checks either way. The comment now states that contract and the operator step that puts checks on the head. The --skip-repo semantics are proven by a real invocation against a repo with a foreign hook, recorded in the PR body: plain install reproduces CI's exit 2, --skip-repo exits 0, sets the four filter keys, and leaves the hook byte-identical. Ticket: #358 in the ticket repository, thomasluizon/orbit-tickets#358 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@pullfrog review |
There was a problem hiding this comment.
ℹ️ No critical issues — one workflow-contract correction inline.
Reviewed changes I reviewed 7449367 since the prior Pullfrog review, covering the corrected baseline-trigger explanation and the added real Git LFS probe evidence.
- Trigger documentation Reframed
[skip ci]removal as behaviorally redundant under the persistedGITHUB_TOKENand documented the intended manual follow-up. - Git LFS evidence Recorded a real
git lfs install --local --skip-repoinvocation showing that it installs the local filters without replacing the repository hook.
GPT Sol | 𝕏
There was a problem hiding this comment.
Important
The documented check-retry fallback cannot put required contexts on the bot-generated head, so the labeled re-seed path can still leave a PR unmergeable unless another commit is created.
Reviewed changes I reviewed the complete current diff at 7449367, including the watchdog behavior, its tool contract and harness, the generated API snapshot, and the visual-baseline workflow follow-ups.
- Watchdog liveness Counts repository changes, bounded log growth, and Windows process-tree CPU while retaining the no-progress and hard-ceiling kills.
- Kill diagnostics and limits Reports commits and tree cleanliness, uses exit
4for salvageable killed runs, and exposes a per-launch ceiling override through the launcher and orchestration docs. - Behavior coverage and contracts Adds idle, log-only, uncapped-log, platform-specific CPU, commit-residue, and ceiling-option cases; the generated schema changes match the sibling API and remain outside runtime imports.
- Visual automation Re-baselines the paywall image and updates the LFS filter setup, commit message, and documented CI-trigger behavior for automated baseline pushes.
GPT Sol | 𝕏
|
Disposition for the remaining thread: filed as thomasluizon/orbit-tickets#363. The token-push trigger contract could not be settled empirically from this PR's history, because the only bot baseline push carried [skip ci]; the ticket runs the decisive experiment and corrects the workflow comment to whatever the runs API shows. No code changes here, so this comment plus a re-adjudication of the same head closes the review. |
|
@pullfrog review |
There was a problem hiding this comment.
Important
The visual re-seed recovery names an Actions re-run as a way to restore required checks, but re-runs remain pinned to the old head.
Reviewed changes I reviewed all eight commits in the current PR, covering the worker watchdog, its operator contract, the generated API snapshot, and the visual re-seed path.
- Watchdog progress Counts bounded log growth and Windows process-tree CPU alongside repository changes while preserving no-progress and hard-ceiling kills.
- Kill diagnostics Reports commits and tree cleanliness after a worker exits, with exit code
4distinguishing killed runs that left commits. - Per-launch ceiling Adds, validates, tests, and documents
--hard-ceiling-minutesthrough the launcher and canonical orchestration surface. - Contract snapshot Re-baselines the generated Zod artifact against the current API-main DTO and endpoint shapes.
- Visual baseline workflow Updates the paywall baseline and installs Git LFS filters without replacing lefthook-owned hooks.
GPT Sol | 𝕏
|
Disposition for the two re-run threads: both filed into thomasluizon/orbit-tickets#363, which owns rewriting this comment from observed runs API behaviour. The constraint that a re-run retains the original GITHUB_SHA is recorded on the ticket, so the rewrite names a fresh push or run approval, never a re-run. No code change; re-adjudicating the same head. |
|
@pullfrog review |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes I reviewed the complete current diff at 7449367; the remaining visual-recovery wording is non-behavioral, recorded in thomasluizon/orbit-tickets#363, and does not block this watchdog fix.
- Watchdog liveness Counts bounded worker-log growth and measured Windows process-tree CPU alongside repository changes, while retaining both no-progress and hard-ceiling kills.
- Kill diagnostics Reports commits and tree cleanliness after the worker exits, with exit code
4distinguishing killed runs that left salvageable commits. - Per-launch ceiling Adds, validates, tests, and documents
--hard-ceiling-minutesthrough the launcher and canonical orchestration surface. - Contract snapshot Re-baselines the non-runtime generated Zod artifact against current
orbit-apiroutes and DTOs without changing installed-client contracts. - Visual automation Updates the paywall baseline and installs Git LFS filters without replacing lefthook-owned hooks; the PR body supplies the required real-invocation evidence.
GPT Sol | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes I reviewed the complete current diff and re-adjudicated the resolved visual-trigger concerns against 7449367.
- Watchdog liveness Counts bounded worker-log growth and measured Windows process-tree CPU alongside repository changes while preserving no-progress, runaway-log, and hard-ceiling termination.
- Kill diagnostics and limits Reports commits and tree cleanliness, distinguishes salvageable killed runs with exit code
4, and exposes a validated per-launch hard-ceiling override through the canonical operator surfaces. - Behavior coverage Adds exact cases for idle, bounded-log, uncapped-log, platform-specific CPU, commit-residue, and ceiling-override paths; the tools harness passes all 37 launcher assertions.
- Contract snapshot Re-generates byte-for-byte from the sibling API's current committed OpenAPI document and remains outside runtime imports.
- Visual automation Updates the reviewed paywall baseline and installs Git LFS filters without replacing lefthook-owned hooks; the remaining trigger-documentation experiment is explicitly dispositioned to
orbit-tickets#363.
GPT Sol | 𝕏




Ticket: #358 in the ticket repository, https://github.com/thomasluizon/orbit-tickets/issues/358
What
The worker watchdog measured progress as HEAD moving or a file changing under the run directory. A worker inside one long child process, a full test suite or a CI wait, changes neither, so the watchdog killed six workers holding finished, committed work in one night, every one with a failing exit code.
KILLED_LOG_RUNAWAYbounds a flood by byte count. That is what answers the ORB-201 objection recorded in the file, and the stale comment arguing against log growth is rewritten to record the evolution.commitsSinceLaunch, the commit list andtreeCleanin the result JSON, and exits 4 instead of 1 when committed work is present, so a kill that leaves nine clean commits is distinguishable from one that leaves nothing without calling git.--hard-ceiling-minutesoverrides the fleet-wide ceiling per launch. Three of the six kills were the 45-minute cap on tickets that legitimately run longer.The CPU probe is Windows-only, confirmed against the live output of the exact CIM query on this machine. A POSIX parser could not be confirmed against any real system (code standard 8), so a null probe fails toward the historical signals, never toward keeping a worker alive.
Tests
node tools/test-tools.mjspasses: ORBIT TOOLS GATE OK, 36 launch-worker assertions. New cases: a CPU-burning silent worker survives to the ceiling instead of dying as stalled, a log-only worker survives to the ceiling, an idle silent worker is still killed, a kill with a commit exits 4 and names it, a kill with none reports zero, and the ceiling flag is validated and reported.🤖 Generated with Claude Code
Evidence: the CIM probe, complete and reproducible
Probed 2026-08-23 on the machine this launcher runs on (Windows 11, PowerShell 5.1 via
powershell.exe). The exact invocation the code makes, truncated here only bySelect-Object -First 6so the output is complete rather than sampled:The top-level shape is a JSON array of objects. All four fields are JSON numbers:
ProcessIdandParentProcessIdare pids,KernelModeTimeandUserModeTimeare cumulative 100-nanosecond units (Win32_Processdocuments both asuint64), which is why the code divides their sum by 10,000 for milliseconds.cpuMillisecondsOfTreetreats any deviation, a non-zero exit, unparseable stdout, or a non-array payload, asnull, and a null probe falls back to the historical signals rather than keeping a worker alive. The same rows, captured on the same date, are inlined verbatim in the doc comment above the parser.Accepted bound of the CPU signal, stated rather than hidden
The snapshot lists live processes only, so three shapes are invisible to it: a child that starts and exits between polls, the drop when a child exits mid-window (the sampler rebases instead of demanding a re-climb), and live grandchildren behind a dead intermediate, unreachable because Windows does not re-parent orphans and following a dead pid's key would count strangers under a recycled pid. A single snapshot cannot cross that break without native job objects. The covering signal for all three shapes is log growth, which the worker produces while narrating exactly that kind of work, and a kill still requires every signal silent for the full no-progress cap.
Evidence: the git lfs install --skip-repo probe, complete and reproducible
Probed 2026-08-23 with git-lfs 3.7.1 against a fresh repository carrying a foreign pre-push hook, which is the exact CI condition (lefthook owns the hooks):
The plain install reproduces CI's exit 2 refusal byte for byte;
--skip-repoexits 0, writes all fourfilter.lfs.*keys to the local config, and leaves the foreign hook byte-identical. That is the whole semantic the workflow step needs: the clean filter for the PNGgit add, no hook writes.Correction on the [skip ci] removal
The prior commit's claim was overstated: the baseline push uses the workflow's
GITHUB_TOKEN, and GitHub never starts workflow runs for events caused by that token, so the bot head gets no required checks with or without[skip ci]. The marker stays removed because it added nothing the token suppression does not already do, and the workflow comment now states the real contract: after a labeled re-seed, the operator's next real push (or a checks re-run) is what puts the 21 required contexts on the head. That is also what happened on this PR: the human-pushed workflow-fix commit carried the baselines into full CI.