Repository navigation
fix(cli): load telemetry without the core and engine barrels, and stop the history test racing - #5153
Merged
Merged
Conversation
…p the history test racing
Edit accuracy: accurate 2059 (base branch 2059), smooth 1581 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
…st fast on a crashed CLI
miguel-heygen
marked this pull request as ready for review
October 7, 2026 04:59
miguel-heygen
enabled auto-merge
October 7, 2026 05:49
jrusso1020
approved these changes
Oct 7, 2026
jrusso1020
left a comment
Collaborator
There was a problem hiding this comment.
Approve at 51f5c36e.
- The test fix is the right shape. Waiting for the CLI's
owner.pid-*claim draft proves the CLI actually tried the held lock, which the 1.5 s sleep never proved. The race againstexitedturns a crash on start into an immediate failure instead of a timeout. BothonTestFinishedcleanups cover the watcher and the child. - The leaf imports are real leaves. I checked at this head:
core/src/telemetryRedaction.tshas no imports at all, andengine/src/services/systemMemory.tsimports onlyfsandos. So telemetry no longer drags in the core and engine barrels.@hyperframes/studio-server/historyalready existed, so that part is reuse, not a new surface. - The subpath entries match their neighbours.
package-subpaths.jsonis the source and the manifests are regenerated. The engine entry carries nonodecondition, like the other engine subpaths, and the bundle alias is added for it. - CI is green.
Non-blocking: the macOS caveat in the body (the watcher can see the test's own claim) is fine because CI runs this test on Linux and Windows only, and the log assertion still holds there.
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.
What
Fixes the Windows timeout in
history.test.ts, "a CLI run waits while another process holds the history". It hit vitest's 20 s limit once on the engine-cli Windows job. Three changes:owner.pid-<uuid>.tmp), armed before the child starts, then checks nothing was written and releases. If the CLI exits before it tries the lock, the test fails at once with its exit code instead of waiting out the timeout, and the child is killed when the test ends. Under CPU load the child used to reach the lock only after the test had already released it. It then took the lock on its first try, and the test passed without testing any waiting: 30 of 30 runs on a runner with 8 busy loops on 4 cores.@hyperframes/coreand@hyperframes/enginebarrels. Every command loads telemetry.events.tspulled the whole core barrel (parsers, Babel, recast, linkedom, esbuild) for one string redaction function, andsystem.tspulled the engine barrel (puppeteer-core included) for one memory reading. New subpaths, declared in each package'spackage-subpaths.jsonwith the manifests regenerated:@hyperframes/core/telemetry-redactionand@hyperframes/engine/system-memory, plus the CLI bundle alias for the engine one.telemetry/canary.tsalready states this rule for the startup path.@hyperframes/studio-server/historysubpath instead of that package's barrel.Why it timed out
The test's run time was 1.5 s plus a cold
bun run src/cli.ts history. Measured on Windows runners, with a trace inside the child:Changing the wait alone removes the race but not the cost: under starvation the fixed test still timed out, because the child had not yet tried the lock. The leaf imports remove most of that cost.
Proof
history.test.tsand the telemetry tests: 348/348, 3 runs in a row. A CLI that crashes on start now fails the test in about 3 s with "the CLI exited (1) before trying the held lock". Typecheck passes for the CLI and core, the subpath check and its tests pass, the CLI bundle builds and inlines both subpaths, andnode dist/cli.js history --helpruns.Not covered
Size
Under the 100-line floor on purpose: this fixes a flaky CI test on main on its own, so it can land and revert independently of any feature PR.