Let a test command raise its watchdog idle threshold - #442
Merged
Merged
Conversation
`gadgets-with-timeout --idle 60` reaps `workshop-backend`'s unit suite under CI contention. Import dominates that suite's runtime and vitest's non-TTY reporter only prints on file completion, so a healthy run goes quiet for long stretches -- no hang, just no output. Widen the command the task builders take to `TestCommand`, so one command can raise its own idle threshold without bumping the default for the other ~25 suites (which would also bust every package's vp cache). `withTestTimeout` stays unary, since `vitestTaskWithExclusions` hands it to `Array.prototype.map`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
LGTM! |
Maximo-Guk
marked this pull request as ready for review
September 4, 2026 03:15
Preview:
|
The HEAD commit blamed vitest's non-TTY reporter, but that is not CI-specific: vp pipes every task's stdout, so vitest is non-TTY locally too and prints one line per completed file everywhere. What differs in CI is the machine -- a 4-vCPU runner at vp's concurrency floor, with the backend unit suite (import 261s, tests 19s) overlapping three other workerd fleets. The 60s cliff is contention, not reporter behaviour. 120s clears the longest healthy silence seen in a green run (23s) by a wide margin while keeping the wedge detection tighter than 180. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
LGTM! |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Maximo-Guk
force-pushed
the
maximo/test-idle-timeout-override
branch
from
September 4, 2026 15:25
97ad90c to
253baeb
Compare
|
LGTM! |
`TESTS_WITH_TIMEOUT_DISABLE=<anything>` makes `with-timeout.ts` skip arming both the idle and wall-clock timers, leaving the stdio piping and signal relay in place: for a CI whose job-level timeout already bounds a wedge, or a deliberately unsupervised local run. A wedge is then killed anonymously by whatever is outside, with no surviving-tree diagnostic. Under vp a cached task sees no ambient variable it does not declare, so the shared task builders declare it in `env` on every vitest `test` task, and the two hand-declared cached tasks that wrap the watchdog (`@gadgets/scripts#test`, `workshop-backend#build:integration-worker`) spread the same list. `env` fingerprints the value, so toggling the switch is a cache miss and a supervised run never replays a pass that only finished because nothing was watching. `scripts/vitest-task.test.ts` checks every cached task wrapping `gadgets-with-timeout` declares it. Not wired into CI here; that is a separate decision. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
run-dev-server spawns `build:configurator` and `build:app:dev` as two `vp run -r --cache` children at once, each with the same `vpRunEnv()` result, so each got the whole machine-derived VP_RUN_CONCURRENCY_LIMIT and together they claimed the budget twice over. `vpRunEnv` now takes an options object with a `concurrentRuns` count, and the new `splitConcurrencyLimit` divides the resolved limit between that many runs: floored at vp's own default of 4, since that is what each child used with the variable unset, so a small machine is unchanged; and capped at the whole limit, so an explicit value below the floor is never raised past what was asked. A value that is not a positive integer passes through untouched for vp to report. The dev server lists its two runs and derives the count from the list; `run.ts` moves to the new signature; run-local and the release build are unchanged (bare `vpRunEnv()` still works). The mechanism previously lived only in gadgets-internal's wrapper over this module, which can now import `splitConcurrencyLimit` instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Maximo-Guk
force-pushed
the
maximo/test-idle-timeout-override
branch
from
September 4, 2026 15:29
253baeb to
b6e44b5
Compare
|
@Maximo-Guk Bonk workflow was cancelled. View workflow run · To retry, trigger Bonk again. |
3 similar comments
|
@Maximo-Guk Bonk workflow was cancelled. View workflow run · To retry, trigger Bonk again. |
|
@Maximo-Guk Bonk workflow was cancelled. View workflow run · To retry, trigger Bonk again. |
|
@Maximo-Guk Bonk workflow was cancelled. View workflow run · To retry, trigger Bonk again. |
ndisidore
approved these changes
Sep 4, 2026
twinprime19
added a commit
to twinprime19/cloudflare-os
that referenced
this pull request
Sep 10, 2026
Absorbs cloudflare/cloudflare-os main 54d5d8b: composer skill picker and pills (cloudflare#422, cloudflare#423), Workers compatibility date bump (cloudflare#448), gatekeeper-kit replayable runs (cloudflare#460), workspace-sheets xlsx export (cloudflare#433), multi-tab Google Docs (cloudflare#450), and fixes cloudflare#442, cloudflare#446, cloudflare#447, cloudflare#454. Conflicts resolved in the chat composer and slash-command picker: upstream's structure with the fork's Lingui wraps and the workspace idle-pause resume call re-applied. Upstream's new React tests mount through the fork's i18n root. Claude-Session: https://claude.ai/code/session_017psYJXWDchbjuq7DarzGkV
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
gadgets-with-timeout --idle 60has been observed reapingworkshop-backend's unit suite in CI. Nothing about CI mode prints less: vp pipes every task's stdout, so vitest is non-TTY everywhere and prints one line per completed file. What differs is the machine — a 4-vCPU runner at vp's concurrency floor, where the import-dominated backend suite (import 261s, tests 19sin a green run) overlaps three other workerd fleets and its healthy silences stretch past 60s.TestCommand(string | { command, idleSeconds }), so one command can raise its own idle threshold without bumping the default for the other suites. The backend unit suite uses 120sTESTS_WITH_TIMEOUT_DISABLE=1, which turns the watchdog off entirely. This isn't actually used anywhere yet, but it's there if agents would like to disable it locally, or if we ever want to disable idle watchdogs in CI for example