Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
assertThrows,
} from "#veryfront/testing/assert.ts";
import { describe, it } from "#veryfront/testing/bdd.ts";
import { basename, join } from "#veryfront/compat/path/index.ts";
import { basename, dirname, join } from "#veryfront/compat/path/index.ts";
import {
getCycleManifestCacheDir,
} from "#veryfront/transforms/mdx/esm-module-loader/cache-format.ts";
Expand Down Expand Up @@ -191,6 +191,65 @@ describe("module-loader/cycle-manifest", () => {
}
});

it("rejects a graph root whose persisted bytes lost their sealed evidence", async () => {
const tmpDir = await Deno.makeTempDir({ prefix: "vf-cycle-manifest-missing-evidence-" });
const localAdapter = await getLocalAdapter();
const transaction = new CycleManifestTransaction(tmpDir, "missing-evidence");
const rootSource = "/project/root.ts";
const rootArtifact = transaction.registerEdge(rootSource, rootSource, false);

try {
await Deno.mkdir(dirname(rootArtifact), { recursive: true });
await transaction.sealRootArtifactCode(
"export const root = true;",
rootSource,
false,
localAdapter,
);
await Deno.writeTextFile(rootArtifact, "export const root = true;");
transaction.recordArtifact(rootSource, rootArtifact, false, true);

await assertRejects(
() => transaction.commit(localAdapter),
VeryfrontError,
"Cycle manifest root evidence is missing",
);
} finally {
await removeFixture(tmpDir);
}
});

it("rejects graph root bytes changed after sealing", async () => {
const tmpDir = await Deno.makeTempDir({ prefix: "vf-cycle-manifest-changed-root-" });
const localAdapter = await getLocalAdapter();
const transaction = new CycleManifestTransaction(tmpDir, "changed-root");
const rootSource = "/project/root.ts";
const rootArtifact = transaction.registerEdge(rootSource, rootSource, false);

try {
await Deno.mkdir(dirname(rootArtifact), { recursive: true });
const sealed = await transaction.sealRootArtifactCode(
"export const root = true;",
rootSource,
false,
localAdapter,
);
await Deno.writeTextFile(
rootArtifact,
sealed.replace("root = true", "root = false"),
);
transaction.recordArtifact(rootSource, rootArtifact, false, true);

await assertRejects(
() => transaction.commit(localAdapter),
VeryfrontError,
"Cycle manifest changed after its root was sealed",
);
} finally {
await removeFixture(tmpDir);
}
});

it("binds the complete entry set to the root artifact", async () => {
const tmpDir = await Deno.makeTempDir({ prefix: "vf-cycle-manifest-complete-" });
const localAdapter = await getLocalAdapter();
Expand Down
50 changes: 22 additions & 28 deletions src/rendering/orchestrator/module-loader/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -730,39 +730,33 @@ describe("module-loader/transformModuleWithDeps", () => {
},
async ({ projectDir, tmpDir, config }) => {
const pagePath = join(projectDir, "app/page.ts");
const [firstPath, secondPath] = await Promise.all([
transformModuleWithDeps(
pagePath,
tmpDir,
config.adapter,
{ ...config, moduleCache: new Map() },
),
transformModuleWithDeps(
pagePath,
tmpDir,
config.adapter,
{ ...config, moduleCache: new Map() },
),
]);
assertStrictEquals(firstPath, secondPath);
const moduleCache = new Map<string, string>();
const paths = await Promise.all(
Array.from({ length: 8 }, () =>
transformModuleWithDeps(
pagePath,
tmpDir,
config.adapter,
{ ...config, moduleCache },
)),
);
const firstPath = paths[0]!;
for (const path of paths) assertStrictEquals(path, firstPath);

const [firstNamespace, secondNamespace] = await Promise.all([
import(toFileUrl(firstPath).href),
import(toFileUrl(secondPath).href),
]);
const [firstCycleNamespace, secondCycleNamespace] = await Promise.all([
firstNamespace.later(),
secondNamespace.later(),
]);
const namespaces = await Promise.all(
paths.map((path) => import(toFileUrl(path).href)),
);
const cycleNamespaces = await Promise.all(
namespaces.map((namespace) => namespace.later()),
);

assertStrictEquals(firstNamespace, secondNamespace);
assertStrictEquals(firstCycleNamespace, firstNamespace);
assertStrictEquals(secondCycleNamespace, secondNamespace);
for (const namespace of [firstNamespace, secondNamespace]) {
for (let index = 0; index < namespaces.length; index++) {
const namespace = namespaces[index]!;
assertStrictEquals(namespace, namespaces[0]);
assertStrictEquals(cycleNamespaces[index], namespace);
const wrapperUrl = namespace.wrapperUrl as string;
const wrapperPath = fromFileUrl(wrapperUrl);
assertStringIncludes(wrapperUrl, "/veryfront-cycle-manifests/");
assertEquals(namespace.wrapperUrl, wrapperUrl);
assertEquals(namespace.bracketUrl, wrapperUrl);
assertEquals(namespace.aliasUrl, wrapperUrl);
assertEquals(namespace.filename, wrapperPath);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import "#veryfront/schemas/_test-setup.ts";
import { assert, assertEquals, assertNotEquals } from "#veryfront/testing/assert.ts";
import { assert, assertEquals, assertNotEquals, assertRejects } from "#veryfront/testing/assert.ts";
import { afterEach, describe, it } from "#veryfront/testing/bdd.ts";
import { basename, dirname, join } from "#veryfront/compat/path/index.ts";
import { getLocalAdapter } from "#veryfront/platform/adapters/registry.ts";
import {
makeTempDir,
mkdir,
readDir,
readTextFile,
remove,
writeTextFile,
Expand Down Expand Up @@ -39,6 +40,15 @@ function deferred<T = void>(): {
return { promise, resolve, reject };
}

/** Names of the staging files a cycle publish left behind in its directory. */
async function stagedArtifactLeftovers(artifactPath: string): Promise<string[]> {
const leftovers: string[] = [];
for await (const entry of readDir(dirname(artifactPath))) {
if (entry.name.startsWith(`${basename(artifactPath)}.pending-`)) leftovers.push(entry.name);
}
return leftovers;
}

describe("module-loader/module-persistence", () => {
const beforeDrainCleanups: Array<() => void> = [];
const afterDrainAssertions: Array<() => void> = [];
Expand Down Expand Up @@ -139,6 +149,103 @@ describe("module-loader/module-persistence", () => {
}
});

it("publishes concurrent cycle artifacts only from complete staged files", async () => {
const projectDir = await makeTempDir({ prefix: "vf-module-persist-project-" });
const tmpDir = await makeTempDir({ prefix: "vf-module-persist-out-" });
const localAdapter = await getLocalAdapter();
const cycleArtifactPath = join(tmpDir, "cycle/artifact.js");
const writes: string[] = [];
const renames: Array<[string, string]> = [];
const fs = Object.create(localAdapter.fs) as typeof localAdapter.fs;
fs.writeFile = async (path, content) => {
writes.push(path);
await localAdapter.fs.writeFile(path, content);
};
fs.rename = async (from, to) => {
renames.push([from, to]);
await localAdapter.fs.rename!(from, to);
};
const adapter = Object.create(localAdapter) as typeof localAdapter;
Object.defineProperty(adapter, "fs", { value: fs });

try {
const paths = await Promise.all(
Array.from({ length: 8 }, (_, index) =>
persistTransformedModule({
filePath: join(projectDir, "app/page.ts"),
projectDir,
tmpDir,
transformedCode: "export const page = 1;",
localAdapter: adapter,
moduleCache: new Map(),
cacheKey: `cycle-${index}`,
cycleArtifactPath,
})),
);

assertEquals(paths, Array(8).fill(cycleArtifactPath));
assert(
writes.every((path) =>
path !== cycleArtifactPath && path.startsWith(`${cycleArtifactPath}.pending-`)
),
"cycle writers must stage complete bytes away from the published path",
);
assertEquals(renames.length, 1, "identical later writers must reuse the durable artifact");
assertEquals(renames[0]?.[1], cycleArtifactPath);
assertEquals(await readTextFile(cycleArtifactPath), "export const page = 1;");
assertEquals(
await stagedArtifactLeftovers(cycleArtifactPath),
[],
"a finished publish must leave no staged file behind",
);
} finally {
await remove(projectDir, { recursive: true }).catch(() => undefined);
await remove(tmpDir, { recursive: true }).catch(() => undefined);
}
});

it("fails closed and clears staging when a cycle artifact path holds other bytes", async () => {
const projectDir = await makeTempDir({ prefix: "vf-module-persist-project-" });
const tmpDir = await makeTempDir({ prefix: "vf-module-persist-out-" });
const localAdapter = await getLocalAdapter();
const cycleArtifactPath = join(tmpDir, "cycle/artifact.js");

try {
await mkdir(dirname(cycleArtifactPath), { recursive: true });
await writeTextFile(cycleArtifactPath, "export const page = 0;");

await assertRejects(
() =>
persistTransformedModule({
filePath: join(projectDir, "app/page.ts"),
projectDir,
tmpDir,
transformedCode: "export const page = 1;",
localAdapter,
moduleCache: new Map(),
cacheKey: "cycle-conflict",
cycleArtifactPath,
}),
Error,
"Cycle artifact path contains conflicting content",
);

assertEquals(
await readTextFile(cycleArtifactPath),
"export const page = 0;",
"a conflicting publish must not replace the bytes already at the path",
);
assertEquals(
await stagedArtifactLeftovers(cycleArtifactPath),
[],
"a rejected publish must leave no staged file behind",
);
} finally {
await remove(projectDir, { recursive: true }).catch(() => undefined);
await remove(tmpDir, { recursive: true }).catch(() => undefined);
}
});

it("registers the artifact under the compile mode it was transformed with", async () => {
const projectDir = await makeTempDir({ prefix: "vf-module-persist-project-" });
const tmpDir = await makeTempDir({ prefix: "vf-module-persist-out-" });
Expand Down
74 changes: 72 additions & 2 deletions src/rendering/orchestrator/module-loader/module-persistence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import type { RuntimeAdapter } from "#veryfront/platform/adapters/base.ts";
import { CACHE_ERROR } from "#veryfront/errors";
import { join } from "#veryfront/compat/path/index.ts";
import { rendererLogger } from "#veryfront/utils";
import { isCacheWriteRaceError } from "#veryfront/utils/cache-file-ops.ts";
Expand Down Expand Up @@ -32,6 +33,7 @@ const createdDirs = new Set<string>();
type ModulePathCacheSave = (cacheDir: string) => Promise<void>;

const pendingModulePathCacheSaves = new Set<Promise<void>>();
const cycleArtifactPublications = new Map<string, Promise<void>>();
let modulePathCacheSave: ModulePathCacheSave = saveModulePathCache;

/** Prune oldest entries when cache exceeds limit. */
Expand Down Expand Up @@ -99,6 +101,69 @@ async function ensureDir(
pruneCreatedDirs();
}

async function readExistingCycleArtifact(
adapter: RuntimeAdapter,
artifactPath: string,
): Promise<string | undefined> {
try {
return await adapter.fs.readFile(artifactPath);
} catch (error) {
if (isNotFoundError(error)) return undefined;
throw error;
}
}

async function publishCycleArtifact(
adapter: RuntimeAdapter,
artifactPath: string,
code: string,
): Promise<void> {
const stagedPath = `${artifactPath}.pending-${crypto.randomUUID()}`;
try {
await adapter.fs.writeFile(stagedPath, code);
const previous = cycleArtifactPublications.get(artifactPath) ?? Promise.resolve();
const publication = previous.catch(() => undefined).then(async () => {
const existing = await readExistingCycleArtifact(adapter, artifactPath);
if (existing !== undefined) {
if (existing !== code) {
throw CACHE_ERROR.create({
detail: "Cycle artifact path contains conflicting content",
});
}
return;
}
if (!adapter.fs.rename) {
throw CACHE_ERROR.create({
detail: "Cycle artifact filesystem cannot publish an atomic replacement",
});
}
try {
await adapter.fs.rename(stagedPath, artifactPath);
Comment thread
kwakayama marked this conversation as resolved.
} catch (error) {
// A different process can win after the read above. Reuse only the
// complete artifact with the exact immutable bytes this path denotes.
const raced = await readExistingCycleArtifact(adapter, artifactPath);
if (raced === code) return;
throw error;
}
}).finally(() => {
if (cycleArtifactPublications.get(artifactPath) === publication) {
cycleArtifactPublications.delete(artifactPath);
}
});
cycleArtifactPublications.set(artifactPath, publication);
await publication;
} finally {
try {
await adapter.fs.remove(stagedPath);
} catch (error) {
if (!isNotFoundError(error)) {
logger.debug("Failed to remove a staged cycle artifact", { error });
}
}
}
}

export interface PersistTransformedModuleInput {
filePath: string;
projectDir: string;
Expand Down Expand Up @@ -574,8 +639,13 @@ export async function persistTransformedModule(
// Fall through to the write, which retries the mkdir on failure.
});

const writeArtifact = () =>
input.cycleArtifactPath
? publishCycleArtifact(input.localAdapter, tempFilePath, input.transformedCode)
: input.localAdapter.fs.writeFile(tempFilePath, input.transformedCode);

try {
await input.localAdapter.fs.writeFile(tempFilePath, input.transformedCode);
await writeArtifact();
} catch (error) {
// The cache directory can vanish between mkdir and write — a manual
// `rm -rf .cache`, a cache sweep, or a mkdir that never actually landed.
Expand All @@ -591,7 +661,7 @@ export async function persistTransformedModule(

try {
await ensureDir(input.localAdapter, tempDir, true);
await input.localAdapter.fs.writeFile(tempFilePath, input.transformedCode);
await writeArtifact();
logger.debug("Recreated module cache directory after failed write", { tempDir });
} catch (retryError) {
logger.error("Failed to write module:", {
Expand Down
Loading