Repository navigation
test(cli): pay the oclif cold load at module scope, not in a 10s hook nobody chose - #18782
Merged
os-support-ai merged 1 commit intoSep 17, 2026
Merged
Conversation
… nobody chose
`envelope-unwrap.test.ts` loaded oclif's `Config.load({ root: CLI_ROOT })`
inside a `beforeAll`, where vitest's DEFAULT `hookTimeout` of 10000ms judged
it. `packages/cli/vitest.config.ts` sets no timeout key at all — by the
declared design in its own header — so the budget is vitest's default and
nobody in this package chose it.
Measured on a 4-vCPU container, n=5 per row, the call itself:
idle 3934 / 3962 / 4272 / 4451 / 4469 ms
4 spinners on 4 vCPU 7843 / 7981 / 8585 / 8782 / 9011 ms
39-45% of the budget on an IDLE box, and as little as 989 ms of margin on a
box that cannot even reach the load a merge-queue shard applies. A budget a
real cost approaches to within a second is not a budget — it is a load
sensor, and what it senses is reported as "this file failed".
The remedy is not a bigger number: widening the window around the cost
relocates the cliff to the next heavier shard. It is the repo's own stated
convention — "clocked windows measure behaviour, never loading" (AGENTS.md,
Build & Test) — so the load moves OUT of every clocked window and is paid by
a module-scope `await`, during COLLECTION. Verified against the runner this
tree installs, not recalled: in `@vitest/runner@4.1.11` `withTimeout(...)`
wraps exactly the hooks and the test bodies, while `collectTests()` awaits
`runner.importFile(filepath, 'collect')` bare.
Same file, same box, before -> after:
before Duration 4.59s (import 481ms, tests 3.95s) 11 tests
after Duration 4.40s (import 4.20s, tests 36ms) 12 tests
The cost did not shrink and was never meant to; it left the clocked region.
A pin holds the placement, because the identical prose warning one package
over ("do not add one back") is a comment nobody asserts. It is a SOURCE
assertion on purpose: the behavioural instrument the prior art used —
`vitest run --hookTimeout=1`, green iff no hook time is left to clock — is
INERT in this package. Measured: a probe `beforeAll` sleeping 500ms passes
under `--hookTimeout=1` in `packages/cli` with and without `--project`, while
the identical probe under `@objectstack/plugin-dev` (no `test.projects`)
fails with `Hook timed out in 1ms`.
Tier unchanged, measured both ways: `unit` 11 -> 12 entries for this file,
`integration` 0 -> 0, against a lit integration list of 413 entries.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
This was referenced Sep 17, 2026
os-support-ai
marked this pull request as ready for review
September 17, 2026 21:20
os-support-ai
enabled auto-merge
September 17, 2026 21:20
os-support-ai
deleted the
claude/issue-18748-envelope-unwrap-hook-timeout
branch
September 17, 2026 21:48
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
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.
Fixes #18748
packages/cli/src/commands/datasource/envelope-unwrap.test.tspaid oclif'sConfig.load({ root: CLI_ROOT })inside abeforeAll, where vitest's defaulthookTimeoutof 10000ms judged it. This moves that cold load out of every clockedwindow and pins the placement. No budget was invented, raised, skipped or quarantined.
The budget nobody chose, and the cost it was judging
packages/cli/vitest.config.tssets no timeout key at all — deliberately, by thedeclared design in its own header ("the
testblock only carries keys with a recordedwarrant"). So the 10000ms is vitest's own default.
Measured on this 4-vCPU container, n=5 per row — the
Config.loadcall itself, timedinside a vitest worker in the same directory as the subject:
An idle box already spends 39–45% of the budget, and a box that cannot even reach the
load a merge-queue shard applies leaves under one second. A budget a real cost approaches
to within a second is not a budget, it is a load sensor — and what it senses is how
busy the runner is, reported as "this file failed".
Why not a bigger number, and why this number was not invented
Widening the window around the cost relocates the cliff to the next heavier shard. The
merge queue runs the full suite where PR-side CI runs only the affected subset, so
the queue shard is heavier than anything a PR check measures — which is where this class
has already ejected other people's green PRs.
The card asked whether an already-written adjacent bound could be reused. Measured, and
the honest answer is no, and none was needed:
runtime-lazy-deps.test.ts"gating a flow in-process" runs on the default 5s timeout and flakes under load — its cold-load sibling already carriesCOLD_LOAD_TIMEOUT_MS#5421's title points at isCOLD_LOAD_TIMEOUT_MS = 30_000, inpackages/lint/src/{lazy-deps,runtime-lazy-deps}.test.ts. It is atestTimeoutargument on
it(...), not a hook budget, and it is in another package.packages/metadata-fs/test/watch-dot-root.test.tscarriesHOOK_TIMEOUT_MS = 30_000,sized for
repo.close()+fs.rm, not for an oclif load.check-test-source-alias.mjsrecords the same class reaching20.26s on a starved core — "past every clock, including a hypothetical 30s". Reusing
30_000 here would buy a bigger cliff, not no cliff.
So this PR takes the repo's own stated convention instead, which needs no number at all:
The load is now a module-scope
await, paid during collection. Verified against therunner this tree installs rather than recalled —
@vitest/runner@**4.1.11**(the priorart verified 4.1.10):
withTimeout(...)wraps exactly the hooks (beforeAll,afterAll,beforeEach,afterEach,onTestFailed,onTestFinished) and the test bodies, whilecollectTests()awaitsrunner.importFile(filepath, 'collect')bare; andvitest --helpon 4.1.11 offers exactly three timeout knobs (testTimeout,hookTimeout,teardownTimeout), none of which covers module loading.Before → after, same file, same box
The cost did not shrink and was never meant to. It left the clocked region:
tests3.95s → 36ms,
import481ms → 4.20s, wall clock unchanged.The pin — fails before, passes after, both readings shown
Reverse verification, run from the committed fix. The implementation half was mutated back
to the pre-fix shape, proved on disk before the run, and restored from
HEAD:Both halves move together, in both directions.
The prior art one package over (
plugins/plugin-dev/src/dev-plugin-security-enforcement-warning.test.ts) witnesses this property behaviourally: "stays green even under--hookTimeout=1— there is no hook time left to clock." That instrument does not workin
packages/cli, measured rather than assumed:beforeAllsleeping 500ms)@objectstack/plugin-dev(notest.projects)vitest run PROBE_FILE --hookTimeout=1Error: Hook timed out in 1ms.@objectstack/cli(test.projects)vitest run --project unit PROBE_FILE --hookTimeout=1@objectstack/clivitest run PROBE_FILE --hookTimeout=1(no--project)A CLI timeout override does not reach a project-level config on vitest 4.1.11. The lit
control is the plugin-dev row: the flag works, so the two zeros are readings and not a
dead instrument. That leaves the structural fact as the only thing assertable here, and
the pin asserts it — using
maskCommentsAndLiteralsbecause this file's prose and thepin's own regex bodies contain the very spellings being searched for.
Tier, measured in BOTH directions
packages/cli/vitest-tiers.tstreatsnew ObjectQL(and friends as KERNEL signals, so apin can move a whole file across tiers. It did not:
unitentries for this fileintegrationentriesunittotalintegrationtotalRead from
vitest list, the config's own answer, not from the predicate by hand. Controlfor the zero column: the integration list is non-empty at 413 entries (e.g.
test/authoring-rule-command-parity.test.ts), so0is a reading.Changeset:
skip-changeset, measured not assumedClause-②: no
Declared from the measured diff, not inherited: the dispatching seat's claim comment
deliberately carried no value, and a fabricated declaration passes where a missing one
reddens. The change set is exactly one test file, and it moves no authorable surface in
either direction — neither widening nor narrowing anything an author can write.
AGENTS.md(§ Post-Task Checklist 3): "⛔ neverskip-changeset: that label is for adiff that publishes nothing from any released package." This diff is exactly that case,
and it was measured after a full build by grepping the paths
@objectstack/cli'sfiles[]actually ships (dist,README.md,CHANGELOG.md):files[]envelope-unwrapSILENT_PASSDRIFT_RESULTreadEnvelopeFromsrc(control)DatasourceValidatesrc(control)No federated objects to validatesrc(control)tsconfig.build.jsonexcludessrc/**/*.test.tsfromdist, so the file cannot ship;the controls are lit, so the zeros are readings. Nothing published moves.
Verification
pnpm --filter @objectstack/cli exec vitest run --project unit212 files / 3035 tests passed, exit 0pnpm --filter @objectstack/cli typechecktsconfig.test.json)turbo … --filter=@objectstack/cli^...)scripts/pm/dispatch-gates.mjs --raneslint . --no-inline-config(the whole repo-wide population, NOT narrowed)The four gates that first exited 3 (
check:dual-build-cjs-loads,check:i18n,check:i18n-coverage,check:i18n-walk-parity) werePREREQUISITE NOT MET— they readbuilt output, and only the cli closure was built. Re-run after a full
turbo run build:all four exit 0. ⛔ Those threes are not recorded as failures; they were not
measurements.
packages/cli'sintegrationtier is declared to CI: this diff touches nointegration-tier file, no
bin/entry and no spawn helper.Acceptance notes
with readings in the report handed back to the dispatching seat: what a shared cold-load
budget would and would not buy, and why the gate-shaped option (extending
check:test-source-alias's clocked-window rule past dynamicimport()to any coldload in a clocked window) is the one with a measured warrant. That is a decision, not an
execution.
--hookTimeout/--testTimeoutare inert inpackages/cli(table above). This iswider than this card: it means no seat can lower or raise a timeout from the CLI in this
package, and a merge-queue triage that reaches for that flag will read a false green.
Reported for filing with dedupe words, ⛔ not filed from here and ⛔ not fixed here.
plugins/plugin-dev/src/dev-plugin-security-enforcement-warning.test.tsguards the identical invariant with prose only (
⛔ Do not add one back) — no assertion.The pin added here is the shape that would close it. Carrier: whoever next touches that
file; none queued.
🤖 Generated with Claude Code
https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3
Generated by Claude Code
Generated by Claude Code