test(no-cloud-guard): budget the whole-tree sweeps instead of timing out - #336
Merged
Conversation
The `no shipped source file imports the retired package` guard failed CI intermittently with "timed out after 5000ms". It is not a flaky assertion; the test is simply larger than the default budget allows for. The scan is driven off `package.json` `files`, which includes the bundled output in `bin/`, `dist/` and `dashboard/dist/` as well as the vendored `connectors/` tree, so one run covers 14,519 files and ~71MB. How long that takes is governed by how much of those 71MB is still in the page cache, which is exactly the thing a test cannot control. Measured warm and idle it is ~250ms, but inside a full-suite run on a two-core runner it was observed at 7169ms. Reproduced with `taskset -c 0,1 bun test`: 3977 pass, 1 fail, the guard timing out. A cold cache on CI is the normal case, not the exception. Two changes, in order of importance: Both whole-tree sweeps now carry an explicit 60s budget rather than inheriting the 5s default. This is the fix. A guard over everything the package ships is not a unit test, and when it exceeds its budget it reports a timeout instead of the breach it exists to detect — which is the worst possible failure mode for a security guard, because it is indistinguishable from a pass to anyone skimming. The import scan also prefilters on the raw bytes before decoding. Every specifier `FORBIDDEN_IMPORT` can match contains the package name verbatim, so a `Buffer.includes` test is an exact prefilter and not an approximation: any file the regex would match must contain those bytes. That halves the sweep warm (~250ms against ~300-380ms) by not decoding 71MB of mostly generated bundles to answer a question whose answer is almost always "no match anywhere". Verified the guard still detects a real breach rather than merely running faster: planting a file that imports the retired package fails the suite and names the planted path (exit 1), and removing it returns to 4 pass / 0 fail (exit 0). An A/B over the same 14,519 files agreed on the match set across three rounds. Full suite on two cores: 3969 pass, 0 fail. Typecheck clean.
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
1 participant
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.
The
no shipped source file imports the retired packageguard has been failing CI intermittently withtimed out after 5000ms. The assertion is not flaky — the test is larger than the default budget allows.The scan is driven off
package.jsonfiles, so it covers the bundled output inbin/,dist/anddashboard/dist/as well as the vendoredconnectors/tree: 14,519 files, ~71MB per run. Its duration is governed by how much of that is still in the page cache, which a test cannot control. Warm and idle it is ~250ms; inside a full-suite run on a two-core runner it was observed at 7169ms.Reproduced with
taskset -c 0,1 bun teston the affected branch:3977 pass, 1 fail, the guard timing out.Changes
1. An explicit 60s budget on both whole-tree sweeps. This is the fix. A guard over everything the package ships is not a unit test, and on exceeding its budget it reports a timeout rather than the breach it exists to detect — the worst failure mode for a security guard, because to anyone skimming CI it is indistinguishable from a pass.
2. The import scan prefilters on raw bytes before decoding. Every specifier
FORBIDDEN_IMPORTcan match contains the package name verbatim, soBuffer.includesis an exact prefilter, not an approximation: any file the regex would match must contain those bytes. Halves the sweep warm by not decoding 71MB of mostly generated bundles.Verification
Measured unpiped (
cmd >file 2>&1; echo $?):bun run typechecktaskset -c 0,1 bun testbun run secrets:scan:npmrcbun run check:package-secretsThe negative control is the important one: it proves the guard still detects a breach rather than merely running faster. An A/B over the same 14,519 files agreed on the match set across three rounds.
Scope
Test-only; no product code changes. This flake is pre-existing on
mainand independent of #335 — neither this file norsrc/cli/cli.test.tsis touched by that PR.Known remaining flake, not addressed here
src/cli/cli.test.ts:453list --json supports --limit and --offset paginationfails rarely withSyntaxError: JSON Parse error: Unterminated string. The payload is 237,506 bytes against a 64KB pipe buffer. I could not reproduce it in ~20 targeted attempts and a rerun of the same commit passed, so I have deliberately not shipped a speculative fix. Recommended follow-up: capture subprocess output via a file descriptor rather than a pipe in therun()helper. Three other test files share that sequential pipe-drain pattern (src/server/server-entry.test.ts,src/mcp/mcp.test.ts,connectors/tiktok-events-api/src/api/events.test.ts).Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.