diff --git a/packages/paperclip-runner/.gitignore b/packages/paperclip-runner/.gitignore index 6bca57979f7..7b56764a2a8 100644 --- a/packages/paperclip-runner/.gitignore +++ b/packages/paperclip-runner/.gitignore @@ -4,3 +4,8 @@ /test-results/ /dist-phase7/ /dist-issue-thread/ + +# The repository root ignores scratch `check-*.mjs` files and only re-includes +# them under the root `scripts/` directory. This package's checks are real +# source, so re-include them here instead of relying on `git add -f`. +!scripts/check-*.mjs diff --git a/packages/paperclip-runner/README.md b/packages/paperclip-runner/README.md index 825aec2eb7d..e72c28af759 100644 --- a/packages/paperclip-runner/README.md +++ b/packages/paperclip-runner/README.md @@ -97,6 +97,7 @@ and JSON content; see the protocol-server tutorial for direct `curl` examples. | `typecheck` | Check TypeScript, Rust, generated schema sources, and browser types. | | `test` | Run Rust/TypeScript fixture, supervisor, fake-driver, live/replay, and boundary tests. | | `check:forbidden-imports` | Reject TypeScript imports and Cargo path dependencies that cross into Paperclip core. | +| `check:tracked-imports` | Reject tracked imports and `package.json` entry points that only resolve against untracked files, so a clean checkout of any commit builds. | | `check:phase0-parity` | Require byte-for-byte equivalent Rust and TypeScript tracer output. | | `check:phase1-goldens` | Require all reducer snapshots and cross-language summaries to match checked goldens. | | `check:phase1-parity` | Run TypeScript and Rust against the same Phase 1 fixture summaries. | diff --git a/packages/paperclip-runner/package.json b/packages/paperclip-runner/package.json index eb7ec3418b3..45b31fb5700 100644 --- a/packages/paperclip-runner/package.json +++ b/packages/paperclip-runner/package.json @@ -41,7 +41,7 @@ "typecheck:typescript": "pnpm run check:protocol-types && tsc -p tsconfig.json --noEmit", "typecheck:rust": "cargo check --manifest-path runner/Cargo.toml --locked --workspace", "typecheck:browser": "tsc -p tsconfig.browser.json --noEmit", - "test": "pnpm run build:phase2-binaries && pnpm run test:typescript && pnpm run test:rust && node --test scripts/check-forbidden-imports.test.mjs scripts/record-phase4-evidence.test.mjs scripts/validate-doc-links.test.mjs test/capability-contract.test.mjs", + "test": "pnpm run build:phase2-binaries && pnpm run test:typescript && pnpm run test:rust && node --test scripts/check-forbidden-imports.test.mjs scripts/check-tracked-imports.test.mjs scripts/record-phase4-evidence.test.mjs scripts/validate-doc-links.test.mjs test/capability-contract.test.mjs", "test:typescript": "vitest run", "test:rust": "cargo test --manifest-path runner/Cargo.toml --locked --workspace", "test:browser": "pnpm run build:typescript && pnpm run build:phase2-binaries && pnpm run build:browser && playwright test --config devtools/browser/playwright.config.ts", @@ -58,6 +58,7 @@ "check:phase1-goldens": "node scripts/generate-phase1-goldens.mjs --check", "check:browser-tokens": "node scripts/check-browser-tokens.mjs", "check:forbidden-imports": "node scripts/check-forbidden-imports.mjs", + "check:tracked-imports": "node scripts/check-tracked-imports.mjs", "generate:phase7-inventory": "node scripts/generate-phase7-inventory.mjs", "check:phase7-inventory": "node scripts/check-phase7-inventory.mjs", "test:phase7-inventory": "node scripts/check-phase7-inventory.test.mjs", @@ -97,7 +98,7 @@ "demo:phase7": "pnpm run build:typescript && vite --config vite.phase7.config.ts --host 127.0.0.1 --port 4183", "browser:dev": "pnpm run build:typescript && pnpm run build:phase2-binaries && vite --config vite.config.ts", "browser:preview": "vite preview --config vite.config.ts", - "verify": "pnpm run build && pnpm run typecheck && pnpm run check:phase1-goldens && pnpm run test && pnpm run test:phase5 && pnpm run test:phase7 && pnpm run check:browser-tokens && pnpm run test:browser && pnpm run test:browser:phase5 && pnpm run test:browser:phase7 && pnpm run check:forbidden-imports && pnpm run docs:validate && pnpm run check:phase0-parity && pnpm run check:phase1-parity && pnpm run trace:phase0 && pnpm run replay:phase1 && pnpm run trace:phase2 -- --quiet && pnpm run trace:phase3", + "verify": "pnpm run build && pnpm run typecheck && pnpm run check:phase1-goldens && pnpm run test && pnpm run test:phase5 && pnpm run test:phase7 && pnpm run check:browser-tokens && pnpm run test:browser && pnpm run test:browser:phase5 && pnpm run test:browser:phase7 && pnpm run check:forbidden-imports && pnpm run check:tracked-imports && pnpm run docs:validate && pnpm run check:phase0-parity && pnpm run check:phase1-parity && pnpm run trace:phase0 && pnpm run replay:phase1 && pnpm run trace:phase2 -- --quiet && pnpm run trace:phase3", "verify:rootless": "bash scripts/verify-rootless-linux.sh" }, "peerDependencies": { diff --git a/packages/paperclip-runner/scripts/check-tracked-imports.mjs b/packages/paperclip-runner/scripts/check-tracked-imports.mjs new file mode 100644 index 00000000000..259dd3d0ebb --- /dev/null +++ b/packages/paperclip-runner/scripts/check-tracked-imports.mjs @@ -0,0 +1,20 @@ +import { + checkTrackedImports, + checkTrackedScriptEntryPoints, + formatTrackedImportViolation, +} from "./lib/tracked-imports.mjs"; + +const violations = [ + ...(await checkTrackedImports()), + ...(await checkTrackedScriptEntryPoints()), +]; +if (violations.length > 0) { + process.stderr.write( + `Tracked import check failed:\n${violations + .map((violation) => `- ${formatTrackedImportViolation(violation)}`) + .join("\n")}\n`, + ); + process.exitCode = 1; +} else { + process.stdout.write("Tracked import check passed.\n"); +} diff --git a/packages/paperclip-runner/scripts/check-tracked-imports.test.mjs b/packages/paperclip-runner/scripts/check-tracked-imports.test.mjs new file mode 100644 index 00000000000..79778857de3 --- /dev/null +++ b/packages/paperclip-runner/scripts/check-tracked-imports.test.mjs @@ -0,0 +1,73 @@ +import assert from "node:assert/strict"; +import { resolve } from "node:path"; +import { test } from "node:test"; + +import { + checkTrackedImports, + checkTrackedScriptEntryPoints, + defaultPackageRoot, + listTrackedFiles, +} from "./lib/tracked-imports.mjs"; + +const fixtureRoot = "test-fixtures/untracked-import"; + +test("every tracked import in the package resolves to a tracked file", async () => { + assert.deepEqual(await checkTrackedImports(), []); +}); + +test("a negative fixture proves that an untracked module is rejected", async () => { + const consumer = resolve(defaultPackageRoot, fixtureRoot, "consumer.ts"); + const violations = await checkTrackedImports({ + scanRoots: [fixtureRoot], + // `module.ts` is deliberately withheld from the tracked set: on disk but + // absent from the commit is exactly the state a clean checkout cannot build. + trackedFiles: [consumer], + }); + + assert.deepEqual( + violations.map(({ specifier, reason }) => ({ specifier, reason })), + [ + { + specifier: "./module.js", + reason: "resolves only to an untracked file, so a clean checkout cannot build it", + }, + { + specifier: "./absent.js", + reason: "does not resolve to any tracked file", + }, + ], + ); + assert.ok(violations.every((violation) => violation.file === consumer)); +}); + +test("every package.json script entry point is tracked", async () => { + assert.deepEqual(await checkTrackedScriptEntryPoints(), []); +}); + +test("a negative case proves that an untracked script entry point is rejected", async () => { + const violations = await checkTrackedScriptEntryPoints({ + // Withholding the check script itself reproduces the repository-root + // `check-*.mjs` ignore rule that kept these entry points out of commits. + trackedFiles: (await listTrackedFiles()).filter( + (file) => !file.endsWith("/scripts/check-tracked-imports.mjs"), + ), + }); + + assert.deepEqual( + violations.map(({ specifier, reason }) => ({ specifier, reason })), + [ + { + specifier: "scripts/check-tracked-imports.mjs", + reason: + 'the "check:tracked-imports" script runs a file that is not tracked, so a clean checkout cannot run it', + }, + ], + ); +}); + +test("the fixture itself is tracked so the package-wide check stays green", async () => { + const tracked = new Set(await listTrackedFiles()); + for (const name of ["consumer.ts", "module.ts"]) { + assert.ok(tracked.has(resolve(defaultPackageRoot, fixtureRoot, name)), name); + } +}); diff --git a/packages/paperclip-runner/scripts/lib/forbidden-imports.mjs b/packages/paperclip-runner/scripts/lib/forbidden-imports.mjs index 1c3c822bfe2..f9bea61dc82 100644 --- a/packages/paperclip-runner/scripts/lib/forbidden-imports.mjs +++ b/packages/paperclip-runner/scripts/lib/forbidden-imports.mjs @@ -44,7 +44,7 @@ async function collectSourceFiles(target) { return files; } -function findSpecifiers(source) { +export function findSpecifiers(source) { const found = []; for (const pattern of IMPORT_PATTERNS) { pattern.lastIndex = 0; diff --git a/packages/paperclip-runner/scripts/lib/tracked-imports.mjs b/packages/paperclip-runner/scripts/lib/tracked-imports.mjs new file mode 100644 index 00000000000..d11dd14e93f --- /dev/null +++ b/packages/paperclip-runner/scripts/lib/tracked-imports.mjs @@ -0,0 +1,214 @@ +import { execFile, spawnSync } from "node:child_process"; +import { existsSync } from "node:fs"; +import { readFile } from "node:fs/promises"; +import { dirname, relative, resolve } from "node:path"; +import { promisify } from "node:util"; + +import { defaultPackageRoot, findSpecifiers } from "./forbidden-imports.mjs"; + +export { defaultPackageRoot }; + +const execFileAsync = promisify(execFile); + +const SCANNED_EXTENSIONS = new Set([".cjs", ".js", ".jsx", ".mjs", ".ts", ".tsx"]); +// TypeScript resolves an ESM ".js" specifier against its ".ts" source sibling, +// so a tracked import may legitimately name a file that never exists on disk. +const JS_TO_SOURCE_EXTENSIONS = new Map([ + [".js", [".ts", ".tsx", ".d.ts"]], + [".jsx", [".tsx", ".d.ts"]], + [".mjs", [".mts", ".d.mts"]], + [".cjs", [".cts", ".d.cts"]], +]); +const EXTENSIONLESS_CANDIDATES = [ + ".ts", + ".tsx", + ".mts", + ".cts", + ".d.ts", + ".js", + ".jsx", + ".mjs", + ".cjs", + "/index.ts", + "/index.tsx", + "/index.js", + "/index.mjs", +]; + +function extension(path) { + const match = path.match(/\.[^./\\]+$/); + return match?.[0] ?? ""; +} + +function isScannable(path) { + return SCANNED_EXTENSIONS.has(extension(path)); +} + +function isRelative(specifier) { + return specifier.startsWith("./") || specifier.startsWith("../"); +} + +/** + * Every on-disk path a bundler or `tsc` could pick for this specifier. The + * check only needs one of them to be tracked. + */ +function resolutionCandidates(file, specifier) { + const base = resolve(dirname(file), specifier); + const specifierExtension = extension(specifier); + const sourceExtensions = JS_TO_SOURCE_EXTENSIONS.get(specifierExtension); + if (sourceExtensions !== undefined) { + const withoutExtension = base.slice(0, base.length - specifierExtension.length); + return [base, ...sourceExtensions.map((suffix) => `${withoutExtension}${suffix}`)]; + } + if (specifierExtension !== "") { + return [base]; + } + return [base, ...EXTENSIONLESS_CANDIDATES.map((suffix) => `${base}${suffix}`)]; +} + +/** + * Build output is untracked on purpose, so an import that points into an + * ignored path is not evidence of a missing source file. + */ +async function listIgnoredPaths(packageRoot, paths) { + if (paths.length === 0) { + return new Set(); + } + const result = spawnSync("git", ["check-ignore", "-z", "--stdin"], { + cwd: packageRoot, + input: paths.join("\0"), + encoding: "utf8", + maxBuffer: 64 * 1024 * 1024, + }); + if (result.error !== undefined) { + throw result.error; + } + // `git check-ignore` exits 1 when nothing matched, which is not a failure. + if (result.status !== 0 && result.status !== 1) { + throw new Error(`git check-ignore failed: ${result.stderr}`); + } + return new Set( + result.stdout + .split("\0") + .filter((entry) => entry !== "") + .map((entry) => resolve(packageRoot, entry)), + ); +} + +export async function listTrackedFiles(packageRoot = defaultPackageRoot) { + const { stdout } = await execFileAsync("git", ["ls-files", "-z", "--", "."], { + cwd: packageRoot, + maxBuffer: 64 * 1024 * 1024, + }); + return stdout + .split("\0") + .filter((entry) => entry !== "") + .map((entry) => resolve(packageRoot, entry)); +} + +function isInside(parent, candidate) { + const pathFromParent = relative(parent, candidate); + return pathFromParent !== "" && !pathFromParent.startsWith(".."); +} + +/** + * Reject tracked sources whose relative imports only resolve against files git + * does not carry. A shared worktree happily builds such a tree while a clean + * clone of the same commit cannot, so this guard runs on the tracked set only. + */ +export async function checkTrackedImports({ + packageRoot = defaultPackageRoot, + scanRoots = ["src", "scripts", "devtools", "examples"], + trackedFiles, +} = {}) { + const tracked = trackedFiles ?? (await listTrackedFiles(packageRoot)); + const trackedSet = new Set(tracked); + const roots = scanRoots.map((root) => resolve(packageRoot, root)); + const files = tracked + .filter((file) => isScannable(file) && roots.some((root) => isInside(root, file))) + .sort(); + + const suspects = []; + for (const file of files) { + const source = await readFile(file, "utf8"); + for (const { specifier, offset } of findSpecifiers(source)) { + if (!isRelative(specifier) || /[?*]|\$\{/.test(specifier)) { + continue; + } + const candidates = resolutionCandidates(file, specifier); + if (candidates.some((candidate) => trackedSet.has(candidate))) { + continue; + } + suspects.push({ + file, + line: source.slice(0, offset).split("\n").length, + specifier, + candidates, + }); + } + } + + const ignored = await listIgnoredPaths( + packageRoot, + [...new Set(suspects.flatMap((suspect) => suspect.candidates))], + ); + return suspects + .filter((suspect) => !suspect.candidates.some((candidate) => ignored.has(candidate))) + .map(({ file, line, specifier, candidates }) => ({ + file, + line, + specifier, + reason: candidates.some((candidate) => existsSync(candidate)) + ? "resolves only to an untracked file, so a clean checkout cannot build it" + : "does not resolve to any tracked file", + })); +} + +const ENTRY_POINT_TOKEN = /^[A-Za-z0-9._@/-]+\.(?:mjs|cjs|js|jsx|ts|tsx|sh|toml)$/; + +/** + * `package.json` scripts are the other way a commit can depend on a file git + * does not carry: an ignored or never-added entry point runs fine locally and + * fails immediately in a clean checkout. + */ +export async function checkTrackedScriptEntryPoints({ + packageRoot = defaultPackageRoot, + trackedFiles, +} = {}) { + const manifestPath = resolve(packageRoot, "package.json"); + const manifest = JSON.parse(await readFile(manifestPath, "utf8")); + const trackedSet = new Set(trackedFiles ?? (await listTrackedFiles(packageRoot))); + + const suspects = []; + for (const [name, body] of Object.entries(manifest.scripts ?? {})) { + for (const token of new Set(body.split(/\s+/))) { + if (!ENTRY_POINT_TOKEN.test(token) || token.startsWith("@")) { + continue; + } + const path = resolve(packageRoot, token); + if (trackedSet.has(path) || !isInside(packageRoot, path)) { + continue; + } + suspects.push({ file: manifestPath, script: name, specifier: token, candidates: [path] }); + } + } + + const ignored = await listIgnoredPaths( + packageRoot, + suspects.map((suspect) => suspect.candidates[0]), + ); + return suspects + .filter((suspect) => !ignored.has(suspect.candidates[0])) + .map(({ file, script, specifier }) => ({ + file, + line: 1, + specifier, + reason: `the "${script}" script runs a file that is not tracked, so a clean checkout cannot run it`, + })); +} + +export function formatTrackedImportViolation(violation, packageRoot = defaultPackageRoot) { + return `${relative(packageRoot, violation.file)}:${violation.line} imports ${JSON.stringify( + violation.specifier, + )}: ${violation.reason}`; +} diff --git a/packages/paperclip-runner/src/index.ts b/packages/paperclip-runner/src/index.ts index c720ca9befc..6d566d05f87 100644 --- a/packages/paperclip-runner/src/index.ts +++ b/packages/paperclip-runner/src/index.ts @@ -15,7 +15,6 @@ export * from "./drivers/codex/codex-app-server-driver.js"; export * from "./mock-core/mock-control-plane-adapter.js"; export * from "./mock-core/phase7-control-plane-types.js"; export * from "./mock-core/phase7-mock-control-plane-adapter.js"; -export * from "./standalone/phase6-demo.js"; export * from "./mock-core/phase4-codex-runner.js"; export * from "./mock-core/phase4b-demo-server.js"; export * from "./mock-core/phase4b-demo-manifests.js"; diff --git a/packages/paperclip-runner/test-fixtures/untracked-import/consumer.ts b/packages/paperclip-runner/test-fixtures/untracked-import/consumer.ts new file mode 100644 index 00000000000..e4eb60e5b11 --- /dev/null +++ b/packages/paperclip-runner/test-fixtures/untracked-import/consumer.ts @@ -0,0 +1,5 @@ +// Negative fixture for scripts/check-tracked-imports.mjs. Both files here are +// tracked; the test supplies a tracked set that omits `module.ts` to stand in +// for a source file that only exists in a contributor's worktree. +export * from "./module.js"; +export * from "./absent.js"; diff --git a/packages/paperclip-runner/test-fixtures/untracked-import/module.ts b/packages/paperclip-runner/test-fixtures/untracked-import/module.ts new file mode 100644 index 00000000000..e0f11d75eff --- /dev/null +++ b/packages/paperclip-runner/test-fixtures/untracked-import/module.ts @@ -0,0 +1 @@ +export const untrackedImportFixture = "present on disk";