fix: stop test fixtures from deleting the repository's dependencies
Four fixtures linked the installed dependencies into a throwaway root with a single directory symlink at <fixture>/node_modules, then ran pnpm inside that root. pnpm does not recognise the modules directory it finds there and purges it; with CI=true it does so without a prompt. The purge followed the symlink and deleted the repository's own node_modules mid-run, so a test suite uninstalled the workspace it was running in. That is what produced the cascading, file-unrelated failures a full test:unit run reported, and it happened twice while running the suites for the adapter re-review. scripts/lib/fixture-node-modules.ts replaces all four sites: node_modules is a real directory whose entries are individual symlinks, so a recursive delete unlinks the fixture's own links instead of walking through one link into the shared tree. Resolution is unchanged. tests/unit/fixture-node-modules.test.ts performs the exact recursive delete pnpm performs and asserts the source tree survives, and check:adapter-inventory now fails on any reintroduction of the directory-symlink form — verified by putting the old line back and watching the gate reject it. A full tests/unit + tests/integration run now leaves the dependencies intact. removal-fixture, supply-chain and security-followup-archive, the three suites that had to be excluded before, pass in that run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
af7f35058b
commit
250531aa43
@@ -314,8 +314,50 @@ Run on the landed tree. Only what actually passed is claimed as passing.
|
||||
| --- | --- | --- |
|
||||
| `tests/unit/ci-workflow-generation.test.ts` | 82 failed / 325 passed | Identical on the pre-change baseline (`git stash` comparison). The subprocess gates it spawns cannot run in this sandbox. |
|
||||
| `tests/unit/ci-artifact-contract.test.ts` | fails | Unchanged pre-existing sandbox, cgroup and `/tmp` permission behaviour. |
|
||||
| `tests/unit/removal-fixture.test.ts`, `tests/unit/supply-chain.test.ts`, `tests/integration/security-followup-archive.test.ts` | destructive | `scripts/lib/removal-fixture.ts:45` and `scripts/check-supply-chain-provider-fixtures.ts:62` symlink the real `node_modules` into a temp fixture root and then run `corepack pnpm` there with `CI=true`. pnpm purges the modules directory it does not recognise, **through the symlink**, deleting the repository's own dependencies mid-run. This is a pre-existing repository hazard, outside the 38 findings, and it is why a full `test:unit` run reports cascading failures. |
|
||||
| `tests/unit/security-followup.test.ts` | flaky under load | Passes in a fresh process; the process-group reaping assertion is timing sensitive. |
|
||||
| `tests/unit/security-followup.test.ts`, `tests/unit/provider-guardian-transaction.test.ts` | flaky under full-suite load | Both pass in a fresh process (49 passed). They spawn and reap process groups, so their timing assertions are load sensitive. |
|
||||
|
||||
### Destructive fixture hazard — fixed
|
||||
|
||||
Outside the 38 findings, and found while running the suites for them.
|
||||
|
||||
Four sites linked the repository's installed dependencies into a throwaway
|
||||
fixture with a single directory symlink at `<fixture>/node_modules`:
|
||||
|
||||
- `scripts/lib/removal-fixture.ts`
|
||||
- `scripts/check-supply-chain-provider-fixtures.ts`
|
||||
- `tests/integration/security-followup-archive.test.ts`
|
||||
- `tests/unit/ci-artifact-contract.test.ts`
|
||||
|
||||
Each fixture then runs `pnpm` inside itself. pnpm does not recognise the modules
|
||||
directory it finds there and purges it; with `CI=true` it does so without a
|
||||
prompt. The purge followed the symlink and deleted the **repository's own**
|
||||
`node_modules` mid-run — a test suite uninstalling the workspace it was running
|
||||
in. That is what produced the cascading, file-unrelated failures a full
|
||||
`test:unit` run reported, and it happened twice during this work.
|
||||
|
||||
`scripts/lib/fixture-node-modules.ts` replaces all four: `node_modules` is a
|
||||
real directory whose entries are individual symlinks, so a recursive delete
|
||||
unlinks the fixture's own links instead of walking through one link into the
|
||||
shared tree. `tests/unit/fixture-node-modules.test.ts` performs the exact
|
||||
recursive delete pnpm performs and asserts the source tree survives, and
|
||||
`corepack pnpm check:adapter-inventory` fails on any reintroduction of the
|
||||
directory-symlink form.
|
||||
|
||||
After the fix a full `tests/unit` + `tests/integration` run leaves the
|
||||
dependencies intact and its failures are confined to the two environmental
|
||||
files above plus the two flaky-under-load ones:
|
||||
|
||||
| File | Failed | Attribution |
|
||||
| --- | ---: | --- |
|
||||
| `tests/unit/ci-workflow-generation.test.ts` | 82 | Identical on the pre-change baseline (`git stash` comparison). Its subprocess gates cannot run in this sandbox. |
|
||||
| `tests/unit/ci-artifact-contract.test.ts` | 19 | Unchanged pre-existing sandbox, cgroup and `/tmp` permission behaviour. |
|
||||
| `tests/unit/security-followup.test.ts` | 2 | Passes in isolation. |
|
||||
| `tests/unit/provider-guardian-transaction.test.ts` | 1 | Passes in isolation. |
|
||||
|
||||
1619 passed / 1723 total, and `tests/unit/removal-fixture.test.ts`,
|
||||
`tests/unit/supply-chain.test.ts` and
|
||||
`tests/integration/security-followup-archive.test.ts` — the three that had to be
|
||||
excluded before — now pass in the full run.
|
||||
|
||||
## Rules for updating this ledger
|
||||
|
||||
|
||||
@@ -91,6 +91,38 @@ async function main(): Promise<void> {
|
||||
}
|
||||
}
|
||||
|
||||
// A fixture that links the repository's node_modules with a single directory
|
||||
// symlink is destructive: pnpm running inside that fixture purges the modules
|
||||
// directory it does not recognise, follows the link, and deletes the real
|
||||
// dependencies mid-run. `linkFixtureNodeModules` is the only sanctioned form.
|
||||
const sources = spawnSync(
|
||||
"git",
|
||||
["grep", "-n", "-e", 'symlink(', "--", "scripts", "tests"],
|
||||
{ encoding: "utf8" },
|
||||
);
|
||||
if (sources.status === 0) {
|
||||
for (const line of sources.stdout.split("\n").filter(Boolean)) {
|
||||
if (!line.includes("node_modules")) continue;
|
||||
if (line.startsWith("scripts/lib/fixture-node-modules.ts:")) continue;
|
||||
problems.push(
|
||||
`fixture node_modules: use linkFixtureNodeModules instead — ${line}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
const linkedFixtures = spawnSync(
|
||||
"git",
|
||||
["grep", "-l", "linkFixtureNodeModules", "--", "scripts", "tests"],
|
||||
{ encoding: "utf8" },
|
||||
);
|
||||
if (
|
||||
linkedFixtures.status !== 0 ||
|
||||
linkedFixtures.stdout.split("\n").filter(Boolean).length < 2
|
||||
) {
|
||||
problems.push(
|
||||
"fixture node_modules: the shared linker has no callers, so it is not the sanctioned path",
|
||||
);
|
||||
}
|
||||
|
||||
if (problems.length > 0) {
|
||||
for (const problem of problems) console.error(problem);
|
||||
process.exitCode = 1;
|
||||
@@ -100,7 +132,8 @@ async function main(): Promise<void> {
|
||||
`Adapter inventory: ${tracked.length} files PASS; ` +
|
||||
`service worker asset table: ${
|
||||
Object.keys(CACHEABLE_ASSET_CONTENT_TYPES).length
|
||||
} shared extensions PASS`,
|
||||
} shared extensions PASS; ` +
|
||||
`fixture node_modules linking PASS`,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -12,7 +12,6 @@ import {
|
||||
readFile,
|
||||
readdir,
|
||||
rm,
|
||||
symlink,
|
||||
writeFile,
|
||||
} from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
@@ -42,6 +41,7 @@ import {
|
||||
releaseCandidateManifestSchema,
|
||||
} from "./lib/release-candidate.ts";
|
||||
import { writeValidatedJsonArtifact } from "./lib/validated-json-artifact.ts";
|
||||
import { linkFixtureNodeModules } from "./lib/fixture-node-modules.ts";
|
||||
|
||||
const repositoryRoot = process.cwd();
|
||||
const fixtureRoot = await mkdtemp(path.join(tmpdir(), "provider-exact-five-fixture-"));
|
||||
@@ -59,7 +59,7 @@ try {
|
||||
recursive: true,
|
||||
});
|
||||
await rm(path.join(fixtureRoot, "artifacts/release"), { recursive: true, force: true });
|
||||
await symlink(path.join(repositoryRoot, "node_modules"), path.join(fixtureRoot, "node_modules"), "dir");
|
||||
await linkFixtureNodeModules(fixtureRoot, repositoryRoot);
|
||||
const git = spawnSync("git", ["show", "-s", "--format=%H%n%ct", "HEAD"], {
|
||||
cwd: repositoryRoot,
|
||||
encoding: "utf8",
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
import { lstat, mkdir, readdir, symlink } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
|
||||
/**
|
||||
* Links the repository's installed dependencies into a throwaway fixture root.
|
||||
*
|
||||
* The obvious form — one directory symlink at `<fixture>/node_modules` — is
|
||||
* destructive. A fixture runs `pnpm` inside itself, pnpm does not recognise the
|
||||
* modules directory it finds there, and its purge follows the symlink and
|
||||
* deletes the *repository's* real dependencies mid-run. With `CI=true` that
|
||||
* happens without a prompt, so a test suite silently uninstalls the workspace
|
||||
* it is running in.
|
||||
*
|
||||
* `node_modules` is therefore a real directory here, and every entry inside it
|
||||
* is an individual symlink. Package resolution is unchanged, but a recursive
|
||||
* delete unlinks the fixture's own symlinks instead of walking through one link
|
||||
* into the shared tree.
|
||||
*/
|
||||
export async function linkFixtureNodeModules(
|
||||
fixtureRoot: string,
|
||||
sourceRoot: string = process.cwd(),
|
||||
): Promise<void> {
|
||||
const source = path.join(sourceRoot, "node_modules");
|
||||
const target = path.join(fixtureRoot, "node_modules");
|
||||
await mkdir(target, { recursive: true });
|
||||
for (const entry of await readdir(source)) {
|
||||
const from = path.join(source, entry);
|
||||
const stats = await lstat(from);
|
||||
await symlink(from, path.join(target, entry), stats.isDirectory() ? "dir" : "file");
|
||||
}
|
||||
}
|
||||
@@ -5,7 +5,6 @@ import {
|
||||
readFile,
|
||||
readdir,
|
||||
rm,
|
||||
symlink,
|
||||
writeFile,
|
||||
} from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
@@ -14,6 +13,7 @@ import {
|
||||
loadCiGateContract,
|
||||
parseCiGateContract,
|
||||
} from "../contracts/ci-gates.ts";
|
||||
import { linkFixtureNodeModules } from "./fixture-node-modules.ts";
|
||||
import { generateCiWorkflow } from "../generate-ci-workflow.ts";
|
||||
|
||||
export const REMOVAL_FIXTURE_COPY_TARGETS = Object.freeze([
|
||||
@@ -42,7 +42,7 @@ export async function prepareRemovalFixture(
|
||||
for (const target of copyTargets) {
|
||||
await cp(target, path.join(root, target), { recursive: true });
|
||||
}
|
||||
await symlink(path.resolve("node_modules"), path.join(root, "node_modules"), "dir");
|
||||
await linkFixtureNodeModules(root);
|
||||
}
|
||||
|
||||
export function runRemovalFixturePnpm(
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { cp, mkdir, mkdtemp, readFile, rm, symlink, writeFile } from "node:fs/promises";
|
||||
import { cp, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
RELEASE_CANDIDATE_MANIFEST_PATH,
|
||||
releaseCandidateManifestSchema,
|
||||
} from "../../scripts/lib/release-candidate.ts";
|
||||
import { linkFixtureNodeModules } from "../../scripts/lib/fixture-node-modules.ts";
|
||||
|
||||
it(
|
||||
"builds a real candidate assessment and passes the default archived verifier from the captured archive",
|
||||
@@ -43,7 +44,7 @@ it(
|
||||
recursive: true,
|
||||
force: true,
|
||||
});
|
||||
await symlink(path.join(sourceRoot, "node_modules"), path.join(fixtureRoot, "node_modules"), "dir");
|
||||
await linkFixtureNodeModules(fixtureRoot, sourceRoot);
|
||||
const git = spawnSync("git", ["show", "-s", "--format=%H%n%ct", "HEAD"], {
|
||||
cwd: sourceRoot,
|
||||
encoding: "utf8",
|
||||
|
||||
@@ -19,6 +19,7 @@ import {
|
||||
validateCiArtifact,
|
||||
} from "../../scripts/lib/ci-artifact-validator.ts";
|
||||
import { writeCiGateLogAtomic } from "../../scripts/lib/ci-gate-log.ts";
|
||||
import { linkFixtureNodeModules } from "../../scripts/lib/fixture-node-modules.ts";
|
||||
import { captureCiCandidateArchive, verifyCiCandidateArchive } from "../../scripts/lib/ci-candidate-archive.ts";
|
||||
import {
|
||||
CANDIDATE_ARCHIVE_USAGE,
|
||||
@@ -2131,7 +2132,7 @@ async function ensureProviderBaseFixture(): Promise<string> {
|
||||
recursive: true,
|
||||
});
|
||||
await rm(path.join(root, "artifacts/release"), { recursive: true, force: true });
|
||||
await symlink(path.join(sourceRoot, "node_modules"), path.join(root, "node_modules"), "dir");
|
||||
await linkFixtureNodeModules(root, sourceRoot);
|
||||
const git = spawnSync("git", ["show", "-s", "--format=%H%n%ct", "HEAD"], {
|
||||
cwd: sourceRoot,
|
||||
encoding: "utf8",
|
||||
|
||||
@@ -0,0 +1,89 @@
|
||||
import { mkdtemp, mkdir, readdir, readFile, rm, stat, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { linkFixtureNodeModules } from "../../scripts/lib/fixture-node-modules.ts";
|
||||
|
||||
/**
|
||||
* A removal or provider fixture runs `pnpm` inside a throwaway copy of the
|
||||
* repository. pnpm does not recognise the modules directory it finds there and
|
||||
* purges it, and with `CI=true` it does so without a prompt. When
|
||||
* `<fixture>/node_modules` was a single directory symlink, that purge followed
|
||||
* the link and deleted the repository's own installed dependencies mid-run —
|
||||
* a test suite uninstalling the workspace it was running in.
|
||||
*/
|
||||
describe("fixture node_modules linking", () => {
|
||||
async function sourceTree() {
|
||||
const source = await mkdtemp(path.join(tmpdir(), "fixture-source-"));
|
||||
const modules = path.join(source, "node_modules");
|
||||
await mkdir(path.join(modules, "left", "lib"), { recursive: true });
|
||||
await mkdir(path.join(modules, "@scope", "right"), { recursive: true });
|
||||
await mkdir(path.join(modules, ".bin"), { recursive: true });
|
||||
await writeFile(path.join(modules, "left", "lib", "index.js"), "export default 1;\n");
|
||||
await writeFile(path.join(modules, "@scope", "right", "package.json"), "{}\n");
|
||||
await writeFile(path.join(modules, ".modules.yaml"), "{}\n");
|
||||
return source;
|
||||
}
|
||||
|
||||
it("survives a recursive delete of the fixture's node_modules", async () => {
|
||||
const source = await sourceTree();
|
||||
const fixture = await mkdtemp(path.join(tmpdir(), "fixture-root-"));
|
||||
try {
|
||||
await linkFixtureNodeModules(fixture, source);
|
||||
|
||||
// The fixture resolves the same packages...
|
||||
expect(
|
||||
await readFile(
|
||||
path.join(fixture, "node_modules/left/lib/index.js"),
|
||||
"utf8",
|
||||
),
|
||||
).toContain("export default 1;");
|
||||
expect(
|
||||
(await readdir(path.join(fixture, "node_modules"))).sort(),
|
||||
).toEqual([".bin", ".modules.yaml", "@scope", "left"]);
|
||||
|
||||
// ...and this is exactly what pnpm's purge does.
|
||||
await rm(path.join(fixture, "node_modules"), {
|
||||
recursive: true,
|
||||
force: true,
|
||||
});
|
||||
|
||||
// The repository's real dependencies are untouched.
|
||||
expect(
|
||||
await readFile(
|
||||
path.join(source, "node_modules/left/lib/index.js"),
|
||||
"utf8",
|
||||
),
|
||||
).toContain("export default 1;");
|
||||
expect(
|
||||
await stat(path.join(source, "node_modules/@scope/right/package.json")),
|
||||
).toBeDefined();
|
||||
expect(
|
||||
(await readdir(path.join(source, "node_modules"))).sort(),
|
||||
).toEqual([".bin", ".modules.yaml", "@scope", "left"]);
|
||||
} finally {
|
||||
await rm(source, { recursive: true, force: true });
|
||||
await rm(fixture, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("creates node_modules as a real directory, never a symlink", async () => {
|
||||
const source = await sourceTree();
|
||||
const fixture = await mkdtemp(path.join(tmpdir(), "fixture-root-"));
|
||||
try {
|
||||
await linkFixtureNodeModules(fixture, source);
|
||||
const { lstat } = await import("node:fs/promises");
|
||||
const entry = await lstat(path.join(fixture, "node_modules"));
|
||||
expect(entry.isSymbolicLink()).toBe(false);
|
||||
expect(entry.isDirectory()).toBe(true);
|
||||
expect(
|
||||
(await lstat(path.join(fixture, "node_modules/left"))).isSymbolicLink(),
|
||||
).toBe(true);
|
||||
} finally {
|
||||
await rm(source, { recursive: true, force: true });
|
||||
await rm(fixture, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user