diff --git a/TESTING.md b/TESTING.md index 18956506a..cacfe08e7 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1,6 +1,6 @@ # Testing data-connectors -Every local test command runs through `scripts/test-sandbox/test-sandbox`, a vendored copy of the host-wide test governor (see `scripts/test-sandbox/README.md`). It gives each run a systemd scope under the shared `agent-tests.slice` with 400% CPU, 8 GB and 4096 tasks, one of the host-wide slots, a loopback-only network unless the lane opts out, and a private `/tmp`. Stopping the scope kills every descendant. Each run prints a JSON receipt (wall, CPU, peak memory) on stderr. Where systemd user sessions, bwrap or flock are missing (macOS, GitHub runners), it runs the command directly and says so in the receipt. +Every local test command runs through `scripts/test-sandbox/test-sandbox`, a vendored copy of the host-wide test governor (see `scripts/test-sandbox/README.md`). It gives each run a systemd scope under the shared `agent-tests.slice` with 400% CPU, 8 GB and 4096 tasks, one of the host-wide slots, a loopback-only network unless the lane opts out, and a private `/tmp`. Stopping the scope kills every descendant. Each run prints a JSON receipt (wall, CPU, peak memory) on stderr. Each layer it cannot use on a host is skipped and named in the receipt; see `scripts/test-sandbox/README.md` for what that means on macOS and on GitHub runners. ## Tiers @@ -9,7 +9,7 @@ Every local test command runs through `scripts/test-sandbox/test-sandbox`, a ven | While editing | `npm run test:fast` (root or packages/polyfill-connectors) | Tests related to files changed since `git merge-base HEAD origin/main`, plus uncommitted and untracked files: the static import graph, plus every test in the same `connectors//` directory. Namespace tests run in their own lane. When the selector cannot trust the graph (for example a change under `scripts/related-tests/` or a deleted file), it runs the whole sandboxed lane. | one-connector edit: 42 tests, 4-10 s | | Same, against any base | `npm run test:related -- ` (in packages/polyfill-connectors) | Same selection; on FULL_SUITE it runs both polyfill lanes. | | | Before a PR | `npm test` (root) | Every lane in turn (below). | about 28 min, 16 of them PageShim | -| PR into main | GitHub Actions | `verify + test`, PageShim shards, connector shards, Contract Guardrails, cross-repo integrity. PRs into other branches run no CI. On GitHub runners the sandbox also enforces (cgroup+netns). | `verify + test` Test step 699 s (was 15-18 min at concurrency 2); PageShim shards 588 s and 835 s | +| PR into main | GitHub Actions | `verify + test`, PageShim shards, connector shards, Contract Guardrails, cross-repo integrity. PRs into other branches run no CI. `verify + test` runs its lanes in the sandbox with cgroup limits and network isolation, and Contract Guardrails with cgroup limits only; the PageShim and connector shards and cross-repo integrity run without it (each CI job is its own VM). | `verify + test` Test step 699 s (was 15-18 min at concurrency 2); PageShim shards 588 s and 835 s | The root `npm test` runs these lanes, each in its own sandbox: @@ -25,10 +25,10 @@ The root `npm test` runs these lanes, each in its own sandbox: ## What is shared, per-worker, per-test, locked -- Shared: the checkout, `node_modules`, the Playwright browser cache, the built PageShim bundles of one run. Tests treat them as read-only, except that the namespace tests plant and remove sockets under the repository root (see below). +- Shared: the checkout, `node_modules`, the Playwright browser cache, the built PageShim bundles of one run. Tests write into the checkout in a few places: the namespace tests plant and remove sockets under the repository root (see below); PageShim tests build bundles under `.tmp/pageshim/` and `.scratch/`; `bin/connector-dev.test.ts` creates and removes `packages/polyfill-connectors/runs/` and `connectors//`; `bin/connector-init.test.ts` scaffolds and removes `connectors/zz_init_smoke_/` and `connectors/zz_init_browser_smoke_/` (a killed run leaves them, and the manifest tests then fail); `bin/scenario-cli.test.ts` writes fixture connectors under `packages/polyfill-connectors/tmp/`; and one consent-time-field test makes a temporary directory inside `node_modules`. Do not run two suites in one checkout at once. - Per test file: one `node --test` child process. Module state (PageShim fixture modules with `setLoggedIn`) is per file; tests in one file run one after another. - Per test: temp directories (`mkdtemp` under the sandbox's private `/tmp`), local HTTP servers on port 0, iMessage `chat.db` files, PageShim browsers (one Chromium per `runHarness`, about 140 ms to start). -- Locked (serial): the namespaces lane. Its files share the repository root as global state (`isolation-mechanism.test.ts` plants sockets under it that `scenario-verify-strict.test.ts`'s isolation scan refuses), so one run executes them one at a time. Nothing locks them across two runs in the same checkout, so do not run two suites in one checkout at once. +- Locked (serial): the namespaces lane. Its files share the repository root as global state (`isolation-mechanism.test.ts` plants sockets under it that `scenario-verify-strict.test.ts`'s isolation scan refuses), so one run executes them one at a time. Nothing locks them across two runs in the same checkout. - Never shared: the network. Unit lanes have loopback only; PageShim routes deny every request that no fixture serves. ## Rules diff --git a/connectors/apple_health/index.test.ts b/connectors/apple_health/index.test.ts index 7c59d8af0..42bdcece7 100644 --- a/connectors/apple_health/index.test.ts +++ b/connectors/apple_health/index.test.ts @@ -109,6 +109,7 @@ interface RunOpts { peakRssPollIntervalMs?: number; /** Restrict every health-area stream to specific record ids, as a host may. */ resources?: string[]; + retainRawStdout?: boolean; state?: Record; /** The streams to request. Defaults to every data stream and the receipts. */ streams?: readonly string[]; @@ -155,6 +156,9 @@ async function run(dir: string, opts: RunOpts = {}) { ? {} : { peakRssPollIntervalMs: opts.peakRssPollIntervalMs }), ...(opts.timeoutMs === undefined ? {} : { timeoutMs: opts.timeoutMs }), + ...(opts.retainRawStdout === undefined + ? {} + : { retainRawStdout: opts.retainRawStdout }), start: { ...(opts.fullRefresh ? { collection_mode: "full_refresh" } : {}), scope: { streams: streams.map((name) => scopeFor(name, opts)) }, @@ -3286,8 +3290,10 @@ test("with both identity budgets spent, memory stays within the streaming bound `\n\n \n` + `${parts.join("\n")}\n\n`; await withImportDir({ "export.xml": xml }, async (dir) => { + // Only parsed messages are read here, and there are ~500k of them. const result = await run(dir, { peakRssPollIntervalMs: 200, + retainRawStdout: false, timeoutMs: 540_000, }); assert.ok(result.peakRssBytes !== null, "expected a sampled peak RSS"); diff --git a/connectors/google_maps/validation-from-file.test.ts b/connectors/google_maps/validation-from-file.test.ts index be6b12f6c..15614ac17 100644 --- a/connectors/google_maps/validation-from-file.test.ts +++ b/connectors/google_maps/validation-from-file.test.ts @@ -561,7 +561,9 @@ test("validateGoogleMapsTimelineArtifactFromFile: a large sparse export never ca } }); -test("outcome proof: a 110 MiB sparse Timeline export separates streaming from whole-buffer parsing at one heap limit", async () => { +test("outcome proof: a 110 MiB sparse Timeline export separates streaming from whole-buffer parsing at one heap limit", { + timeout: 240_000, +}, async () => { // Deterministic worker-thread oracle, not an in-process memory-delta // measurement (the prior in-process external-memory-delta test here was // proven decorative by mutation testing -- it passed even when reverted @@ -620,7 +622,9 @@ test("outcome proof: a 110 MiB sparse Timeline export separates streaming from w // A worker resource limit reports OOM without aborting the process or // generating a core dump for this expected negative-control outcome. const heapLimitMiB = 128; - const timeoutMs = 30_000; + // A hang guard, not a speed bound: on a 4-vCPU CI runner with four test + // files at once, the streaming pass ran past 30 s (the whole test took 38.6 s). + const timeoutMs = 90_000; type OracleOutcome = { error: (Error & { code?: string }) | undefined; exitCode: number; diff --git a/connectors/strava/no-whole-file-read.test.ts b/connectors/strava/no-whole-file-read.test.ts index 1ee740471..7feacc47b 100644 --- a/connectors/strava/no-whole-file-read.test.ts +++ b/connectors/strava/no-whole-file-read.test.ts @@ -96,6 +96,8 @@ test("large synthetic activities.csv emits with bounded RSS", async () => { TZ: "UTC", }, peakRssPollIntervalMs: 25, + // Only parsed messages are asserted on, so skip the raw stdout copy. + retainRawStdout: false, start: { scope: { streams: [{ name: "activities" }, ], diff --git a/connectors/whatsapp/no-whole-file-read.test.ts b/connectors/whatsapp/no-whole-file-read.test.ts index a524fdc86..56ce522d1 100644 --- a/connectors/whatsapp/no-whole-file-read.test.ts +++ b/connectors/whatsapp/no-whole-file-read.test.ts @@ -139,6 +139,8 @@ test("a large synthetic WhatsApp .txt export (well past the old 1 GiB cap) parse WHATSAPP_EXPORT_DIR: importRoot, }, peakRssPollIntervalMs: 25, + // Only parsed messages are asserted on, so skip the raw stdout copy. + retainRawStdout: false, start: { scope: { streams: [{ name: "chats" }, { name: "messages" }] }, type: "START", diff --git a/packages/polyfill-connectors/bin/connector-dev.test.ts b/packages/polyfill-connectors/bin/connector-dev.test.ts index 622674483..d5fb1d5f1 100644 --- a/packages/polyfill-connectors/bin/connector-dev.test.ts +++ b/packages/polyfill-connectors/bin/connector-dev.test.ts @@ -23,6 +23,7 @@ import { readFileSync, rmdirSync, rmSync, + writeFileSync, } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -1068,3 +1069,62 @@ test("connector-dev CLI: a connector cannot launder its auth failure into runtim rmSync(tmpDir, { recursive: true, force: true }); } }); + +test("connector-dev CLI: preserves multi-byte STATE cursors across pipe chunks", () => { + const tmpDir = mkdtempSync(join(tmpdir(), "connector-dev-utf8-")); + const connector = `connector-dev-utf8-${String(process.pid)}`; + const entrypoint = join(tmpDir, "connector.mjs"); + const expected = "€".repeat(100_001); + try { + writeFileSync( + entrypoint, + ` +import { once } from "node:events"; +import { setTimeout as delay } from "node:timers/promises"; +await once(process.stdin, "data"); +const emit = (message) => new Promise((resolve, reject) => { + process.stdout.write(JSON.stringify(message) + "\\n", (error) => error ? reject(error) : resolve()); +}); +await emit({ type: "RECORD", stream: "items", key: "utf8", emitted_at: "2026-08-01T00:00:00.000Z", data: { id: "utf8", value: "fixture" } }); +const bytes = Buffer.from(JSON.stringify({ + type: "STATE", stream: "items", cursor: { value: "€".repeat(100_001) } +}) + "\\n"); +// The 64 KiB split lands inside a three-byte character. Waiting after the +// first write makes the split deterministic even when pipe reads coalesce. +const split = 65_536; +await new Promise((resolve, reject) => { + process.stdout.write(bytes.subarray(0, split), (error) => error ? reject(error) : resolve()); +}); +await delay(40); +await new Promise((resolve, reject) => { + process.stdout.write(bytes.subarray(split), (error) => error ? reject(error) : resolve()); +}); +await emit({ type: "DONE", status: "succeeded", records_emitted: 1 }); +process.stdin.resume(); +`, + ); + const result = runCli([ + connector, + "--entrypoint", + entrypoint, + "--summary-out", + join(tmpDir, "summary.json"), + ]); + assert.equal(result.code, 0, result.stderr); + const saved = JSON.parse( + readFileSync(lastStatePathFor(connector), "utf8"), + ) as LastState; + assert.deepEqual(Object.keys(saved.state), ["items"]); + assert.equal( + (saved.state.items as { value: string }).value === expected, + true, + "saved cursor must retain every multi-byte character exactly", + ); + } finally { + rmSync(tmpDir, { recursive: true, force: true }); + rmSync(join(PACKAGE_ROOT, "runs", connector), { + recursive: true, + force: true, + }); + } +}); diff --git a/packages/polyfill-connectors/bin/connector-dev.ts b/packages/polyfill-connectors/bin/connector-dev.ts index 5ab93ab59..52b137af3 100644 --- a/packages/polyfill-connectors/bin/connector-dev.ts +++ b/packages/polyfill-connectors/bin/connector-dev.ts @@ -987,8 +987,9 @@ export function runAndStream( } }; - child.stdout?.on("data", (chunk: Buffer) => { - stdoutBuffer += chunk.toString(); + child.stdout?.setEncoding("utf8"); + child.stdout?.on("data", (chunk: string) => { + stdoutBuffer += chunk; let newlineIndex = stdoutBuffer.indexOf("\n"); while (newlineIndex !== -1) { const line = stdoutBuffer.slice(0, newlineIndex); diff --git a/packages/polyfill-connectors/bin/scenario-cli.test.ts b/packages/polyfill-connectors/bin/scenario-cli.test.ts index a50bbf8f2..190ed5ce5 100644 --- a/packages/polyfill-connectors/bin/scenario-cli.test.ts +++ b/packages/polyfill-connectors/bin/scenario-cli.test.ts @@ -4059,3 +4059,129 @@ test("scenario-verify: replay subprocess forces PLAYWRIGHT_BROWSERS_PATH and PDP rmSync(connectorPath, { force: true }); } }); + +function writeMultiByteConnector(directory: string): string { + const entrypoint = join(directory, "connector.mjs"); + writeFileSync( + entrypoint, + ` +import { once } from "node:events"; +import { setTimeout as delay } from "node:timers/promises"; +await once(process.stdin, "data"); +const emit = (message) => new Promise((resolve, reject) => { + process.stdout.write(JSON.stringify(message) + "\\n", (error) => error ? reject(error) : resolve()); +}); +await emit({ type: "RECORD", stream: "items", key: "utf8", emitted_at: "2026-08-01T00:00:00.000Z", data: { id: "utf8", value: "fixture" } }); +const bytes = Buffer.from(JSON.stringify({ + type: "STATE", stream: "items", cursor: { value: "€".repeat(100_001) } +}) + "\\n"); +// The 64 KiB split lands inside a three-byte character. Waiting after the +// first write makes the split deterministic even when pipe reads coalesce. +const split = 65_536; +await new Promise((resolve, reject) => { + process.stdout.write(bytes.subarray(0, split), (error) => error ? reject(error) : resolve()); +}); +await delay(40); +await new Promise((resolve, reject) => { + process.stdout.write(bytes.subarray(split), (error) => error ? reject(error) : resolve()); +}); +await emit({ type: "DONE", status: "succeeded", records_emitted: 1 }); +process.stdin.resume(); +`, + ); + return entrypoint; +} + +test("scenario-record: preserves multi-byte STATE cursors across pipe chunks", () => { + const tmpDir = mkdtempSync( + join(packageScratchDir(), "scenario-record-utf8-"), + ); + const scenarioPath = join(tmpDir, "utf8.scenario.json"); + try { + const entrypoint = writeMultiByteConnector(tmpDir); + const result = runRecordCli( + [ + "scenario-utf8-fixture", + "--entrypoint", + entrypoint, + "--runs", + "1", + "--out", + scenarioPath, + ], + {}, + ); + assert.equal(result.code, 0, result.stderr); + const scenario = JSON.parse( + readFileSync(scenarioPath, "utf8"), + ) as ConnectorScenario; + assert.equal(scenario.capture.complete, true); + const finalState = scenario.runs[0]?.expected.final_state as + | { items?: { value?: string } } + | undefined; + assert.equal( + finalState?.items?.value === "€".repeat(100_001), + true, + "recorded cursor must retain every multi-byte character exactly", + ); + } finally { + rmSync(tmpDir, { recursive: true, force: true }); + } +}); + +test("scenario-verify: preserves multi-byte STATE cursors across pipe chunks", () => { + const tmpDir = mkdtempSync( + join(packageScratchDir(), "scenario-verify-utf8-"), + ); + const scenarioPath = join(tmpDir, "utf8.scenario.json"); + try { + const entrypoint = writeMultiByteConnector(tmpDir); + // Construct the oracle independently: a broken recorder must not hide + // identical corruption in the verifier by supplying a corrupted cursor. + const scenario: ConnectorScenario = { + format: "pdpp.connector-scenario/1", + connector: { id: "scenario-utf8-fixture" }, + capture: { + captured_at: "2026-08-01T00:00:00.000Z", + evidence_class: "synthetic-spike", + privacy_class: "local-only", + recorder_version: "test", + complete: true, + }, + runs: [ + { + start: { scope: { streams: [{ name: "items" }] }, state: null }, + interactions: [], + expected: { + records: { + items: { + count: 1, + ids: ["utf8"], + ops: ["upsert"], + record_sha256s: [ + hashCanonicalJson({ id: "utf8", value: "fixture" }), + ], + }, + }, + final_state: { items: { value: "€".repeat(100_001) } }, + }, + }, + ], + }; + writeFileSync(scenarioPath, JSON.stringify(scenario)); + const result = runVerifyCli([ + "scenario-utf8-fixture", + "--entrypoint", + entrypoint, + scenarioPath, + ]); + assert.equal( + result.code, + 0, + `stdout=${result.stdout.slice(0, 2000)} stderr=${result.stderr}`, + ); + assert.match(result.stdout, /run 0: PASS/); + } finally { + rmSync(tmpDir, { recursive: true, force: true }); + } +}); diff --git a/packages/polyfill-connectors/bin/scenario-record.ts b/packages/polyfill-connectors/bin/scenario-record.ts index 770c33e78..8b3f10217 100644 --- a/packages/polyfill-connectors/bin/scenario-record.ts +++ b/packages/polyfill-connectors/bin/scenario-record.ts @@ -1684,12 +1684,13 @@ function runRecordSubprocess(args: { } }; - child.stdout.on("data", (chunk: Buffer) => { + child.stdout.setEncoding("utf8"); + child.stdout.on("data", (chunk: string) => { // Activity — resets the inactivity window (a no-op while suspended // for a pending INTERACTION; see `answerInteraction`/`handleParsedLine` // above). watchdog.touch(); - stdoutBuffer += chunk.toString(); + stdoutBuffer += chunk; let newlineIndex = stdoutBuffer.indexOf("\n"); while (newlineIndex !== -1) { const line = stdoutBuffer.slice(0, newlineIndex); @@ -1700,9 +1701,10 @@ function runRecordSubprocess(args: { newlineIndex = stdoutBuffer.indexOf("\n"); } }); - child.stderr.on("data", (chunk: Buffer) => { + child.stderr.setEncoding("utf8"); + child.stderr.on("data", (chunk: string) => { watchdog.touch(); - stderr += chunk.toString(); + stderr += chunk; }); child.on("error", (err) => { watchdog.dispose(); diff --git a/packages/polyfill-connectors/bin/scenario-verify.ts b/packages/polyfill-connectors/bin/scenario-verify.ts index fb51591d0..c827c1808 100644 --- a/packages/polyfill-connectors/bin/scenario-verify.ts +++ b/packages/polyfill-connectors/bin/scenario-verify.ts @@ -1504,7 +1504,8 @@ function runReplaySubprocess(args: { } }; - childStdout.on("data", (chunk: Buffer) => { + childStdout.setEncoding("utf8"); + childStdout.on("data", (chunk: string) => { // Activity — resets the inactivity window (replay's scripted // interaction answering never waits on a human, so this watchdog is // never suspended — see this file's "Inactivity watchdog" section). @@ -1512,7 +1513,7 @@ function runReplaySubprocess(args: { if (hardFailure) { return; } - stdoutBuffer += chunk.toString(); + stdoutBuffer += chunk; let newlineIndex = stdoutBuffer.indexOf("\n"); while (newlineIndex !== -1) { const line = stdoutBuffer.slice(0, newlineIndex); @@ -1529,9 +1530,10 @@ function runReplaySubprocess(args: { newlineIndex = stdoutBuffer.indexOf("\n"); } }); - childStderr.on("data", (chunk: Buffer) => { + childStderr.setEncoding("utf8"); + childStderr.on("data", (chunk: string) => { watchdog.touch(); - stderr += chunk.toString(); + stderr += chunk; }); child.on("error", (err) => { watchdog.dispose(); diff --git a/packages/polyfill-connectors/scripts/no-await-in-loops-allowlist.ts b/packages/polyfill-connectors/scripts/no-await-in-loops-allowlist.ts index a9d171ff5..415738f28 100644 --- a/packages/polyfill-connectors/scripts/no-await-in-loops-allowlist.ts +++ b/packages/polyfill-connectors/scripts/no-await-in-loops-allowlist.ts @@ -686,7 +686,7 @@ export const NO_AWAIT_IN_LOOPS_ALLOWLIST: readonly NoAwaitInLoopsAllowlistEntry[ }, { path: "connectors/apple_health/index.test.ts", - line: 3498, + line: 3504, column: 3, category: "test_assertion_sequencing", note: "small-XML-entry archive cases: each imported in turn by its own subprocess, whose peak RSS is measured alone", diff --git a/packages/polyfill-connectors/src/connector-runtime-consent-time-field.test.ts b/packages/polyfill-connectors/src/connector-runtime-consent-time-field.test.ts index dffb342b2..d8080939a 100644 --- a/packages/polyfill-connectors/src/connector-runtime-consent-time-field.test.ts +++ b/packages/polyfill-connectors/src/connector-runtime-consent-time-field.test.ts @@ -121,8 +121,12 @@ test("the generated consent-time-field modules match the manifests on disk", () ); // One slice per manifest, no extra, each current. const expected = readdirSync(join(scratch, "consent-time-fields")).sort(); + // `npm run build` (and so `npm pack`) emits .js and .d.ts next to each + // slice; only the generated sources are the generator's output. assert.deepEqual( - readdirSync(join(tracked, "consent-time-fields")).sort(), + readdirSync(join(tracked, "consent-time-fields")) + .filter((name) => name.endsWith(".ts") && !name.endsWith(".d.ts")) + .sort(), expected, regenerate, ); diff --git a/packages/polyfill-connectors/src/test-harness.test.ts b/packages/polyfill-connectors/src/test-harness.test.ts index 92bc7de0c..f8dd13832 100644 --- a/packages/polyfill-connectors/src/test-harness.test.ts +++ b/packages/polyfill-connectors/src/test-harness.test.ts @@ -482,3 +482,100 @@ test("runConnectorProtocolSubprocess: stream failure evidence forces failed DONE assert.equal(done.error?.code, "stream_collection_failed"); assert.equal(done.error?.retryable, true); }); + +/** `emitted_at` is each run's own clock; blank it so two runs of one fixture compare. */ +function withoutEmittedAt( + messages: readonly EmittedMessage[], +): EmittedMessage[] { + return messages.map((message) => { + if (message.type !== "RECORD") { + return message; + } + return { ...message, emitted_at: "" }; + }); +} + +test("runConnectorProtocolSubprocess: retainRawStdout false keeps the parsed messages and returns no raw text", async () => { + const retained = await runConnectorProtocolSubprocess({ + cwd: PACKAGE_ROOT, + entrypoint: fixturePath("protocol-subprocess-non-browser.ts"), + start: { type: "START", scope: { streams: [{ name: "items" }] } }, + }); + const dropped = await runConnectorProtocolSubprocess({ + cwd: PACKAGE_ROOT, + entrypoint: fixturePath("protocol-subprocess-non-browser.ts"), + retainRawStdout: false, + start: { type: "START", scope: { streams: [{ name: "items" }] } }, + }); + + assert.deepEqual( + withoutEmittedAt(dropped.messages), + withoutEmittedAt(retained.messages), + ); + assert.equal(dropped.rawStdout, ""); + + const rawLines = retained.rawStdout.split("\n").filter((line) => line !== ""); + assert.deepEqual( + rawLines.map((line) => JSON.parse(line)), + retained.messages, + ); +}); + +test("runConnectorProtocolSubprocess: a multi-byte character split across stdout chunks survives", async () => { + const tempDir = await mkdtemp(join(tmpdir(), "connector-subprocess-utf8-")); + const entrypoint = join(tempDir, "utf8-connector.mjs"); + // 3-byte characters, 300 KB in one line: pipe chunks (64 KiB) cannot all + // end on a character boundary, so per-chunk decoding would corrupt some. + const text = "\u20ac".repeat(100_000); + await writeFile( + entrypoint, + [ + `const text = ${JSON.stringify(text)};`, + 'process.stdout.write(JSON.stringify({ type: "RECORD", stream: "items", data: { id: "1", text } }) + "\\n");', + 'process.stdout.write(JSON.stringify({ type: "DONE", status: "succeeded", records_emitted: 1 }) + "\\n");', + ].join("\n"), + ); + try { + const result = await runConnectorProtocolSubprocess({ + cwd: PACKAGE_ROOT, + entrypoint, + start: { type: "START", scope: { streams: [{ name: "items" }] } }, + }); + const record = result.messages.find((m) => m.type === "RECORD") as + | { data: { text: string } } + | undefined; + assert.ok(record, "the RECORD reached messages"); + assert.equal( + record.data.text.includes("\ufffd"), + false, + "no replacement characters", + ); + assert.equal(record.data.text, text); + } finally { + await rm(tempDir, { recursive: true, force: true }); + } +}); + +test("runConnectorProtocolSubprocess: a child that exits before the first RSS tick still has a sampled peak", { + skip: process.platform !== "linux", +}, async () => { + const tempDir = await mkdtemp(join(tmpdir(), "connector-subprocess-rss-")); + const entrypoint = join(tempDir, "quick-connector.mjs"); + await writeFile( + entrypoint, + 'process.stdout.write(JSON.stringify({ type: "DONE", status: "succeeded", records_emitted: 0 }) + "\\n");\n', + ); + try { + const result = await runConnectorProtocolSubprocess({ + cwd: PACKAGE_ROOT, + entrypoint, + // Far longer than the child lives, so only the start and output + // samples can produce a value. + peakRssPollIntervalMs: 60_000, + start: { type: "START", scope: { streams: [{ name: "items" }] } }, + }); + assert.notEqual(result.peakRssBytes, null, "expected a sampled peak RSS"); + } finally { + await rm(tempDir, { recursive: true, force: true }); + } +}); diff --git a/packages/polyfill-connectors/src/test-harness.ts b/packages/polyfill-connectors/src/test-harness.ts index 0f1a8f8ea..0baebe8f8 100644 --- a/packages/polyfill-connectors/src/test-harness.ts +++ b/packages/polyfill-connectors/src/test-harness.ts @@ -190,6 +190,10 @@ export interface ConnectorSubprocessOptions { * poll timer is pure overhead for the many tests that don't need a * memory proof. */ peakRssPollIntervalMs?: number; + /** Set false for a run whose stdout is too large to hold twice: `rawStdout` + * is then "" and `messages` is unchanged. Callers that assert on + * `rawStdout` must keep the default (true). */ + retainRawStdout?: boolean; start: { /** Mirrors the real runtime's START `collection_mode` field, so a test can * exercise the owner-triggered full-refresh bypass end to end rather than @@ -273,6 +277,7 @@ export function runConnectorProtocolSubprocess( options: ConnectorSubprocessOptions, ): Promise { const timeoutMs = options.timeoutMs ?? 15_000; + const retainRawStdout = options.retainRawStdout ?? true; return new Promise((resolve, reject) => { const child = spawn( @@ -293,22 +298,28 @@ export function runConnectorProtocolSubprocess( let peakRssBytes: number | null = null; let rssPoller: NodeJS.Timeout | undefined; const { pid } = child; - if (options.peakRssPollIntervalMs && pid) { - rssPoller = setInterval(() => { - try { - const status = readFileSync(`/proc/${pid}/status`, "utf8"); - const match = VM_RSS_STATUS_RE.exec(status); - if (match?.[1]) { - const bytes = Number.parseInt(match[1], 10) * 1024; - peakRssBytes = - peakRssBytes === null ? bytes : Math.max(peakRssBytes, bytes); - } - } catch { - // /proc//status is unavailable (non-Linux, or the process - // already exited between the tick firing and the read) -- leave - // peakRssBytes at its last known value. + const sampleRss = (): void => { + try { + const status = readFileSync(`/proc/${pid}/status`, "utf8"); + const match = VM_RSS_STATUS_RE.exec(status); + if (match?.[1]) { + const bytes = Number.parseInt(match[1], 10) * 1024; + peakRssBytes = + peakRssBytes === null ? bytes : Math.max(peakRssBytes, bytes); } - }, options.peakRssPollIntervalMs); + } catch { + // /proc//status is unavailable (non-Linux, or the process + // already exited between the tick firing and the read) -- leave + // peakRssBytes at its last known value. + } + }; + const sampleRssOnOutput = Boolean(options.peakRssPollIntervalMs && pid); + if (sampleRssOnOutput) { + // The interval alone can miss a child that finishes before its first + // tick; sampling at start and on each stdout chunk (below) always + // yields a value while the child is producing records. + sampleRss(); + rssPoller = setInterval(sampleRss, options.peakRssPollIntervalMs); } const messages: EmittedMessage[] = []; @@ -369,9 +380,15 @@ export function runConnectorProtocolSubprocess( ); }, timeoutMs); - child.stdout?.on("data", (chunk: Buffer) => { - const text = chunk.toString(); - rawStdout += text; + // Decode as streams, not per chunk: a multi-byte character split across + // two pipe chunks would otherwise become U+FFFD in both halves. + child.stdout?.setEncoding("utf8"); + child.stderr?.setEncoding("utf8"); + child.stdout?.on("data", (text: string) => { + if (sampleRssOnOutput) sampleRss(); + if (retainRawStdout) { + rawStdout += text; + } stdoutBuffer += text; let newlineIndex = stdoutBuffer.indexOf("\n"); while (newlineIndex !== -1) { @@ -382,8 +399,8 @@ export function runConnectorProtocolSubprocess( } }); - child.stderr?.on("data", (chunk: Buffer) => { - stderr += chunk.toString(); + child.stderr?.on("data", (text: string) => { + stderr += text; }); child.on("error", (err) => { diff --git a/scripts/pageshim/chatgpt-replay.test.mjs b/scripts/pageshim/chatgpt-replay.test.mjs index ce7c97de7..49f08d5d6 100644 --- a/scripts/pageshim/chatgpt-replay.test.mjs +++ b/scripts/pageshim/chatgpt-replay.test.mjs @@ -3,7 +3,7 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { mkdirSync } from "node:fs"; +import { mkdirSync, rmSync } from "node:fs"; import { join } from "node:path"; import { buildPageshim } from "./build.mjs"; import { @@ -15,6 +15,10 @@ import { runHarness } from "./harness.mjs"; const out = join(process.cwd(), ".scratch", "pageshim-chatgpt-replay-test"); mkdirSync(out, { recursive: true }); +// Keep a failed run's bundle for debugging; remove it after a passing run. +process.on("exit", (code) => { + if (code === 0) rmSync(out, { recursive: true, force: true }); +}); test("reverse-projects retained ChatGPT records into list pages and details", () => { const projected = projectChatGptExport({ diff --git a/scripts/test-sandbox/README.md b/scripts/test-sandbox/README.md index 061e295a5..de9f83d5d 100644 --- a/scripts/test-sandbox/README.md +++ b/scripts/test-sandbox/README.md @@ -3,11 +3,15 @@ `test-sandbox` runs one test command in a transient systemd scope under the shared `agent-tests.slice`, with CPU, memory and task caps, a host-wide slot limit, loopback-only network by default (`--net host` opts out) and a private `/tmp`. -It prints a JSON receipt on stderr. Where systemd user sessions, bwrap or flock are -missing or unusable (macOS, GitHub runners), it runs the command directly and says so -in the receipt. +It prints a JSON receipt on stderr. Each layer it cannot use is skipped, and the receipt's `enforcement` field +shows what was applied (notes name a missing bwrap only for `--net none` runs): no systemd user session means no cgroup limits, no working bwrap +means no network or /tmp isolation, no flock means no host-wide slots. GitHub's Ubuntu +runners give cgroup limits; they give network isolation only in a job that installs +bubblewrap and lifts the AppArmor user-namespace restriction, which `verify + test` in +polyfill-connectors.yml does. With bash older than 5 (macOS) the command runs directly +and prints one warning line instead of a receipt. Source: `dotfiles/ai/skills/local/test-architecture/scripts/test-sandbox` -Commit: `7ed10f537e7fe5dc370729264ff7bad23a8ae62e` +Commit: `ce0d9665ab830e2c7a4872325f98974484d45fa1` Do not edit here. Update the canonical copy, re-vendor, then update the `.sha256`. diff --git a/scripts/test-sandbox/test-sandbox b/scripts/test-sandbox/test-sandbox index 120c43b04..b2403cfce 100755 --- a/scripts/test-sandbox/test-sandbox +++ b/scripts/test-sandbox/test-sandbox @@ -22,9 +22,27 @@ if [ -z "${BASH_VERSINFO:-}" ] || [ "${BASH_VERSINFO[0]}" -lt 5 ]; then echo "test-sandbox: bash 5 not found; running without enforcement" >&2 # Drop our options (bash 3.2 syntax only): everything after "--" is the command. - for a in "$@"; do shift; [ "$a" = "--" ] && exec "$@"; done + # The scratch contract still holds: TMPDIR is --scratch (created 0700 if + # absent) or a fresh directory removed afterwards. + _prev="" _scr="" _own="" + for a in "$@"; do + shift + if [ "$a" = "--" ]; then + if [ -n "$_scr" ]; then [ -e "$_scr" ] || (umask 077; mkdir -p "$_scr") || exit 2 + else _scr=$(mktemp -d "${TMPDIR:-/tmp}/test-sandbox.XXXXXX") || exit 2; _own=1; fi + TMPDIR=$_scr TMP=$_scr TEMP=$_scr TEST_TMPDIR=$_scr; export TMPDIR TMP TEMP TEST_TMPDIR + "$@"; _rc=$? + [ -n "$_own" ] && rm -rf "$_scr" + exit $_rc + fi + [ "$_prev" = "--scratch" ] && _scr=$a + _prev=$a + done echo "test-sandbox: usage: test-sandbox [options] -- cmd..." >&2; exit 2 fi +# The body is one { ... } block so bash parses all of it before running: +# editing this file while a run is in flight cannot break that run. +{ set -uo pipefail # systemd-run (>= 254) expands ${VAR} in the command line by default, which # mangles shell code and test arguments; turn that off where supported. @@ -78,7 +96,6 @@ have_flock=1; command -v flock >/dev/null || have_flock=0 # A command already inside a sandbox runs directly: a nested call would take a # second slot (and could deadlock with --slots 1) and gain nothing. if [[ -n "${TEST_SANDBOX_ACTIVE:-}" ]]; then exec "$@"; fi -export TEST_SANDBOX_ACTIVE=1 slice=${TEST_SANDBOX_SLICE:-agent-tests} slice=${slice%.slice} slice_unit="$slice.slice" @@ -144,6 +161,12 @@ PY exit 3 fi +# Tell the command (and any nested harness) what it runs under (TEST_SANDBOX_RUN_*), so it can +# verify instead of trust: a nested test-sandbox call runs directly. +# Output names differ from the input knobs (TEST_SANDBOX_NET, _ISOLATE, ...), +# so a sandbox a test starts with TEST_SANDBOX_ACTIVE cleared keeps its defaults. +export TEST_SANDBOX_ACTIVE=1 TEST_SANDBOX_RUN_ENFORCEMENT="$enforcement" TEST_SANDBOX_RUN_ISOLATE="$isolate" TEST_SANDBOX_RUN_PIDNS="$pidns" +if [[ "$enforcement" == *netns* ]]; then export TEST_SANDBOX_RUN_NET=none; else export TEST_SANDBOX_RUN_NET=host; fi runtime=${XDG_RUNTIME_DIR:-$HOME/.cache} state_dir=${TEST_SANDBOX_STATE_DIR:-$runtime/agent-tests} mkdir -p "$state_dir/slots" || die "cannot create slot directory $state_dir/slots" @@ -283,7 +306,11 @@ if [[ $created_scratch -eq 1 ]]; then rm -rf "$scratch"; fi exit "$rc"' if [[ "$disabled" == 1 ]]; then - "$@"; rc=$? + # Same scratch contract as an enforced run. + own_scratch=0 + if [[ -z "$scratch" ]]; then scratch=$(mktemp -d "${TMPDIR:-/tmp}/test-sandbox.XXXXXX") || die "mktemp failed"; own_scratch=1; fi + ( export TMPDIR="$scratch" TMP="$scratch" TEMP="$scratch" TEST_TMPDIR="$scratch"; "$@" ); rc=$? + (( own_scratch )) && rm -rf "$scratch" wall=$(python3 - "$started" <<'PY' import sys,time;print(max(0,time.monotonic()-float(sys.argv[1]))) PY @@ -398,3 +425,5 @@ PY if [[ -n "$receipt" ]]; then printf '%s\n' "$line" >"$receipt" || { echo "test-sandbox: cannot write receipt $receipt" >&2; exit 125; } else printf '%s\n' "$line" >&2; fi exit "$rc" +exit +} diff --git a/scripts/test-sandbox/test-sandbox.sha256 b/scripts/test-sandbox/test-sandbox.sha256 index 75eaf6fc7..55259c461 100644 --- a/scripts/test-sandbox/test-sandbox.sha256 +++ b/scripts/test-sandbox/test-sandbox.sha256 @@ -1 +1 @@ -9f9569a98a0218a38e6ee005c787aa81adec31f23a794a9c3eb4cef8a2090b1d test-sandbox +b51043419454d9836b9a90014fe7a077129711913751bf9c59cba8c747203941 test-sandbox