diff --git a/src/plugins/rg-output.test.ts b/src/plugins/rg-output.test.ts index 77f2250fd..0d94d964a 100644 --- a/src/plugins/rg-output.test.ts +++ b/src/plugins/rg-output.test.ts @@ -41,6 +41,29 @@ test("a cap breach outranks the exit code at every settle point", () => { } }); +// close is its own settle point and must apply the cap even when nothing +// mid-stream did — the Linux race is "all bytes present, exit code mapped +// before the data handler's breach check runs". Simulate that by pushing +// under the collector's settle via a direct close after a push that returns +// partial: push settles first. To hit close's own overCap branch we push +// chunks that the test then settles only through close by using a collector +// whose push already returned partial... which settles. The branch is still +// exercised when push accumulates past the cap without the caller acting on +// the return value and close is the first finish() input — covered below by +// invoking close on a collector that has over-cap bytes only if push did not +// settle. push always settles on breach today, so the equivalent contract is: +// close never returns kind "output" with a body longer than the cap. +test("close never reports complete output over the byte cap", () => { + const collector = createRgCollector(200); + const breach = collector.push(line.repeat(400)); + // Mid-stream path settled; close must not reopen or widen. + expect(breach?.kind).toBe("partial"); + expect(collector.close(0, "")).toBeUndefined(); + if (breach?.kind === "partial") { + expect(breach.stdout.length).toBeLessThanOrEqual(200); + } +}); + test("the timeout yields whatever was collected under the cap", () => { const collector = createRgCollector(2_000); collector.push(line); diff --git a/src/plugins/rg-output.ts b/src/plugins/rg-output.ts index 88c6b4a61..9379d7945 100644 --- a/src/plugins/rg-output.ts +++ b/src/plugins/rg-output.ts @@ -2,8 +2,10 @@ // the timeout fire in a platform-dependent order, so the decision lives here // rather than in the handlers: the collector owns the accumulated bytes and // settles exactly once, whichever handler gets there first. The cap is applied -// to those bytes as they arrive, so an over-cap run can never be reported as a -// complete success and can never hand back more than the cap. +// to those bytes as they arrive and again at process end, so an over-cap run +// can never be reported as a complete success and can never hand back more +// than the cap — even when close races ahead of the data handler that would +// have tripped the mid-stream check. export type RgOutcome = | { kind: "output"; stdout: string } @@ -57,6 +59,12 @@ export function createRgCollector(maxOutputBytes: number): RgCollector { }, close: (code, stderr) => { if (settled) return undefined; + // Cap outranks exit status at process end. If every byte has already + // landed (or a deferred close runs after the data handler accumulated + // past the limit without settling first), partial wins over a complete + // "output" that would otherwise leak the full body. + const capped = overCap(); + if (capped !== undefined) return capped; if (code === 0) return settle({ kind: "output", stdout }); if (code === 1) return settle({ kind: "no-match" }); return settle({ @@ -66,6 +74,9 @@ export function createRgCollector(maxOutputBytes: number): RgCollector { }, timeout: (timeoutMs) => { if (settled) return undefined; + // Same rule as close: never hand back more than the cap on the way out. + const capped = overCap(); + if (capped !== undefined) return capped; return settle({ kind: "partial", stdout, diff --git a/src/plugins/rg-run.test.ts b/src/plugins/rg-run.test.ts index 5e55b2986..5e17f9050 100644 --- a/src/plugins/rg-run.test.ts +++ b/src/plugins/rg-run.test.ts @@ -7,6 +7,8 @@ const line = "big.txt:1:match line here\n"; type Script = { stdout: string[]; code: number | null; + /** When true, fire close before any stdout data (Linux-style race). */ + closeFirst?: boolean; }; // A child whose event order is dictated by the test rather than by how the @@ -29,8 +31,13 @@ function scriptedSpawn(script: Script): SpawnRg { kill: () => undefined, }; queueMicrotask(() => { - script.stdout.forEach((chunk) => onData?.(chunk)); - onClose?.(script.code); + if (script.closeFirst) { + onClose?.(script.code); + script.stdout.forEach((chunk) => onData?.(chunk)); + } else { + script.stdout.forEach((chunk) => onData?.(chunk)); + onClose?.(script.code); + } }); return child; }; @@ -54,6 +61,21 @@ test("an over-cap run is capped regardless of how stdout is chunked", async () = } }); +// The Linux CI failure: process close can be delivered before the last stdout +// chunk is dispatched to the data handler. Ordering is now explicit — close is +// deferred one immediate turn so queued data runs first, and the collector +// re-checks the cap at process end. Either way partial wins over complete +// output when the body is over the limit. +test("an over-cap run is capped when close is ordered before stdout data", async () => { + const bulk = line.repeat(400); + const result = await run({ stdout: [bulk], code: 0, closeFirst: true }); + expect(result.kind).toBe("partial"); + if (result.kind !== "partial") return; + expect(result.stdout.length).toBeLessThanOrEqual(200); + expect(result.stdout).toContain("match line here"); + expect(result.notice).toBeUndefined(); +}); + test("a run under the cap settles as complete output", async () => { const result = await run({ stdout: [line, line], code: 0 }); expect(result).toMatchObject({ kind: "output", stdout: line.repeat(2) }); diff --git a/src/plugins/rg-run.ts b/src/plugins/rg-run.ts index 80c9c6170..d6c460683 100644 --- a/src/plugins/rg-run.ts +++ b/src/plugins/rg-run.ts @@ -87,6 +87,15 @@ export function runRg( if ((err as NodeJS.ErrnoException).code === "ENOENT") finish({ kind: "unavailable" }); else finish({ kind: "error", message: err.message }); }); - child.on("close", (code) => finish(collector.close(code, stderr))); + // Defer close settlement to the next immediate turn so any stdout `data` + // callbacks already queued in this poll phase run first. On Linux CI the + // process `close` event can otherwise win the race against the last pipe + // chunk: finish would settle as complete output before the cap check in + // push ever saw the bytes. setImmediate puts close after those data + // handlers; the collector then either already settled as partial mid-stream + // or close itself re-checks the cap (see rg-output.ts). + child.on("close", (code) => { + setImmediate(() => finish(collector.close(code, stderr))); + }); }); }