mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 17:45:53 +01:00
fix(runner): drop the untracked standalone export from the package barrel
`src/index.ts` re-exported `./standalone/phase6-demo.js`, but that module only ever existed as an untracked file in the shared worktree. The commit therefore built there and failed `build:typescript` with TS2307 in any clean checkout, which blocked the Phase 7U deployment. Nothing tracked consumes the module, so removing the export restores a buildable tree without touching the accepted remediation semantics — the issue-thread bundle stays byte-identical at index-Df3v8-4P.js, SHA-256 2e37dcb74d544ed103fdf6f22554de9e8747ab11067f7b3e8fb6c5811e38a528. Guard the class of defect rather than the instance. `check:tracked-imports` rejects any tracked source whose relative import resolves only against an untracked file, and any `package.json` script whose entry point is untracked. Ignored build output is excused via `git check-ignore`. The same sweep found the second half of the trap: the repository-root `.gitignore` swallows `packages/paperclip-runner/scripts/check-*.mjs` because its `!scripts/check-*.mjs` negation is anchored to the root. Existing checks survived only through `git add -f`. This package's `.gitignore` now re-includes them so new checks cannot be silently dropped from a commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
31b1e228b3
commit
e995b59933
10 files changed
+323
-4
No files matched your search
@@ -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
|
||||
@@ -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. |
|
||||
|
||||
@@ -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": {
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
@@ -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);
|
||||
}
|
||||
});
|
||||
@@ -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;
|
||||
|
||||
@@ -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}`;
|
||||
}
|
||||
@@ -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";
|
||||
|
||||
@@ -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";
|
||||
@@ -0,0 +1 @@
|
||||
export const untrackedImportFixture = "present on disk";
|
||||
Reference in new issue
Block a user