From c64021032fb6f03bffa9215798a51a839bb7bdd2 Mon Sep 17 00:00:00 2001 From: Jeff Repanich Date: Sat, 15 Aug 2026 14:50:07 -0400 Subject: [PATCH 1/4] test: expose concurrent add write loss --- tests/cli-smoke.test.ts | 114 +++++++++++++++++++++++++++++++++++++ tests/file-changes.test.ts | 22 +++++++ 2 files changed, 136 insertions(+) diff --git a/tests/cli-smoke.test.ts b/tests/cli-smoke.test.ts index 671ca3d..b793cb8 100644 --- a/tests/cli-smoke.test.ts +++ b/tests/cli-smoke.test.ts @@ -36,6 +36,21 @@ function createIo(): { }; } +function createConcurrentWriter(parties: number): typeof writeFileChanges { + let arrivals = 0; + let release: (() => void) | undefined; + const ready = new Promise((resolve) => { + release = resolve; + }); + + return async (changes, options) => { + arrivals += 1; + if (arrivals === parties) release?.(); + await ready; + await writeFileChanges(changes, options); + }; +} + function getMarkdownSection(markdown: string, heading: string): string { const escapedHeading = heading.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); const match = markdown.match( @@ -718,6 +733,40 @@ test("should ensure runAddCli scaffolds a page and registers the app route", asy } }); +test("should ensure concurrent add page commands cannot silently lose a registration", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "askr-cli-add-concurrent-page-")); + const previousCwd = process.cwd(); + try { + process.chdir(tempRoot); + expect( + await runCreateCli(["spa", "sample-spa", "--no-install", "--no-skills"], createIo().io), + ).toBe(0); + const appRoot = path.join(tempRoot, "sample-spa"); + const alpha = createIo(); + const beta = createIo(); + const writer = createConcurrentWriter(2); + + const results = await Promise.all([ + runAddCli(["page", "alpha", "--cwd", appRoot], alpha.io, writer), + runAddCli(["page", "beta", "--cwd", appRoot], beta.io, writer), + ]); + + expect([...results].sort()).toEqual([0, 1]); + const failed = results[0] === 1 ? { name: "alpha", errors: alpha.errors } : { name: "beta", errors: beta.errors }; + const succeeded = results[0] === 0 ? "alpha" : "beta"; + expect(failed.errors.join("\n")).toContain("File changed before writing"); + + const routes = await fs.readFile(path.join(appRoot, "src/pages/app/_routes.tsx"), "utf8"); + expect(routes).toContain(`route('/app/${succeeded}',`); + expect(routes).not.toContain(`route('/app/${failed.name}',`); + await expect(fs.access(path.join(appRoot, `src/pages/app/${succeeded}.tsx`))).resolves.toBeUndefined(); + await expect(fs.access(path.join(appRoot, `src/pages/app/${failed.name}.tsx`))).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + process.chdir(previousCwd); + await fs.rm(tempRoot, { recursive: true, force: true }); + } +}); + test("should ensure runAddCli transactionally scaffolds both database dialects", async () => { for (const dialect of ["sqlite", "postgres"] as const) { const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), `askr-cli-database-${dialect}-`)); @@ -751,6 +800,30 @@ test("should ensure runAddCli transactionally scaffolds both database dialects", } }); +test("should ensure add database rejects a package manifest changed after planning", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "askr-cli-database-conflict-")); + const manifestFile = path.join(tempRoot, "package.json"); + const externalManifest = `${JSON.stringify({ name: "database-app", dependencies: { external: "1.0.0" } }, null, 2)}\n`; + try { + await fs.writeFile( + manifestFile, + `${JSON.stringify({ name: "database-app", type: "module" }, null, 2)}\n`, + ); + const { io, errors } = createIo(); + const code = await runAddCli(["database", "sqlite", "--cwd", tempRoot], io, async (changes, options) => { + await fs.writeFile(manifestFile, externalManifest); + await writeFileChanges(changes, options); + }); + + expect(code).toBe(1); + expect(errors.join("\n")).toContain("File changed before writing"); + expect(await fs.readFile(manifestFile, "utf8")).toBe(externalManifest); + await expect(fs.access(path.join(tempRoot, "src/database/index.ts"))).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + await fs.rm(tempRoot, { recursive: true, force: true }); + } +}); + test("should ensure runAddCli rolls back page registration given a replacement failure", async () => { const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "askr-cli-add-rollback-")); const previousCwd = process.cwd(); @@ -901,6 +974,47 @@ test("should ensure runAddCli generates a browser-safe action and server registr } }); +test("should ensure concurrent add action commands cannot silently lose a registration", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "askr-cli-add-concurrent-action-")); + const previousCwd = process.cwd(); + try { + process.chdir(tempRoot); + expect( + await runCreateCli(["full-stack", "sample-full-stack", "--no-install", "--no-skills"], createIo().io), + ).toBe(0); + const appRoot = path.join(tempRoot, "sample-full-stack"); + const archive = createIo(); + const publish = createIo(); + const writer = createConcurrentWriter(2); + + const results = await Promise.all([ + runAddCli(["action", "archive-project", "--route", "/", "--cwd", appRoot], archive.io, writer), + runAddCli(["action", "publish-project", "--route", "/", "--cwd", appRoot], publish.io, writer), + ]); + + expect([...results].sort()).toEqual([0, 1]); + const failed = results[0] === 1 + ? { slug: "archive-project", identifier: "archiveProject", errors: archive.errors } + : { slug: "publish-project", identifier: "publishProject", errors: publish.errors }; + const succeeded = results[0] === 0 + ? { slug: "archive-project", identifier: "archiveProject" } + : { slug: "publish-project", identifier: "publishProject" }; + expect(failed.errors.join("\n")).toContain("File changed before writing"); + + const registry = await fs.readFile(path.join(appRoot, "src/server/action-registry.ts"), "utf8"); + const authorizations = await fs.readFile(path.join(appRoot, "src/action-authorizations.ts"), "utf8"); + expect(registry).toContain(`${succeeded.identifier}Action`); + expect(registry).not.toContain(`${failed.identifier}Action`); + expect(authorizations).toContain(`${succeeded.identifier}Action`); + expect(authorizations).not.toContain(`${failed.identifier}Action`); + await expect(fs.access(path.join(appRoot, `src/actions/${succeeded.slug}.ts`))).resolves.toBeUndefined(); + await expect(fs.access(path.join(appRoot, `src/actions/${failed.slug}.ts`))).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + process.chdir(previousCwd); + await fs.rm(tempRoot, { recursive: true, force: true }); + } +}); + test("should ensure runSsgCli prints help without requiring config", async () => { const { io, logs, errors } = createIo(); const code = await runSsgCli(["--help"], undefined, io); diff --git a/tests/file-changes.test.ts b/tests/file-changes.test.ts index 5b46db4..e1ab138 100644 --- a/tests/file-changes.test.ts +++ b/tests/file-changes.test.ts @@ -11,6 +11,28 @@ afterEach(async () => { }); describe("writeFileChanges", () => { + it("should reject a stale shared-file edit before writing any transaction artifacts", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "askr-file-changes-stale-")); + roots.push(root); + const shared = path.join(root, "shared.ts"); + const created = path.join(root, "created.ts"); + await fs.writeFile(shared, "changed by another process\n"); + + await expect( + writeFileChanges([ + { filePath: created, content: "orphan\n" }, + { + filePath: shared, + content: "planned replacement\n", + expectedContent: "original at plan time\n", + }, + ]), + ).rejects.toThrow("File changed before writing"); + + expect(await fs.readFile(shared, "utf8")).toBe("changed by another process\n"); + await expect(fs.stat(created)).rejects.toMatchObject({ code: "ENOENT" }); + }); + it("should restore replaced files and remove created files after a replacement failure", async () => { const root = await fs.mkdtemp(path.join(os.tmpdir(), "askr-file-changes-")); roots.push(root); From 1e7529c63751bfaede957fc43beb8aabbd94708e Mon Sep 17 00:00:00 2001 From: Jeff Repanich Date: Sat, 15 Aug 2026 14:52:23 -0400 Subject: [PATCH 2/4] fix: reject stale concurrent add writes --- src/bin/add.ts | 45 ++++++++++---- src/file-changes.ts | 116 +++++++++++++++++++++++++++++++++++++ tests/cli-smoke.test.ts | 73 ++++++++++++++++------- tests/file-changes.test.ts | 3 + 4 files changed, 207 insertions(+), 30 deletions(-) diff --git a/src/bin/add.ts b/src/bin/add.ts index 31daea9..76aebc4 100644 --- a/src/bin/add.ts +++ b/src/bin/add.ts @@ -498,9 +498,10 @@ async function addPage(parsed: ParsedArgs, io: CliIo, writeChanges: WriteChanges } const importSpecifier = toImportSpecifier(routesFile, pageFile); + let routeFileContent = ""; let updatedRoutes = ""; try { - const routeFileContent = await fs.readFile(routesFile, "utf8"); + routeFileContent = await fs.readFile(routesFile, "utf8"); updatedRoutes = createUpdatedRouteFile(routeFileContent, { componentName, importSpecifier, @@ -522,7 +523,7 @@ async function addPage(parsed: ParsedArgs, io: CliIo, writeChanges: WriteChanges title, }), }, - { filePath: routesFile, content: updatedRoutes }, + { filePath: routesFile, content: updatedRoutes, expectedContent: routeFileContent }, ]); } catch (error) { io.error("Failed to write generated page artifacts."); @@ -586,6 +587,10 @@ async function addAction( filePath: descriptorFile, content: descriptor, }); + const [registryContent, authorizationContent] = await Promise.all([ + fs.readFile(registryFile, "utf8"), + fs.readFile(authorizationFile, "utf8"), + ]); await writeChanges([ { filePath: descriptorFile, content: descriptor }, { @@ -593,8 +598,16 @@ async function addAction( content: renderActionHandler({ descriptorName, handlerName, slug }), }, { filePath: testFile, content: renderActionTest({ descriptorName, slug }) }, - { filePath: registryFile, content: renderServerActionRegistry(actions) }, - { filePath: authorizationFile, content: renderAuthorizationRegistry(actions) }, + { + filePath: registryFile, + content: renderServerActionRegistry(actions), + expectedContent: registryContent, + }, + { + filePath: authorizationFile, + content: renderAuthorizationRegistry(actions), + expectedContent: authorizationContent, + }, ]); } catch (error) { io.error("Failed to write generated action artifacts."); @@ -633,7 +646,8 @@ async function addDatabase( return 1; } try { - const manifest = JSON.parse(await fs.readFile(manifestFile, "utf8")) as { + const manifestContent = await fs.readFile(manifestFile, "utf8"); + const manifest = JSON.parse(manifestContent) as { dependencies?: Record; }; manifest.dependencies = { @@ -644,7 +658,10 @@ async function addDatabase( manifest.dependencies = Object.fromEntries( Object.entries(manifest.dependencies).sort(([left], [right]) => left.localeCompare(right)), ); - const currentEnvironment = await fs.readFile(environmentFile, "utf8").catch(() => ""); + const currentEnvironment = await fs.readFile(environmentFile, "utf8").catch((error) => { + if (error instanceof Error && "code" in error && error.code === "ENOENT") return null; + throw error; + }); const environmentLines = parsed.name === "postgres" ? [ @@ -653,9 +670,9 @@ async function addDatabase( ] : ["DATABASE_PATH=./data/app.sqlite"]; const additions = environmentLines.filter( - (line) => !currentEnvironment.includes(`${line.split("=")[0]}=`), + (line) => !currentEnvironment?.includes(`${line.split("=")[0]}=`), ); - const environment = `${currentEnvironment}${currentEnvironment && !currentEnvironment.endsWith("\n") ? "\n" : ""}${additions.join("\n")}${additions.length ? "\n" : ""}`; + const environment = `${currentEnvironment ?? ""}${currentEnvironment && !currentEnvironment.endsWith("\n") ? "\n" : ""}${additions.join("\n")}${additions.length ? "\n" : ""}`; const driverImport = parsed.name; const definition = [ "import { defineDatabase } from '@askrjs/orm';", @@ -684,8 +701,16 @@ async function addDatabase( { filePath: definitionFile, content: definition }, { filePath: generatedFile, content: generated }, { filePath: migrationKeep, content: "" }, - { filePath: environmentFile, content: environment }, - { filePath: manifestFile, content: `${JSON.stringify(manifest, null, 2)}\n` }, + { + filePath: environmentFile, + content: environment, + expectedContent: currentEnvironment, + }, + { + filePath: manifestFile, + content: `${JSON.stringify(manifest, null, 2)}\n`, + expectedContent: manifestContent, + }, ]); } catch (error) { io.error("Failed to write generated database artifacts."); diff --git a/src/file-changes.ts b/src/file-changes.ts index c80a709..1b1fca8 100644 --- a/src/file-changes.ts +++ b/src/file-changes.ts @@ -5,6 +5,8 @@ import { randomUUID } from "node:crypto"; export interface FileChange { readonly filePath: string; readonly content: string; + /** Content observed while planning a shared-file edit; `null` means the file was absent. */ + readonly expectedContent?: string | null; } interface StagedChange extends FileChange { @@ -17,6 +19,98 @@ export interface FileChangeWriterOptions { readonly replace?: (temporaryPath: string, filePath: string) => Promise; } +interface FileLock { + readonly lockPath: string; +} + +const LOCK_RETRY_MS = 10; +const LOCK_TIMEOUT_MS = 10_000; +const ORPHANED_LOCK_AGE_MS = 30_000; + +function isNodeError(error: unknown, code: string): error is NodeJS.ErrnoException { + return error instanceof Error && "code" in error && error.code === code; +} + +function delay(milliseconds: number): Promise { + return new Promise((resolve) => setTimeout(resolve, milliseconds)); +} + +async function ownerIsAlive(lockPath: string): Promise { + try { + const owner = JSON.parse(await fs.readFile(path.join(lockPath, "owner.json"), "utf8")) as { + pid?: unknown; + }; + if (!Number.isInteger(owner.pid) || (owner.pid as number) <= 0) return undefined; + try { + process.kill(owner.pid as number, 0); + return true; + } catch (error) { + if (isNodeError(error, "ESRCH")) return false; + return true; + } + } catch { + return undefined; + } +} + +async function removeOrphanedLock(lockPath: string): Promise { + const ownerAlive = await ownerIsAlive(lockPath); + if (ownerAlive === true) return false; + if (ownerAlive === undefined) { + const stat = await fs.stat(lockPath).catch(() => null); + if (!stat || Date.now() - stat.mtimeMs < ORPHANED_LOCK_AGE_MS) return false; + } + await fs.rm(lockPath, { recursive: true, force: true }); + return true; +} + +async function acquireFileLock(filePath: string): Promise { + const lockPath = path.join(path.dirname(filePath), `.${path.basename(filePath)}.askr-lock`); + const deadline = Date.now() + LOCK_TIMEOUT_MS; + while (true) { + try { + await fs.mkdir(lockPath); + await fs.writeFile( + path.join(lockPath, "owner.json"), + `${JSON.stringify({ pid: process.pid })}\n`, + { flag: "wx" }, + ); + return { lockPath }; + } catch (error) { + if (!isNodeError(error, "EEXIST")) { + await fs.rm(lockPath, { recursive: true, force: true }).catch(() => undefined); + throw error; + } + if (await removeOrphanedLock(lockPath)) continue; + if (Date.now() >= deadline) { + throw new Error(`Timed out waiting for file transaction lock: ${filePath}`); + } + await delay(LOCK_RETRY_MS); + } + } +} + +async function releaseFileLocks(locks: readonly FileLock[]): Promise { + await Promise.all( + [...locks].reverse().map((lock) => fs.rm(lock.lockPath, { recursive: true, force: true })), + ); +} + +async function readCurrentContent(filePath: string): Promise { + try { + return await fs.readFile(filePath, "utf8"); + } catch (error) { + if (isNodeError(error, "ENOENT")) return null; + throw error; + } +} + +function hasExpectedContent( + change: FileChange, +): change is FileChange & { readonly expectedContent: string | null } { + return Object.prototype.hasOwnProperty.call(change, "expectedContent"); +} + async function remove(paths: readonly string[]): Promise { await Promise.all( paths.map((filePath) => fs.rm(filePath, { force: true }).catch(() => undefined)), @@ -53,6 +147,28 @@ export async function writeFileChanges( throw new Error("File changes contain duplicate target paths."); } const replace = options.replace ?? fs.rename; + const guarded = ordered.filter(hasExpectedContent); + const locks: FileLock[] = []; + try { + for (const change of guarded) { + await fs.mkdir(path.dirname(change.filePath), { recursive: true }); + locks.push(await acquireFileLock(change.filePath)); + } + for (const change of guarded) { + if ((await readCurrentContent(change.filePath)) !== change.expectedContent) { + throw new Error(`File changed before writing: ${change.filePath}`); + } + } + await writeStagedChanges(ordered, replace); + } finally { + await releaseFileLocks(locks); + } +} + +async function writeStagedChanges( + ordered: readonly FileChange[], + replace: (temporaryPath: string, filePath: string) => Promise, +): Promise { const staged: StagedChange[] = []; try { for (const change of ordered) { diff --git a/tests/cli-smoke.test.ts b/tests/cli-smoke.test.ts index b793cb8..bf05ed6 100644 --- a/tests/cli-smoke.test.ts +++ b/tests/cli-smoke.test.ts @@ -752,15 +752,22 @@ test("should ensure concurrent add page commands cannot silently lose a registra ]); expect([...results].sort()).toEqual([0, 1]); - const failed = results[0] === 1 ? { name: "alpha", errors: alpha.errors } : { name: "beta", errors: beta.errors }; + const failed = + results[0] === 1 + ? { name: "alpha", errors: alpha.errors } + : { name: "beta", errors: beta.errors }; const succeeded = results[0] === 0 ? "alpha" : "beta"; expect(failed.errors.join("\n")).toContain("File changed before writing"); const routes = await fs.readFile(path.join(appRoot, "src/pages/app/_routes.tsx"), "utf8"); expect(routes).toContain(`route('/app/${succeeded}',`); expect(routes).not.toContain(`route('/app/${failed.name}',`); - await expect(fs.access(path.join(appRoot, `src/pages/app/${succeeded}.tsx`))).resolves.toBeUndefined(); - await expect(fs.access(path.join(appRoot, `src/pages/app/${failed.name}.tsx`))).rejects.toMatchObject({ code: "ENOENT" }); + await expect( + fs.access(path.join(appRoot, `src/pages/app/${succeeded}.tsx`)), + ).resolves.toBeUndefined(); + await expect( + fs.access(path.join(appRoot, `src/pages/app/${failed.name}.tsx`)), + ).rejects.toMatchObject({ code: "ENOENT" }); } finally { process.chdir(previousCwd); await fs.rm(tempRoot, { recursive: true, force: true }); @@ -810,15 +817,21 @@ test("should ensure add database rejects a package manifest changed after planni `${JSON.stringify({ name: "database-app", type: "module" }, null, 2)}\n`, ); const { io, errors } = createIo(); - const code = await runAddCli(["database", "sqlite", "--cwd", tempRoot], io, async (changes, options) => { - await fs.writeFile(manifestFile, externalManifest); - await writeFileChanges(changes, options); - }); + const code = await runAddCli( + ["database", "sqlite", "--cwd", tempRoot], + io, + async (changes, options) => { + await fs.writeFile(manifestFile, externalManifest); + await writeFileChanges(changes, options); + }, + ); expect(code).toBe(1); expect(errors.join("\n")).toContain("File changed before writing"); expect(await fs.readFile(manifestFile, "utf8")).toBe(externalManifest); - await expect(fs.access(path.join(tempRoot, "src/database/index.ts"))).rejects.toMatchObject({ code: "ENOENT" }); + await expect(fs.access(path.join(tempRoot, "src/database/index.ts"))).rejects.toMatchObject({ + code: "ENOENT", + }); } finally { await fs.rm(tempRoot, { recursive: true, force: true }); } @@ -980,7 +993,10 @@ test("should ensure concurrent add action commands cannot silently lose a regist try { process.chdir(tempRoot); expect( - await runCreateCli(["full-stack", "sample-full-stack", "--no-install", "--no-skills"], createIo().io), + await runCreateCli( + ["full-stack", "sample-full-stack", "--no-install", "--no-skills"], + createIo().io, + ), ).toBe(0); const appRoot = path.join(tempRoot, "sample-full-stack"); const archive = createIo(); @@ -988,27 +1004,44 @@ test("should ensure concurrent add action commands cannot silently lose a regist const writer = createConcurrentWriter(2); const results = await Promise.all([ - runAddCli(["action", "archive-project", "--route", "/", "--cwd", appRoot], archive.io, writer), - runAddCli(["action", "publish-project", "--route", "/", "--cwd", appRoot], publish.io, writer), + runAddCli( + ["action", "archive-project", "--route", "/", "--cwd", appRoot], + archive.io, + writer, + ), + runAddCli( + ["action", "publish-project", "--route", "/", "--cwd", appRoot], + publish.io, + writer, + ), ]); expect([...results].sort()).toEqual([0, 1]); - const failed = results[0] === 1 - ? { slug: "archive-project", identifier: "archiveProject", errors: archive.errors } - : { slug: "publish-project", identifier: "publishProject", errors: publish.errors }; - const succeeded = results[0] === 0 - ? { slug: "archive-project", identifier: "archiveProject" } - : { slug: "publish-project", identifier: "publishProject" }; + const failed = + results[0] === 1 + ? { slug: "archive-project", identifier: "archiveProject", errors: archive.errors } + : { slug: "publish-project", identifier: "publishProject", errors: publish.errors }; + const succeeded = + results[0] === 0 + ? { slug: "archive-project", identifier: "archiveProject" } + : { slug: "publish-project", identifier: "publishProject" }; expect(failed.errors.join("\n")).toContain("File changed before writing"); const registry = await fs.readFile(path.join(appRoot, "src/server/action-registry.ts"), "utf8"); - const authorizations = await fs.readFile(path.join(appRoot, "src/action-authorizations.ts"), "utf8"); + const authorizations = await fs.readFile( + path.join(appRoot, "src/action-authorizations.ts"), + "utf8", + ); expect(registry).toContain(`${succeeded.identifier}Action`); expect(registry).not.toContain(`${failed.identifier}Action`); expect(authorizations).toContain(`${succeeded.identifier}Action`); expect(authorizations).not.toContain(`${failed.identifier}Action`); - await expect(fs.access(path.join(appRoot, `src/actions/${succeeded.slug}.ts`))).resolves.toBeUndefined(); - await expect(fs.access(path.join(appRoot, `src/actions/${failed.slug}.ts`))).rejects.toMatchObject({ code: "ENOENT" }); + await expect( + fs.access(path.join(appRoot, `src/actions/${succeeded.slug}.ts`)), + ).resolves.toBeUndefined(); + await expect( + fs.access(path.join(appRoot, `src/actions/${failed.slug}.ts`)), + ).rejects.toMatchObject({ code: "ENOENT" }); } finally { process.chdir(previousCwd); await fs.rm(tempRoot, { recursive: true, force: true }); diff --git a/tests/file-changes.test.ts b/tests/file-changes.test.ts index e1ab138..7b21cfd 100644 --- a/tests/file-changes.test.ts +++ b/tests/file-changes.test.ts @@ -31,6 +31,9 @@ describe("writeFileChanges", () => { expect(await fs.readFile(shared, "utf8")).toBe("changed by another process\n"); await expect(fs.stat(created)).rejects.toMatchObject({ code: "ENOENT" }); + await expect(fs.stat(path.join(root, ".shared.ts.askr-lock"))).rejects.toMatchObject({ + code: "ENOENT", + }); }); it("should restore replaced files and remove created files after a replacement failure", async () => { From 8730341ba04b0d2e386c1a15c70ebbc3eee170cb Mon Sep 17 00:00:00 2001 From: Jeff Repanich Date: Sat, 15 Aug 2026 14:55:19 -0400 Subject: [PATCH 3/4] test: guard action planning snapshot --- src/bin/add.ts | 8 +++--- tests/cli-smoke.test.ts | 55 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/src/bin/add.ts b/src/bin/add.ts index 76aebc4..05e594b 100644 --- a/src/bin/add.ts +++ b/src/bin/add.ts @@ -583,14 +583,14 @@ async function addAction( routePath: parsed.routePath, slug, }); - const actions = await discoverDeclaredActions(projectRoot, { - filePath: descriptorFile, - content: descriptor, - }); const [registryContent, authorizationContent] = await Promise.all([ fs.readFile(registryFile, "utf8"), fs.readFile(authorizationFile, "utf8"), ]); + const actions = await discoverDeclaredActions(projectRoot, { + filePath: descriptorFile, + content: descriptor, + }); await writeChanges([ { filePath: descriptorFile, content: descriptor }, { diff --git a/tests/cli-smoke.test.ts b/tests/cli-smoke.test.ts index bf05ed6..633aa34 100644 --- a/tests/cli-smoke.test.ts +++ b/tests/cli-smoke.test.ts @@ -4,7 +4,7 @@ import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { promisify } from "node:util"; -import { expect, test } from "vitest"; +import { expect, test, vi } from "vitest"; import { runAddCli } from "../src/bin/add"; import { runCli } from "../src/bin/cli"; import { runCreateCli } from "../src/bin/create"; @@ -1048,6 +1048,59 @@ test("should ensure concurrent add action commands cannot silently lose a regist } }); +test("should ensure add action snapshots registries before discovering descriptors", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "askr-cli-add-action-snapshot-")); + const previousCwd = process.cwd(); + try { + process.chdir(tempRoot); + expect( + await runCreateCli( + ["full-stack", "sample-full-stack", "--no-install", "--no-skills"], + createIo().io, + ), + ).toBe(0); + const appRoot = path.join(tempRoot, "sample-full-stack"); + const actionsDir = path.join(appRoot, "src/actions"); + const registryFile = path.join(appRoot, "src/server/action-registry.ts"); + const authorizationFile = path.join(appRoot, "src/action-authorizations.ts"); + const externalRegistry = "// changed registry\n"; + const externalAuthorizations = "// changed authorizations\n"; + const originalReaddir = fs.readdir.bind(fs); + const readdir = vi.spyOn(fs, "readdir").mockImplementation((async ( + ...args: Parameters + ) => { + const entries = await originalReaddir(...args); + if (path.resolve(String(args[0])) === actionsDir) { + await Promise.all([ + fs.writeFile(registryFile, externalRegistry), + fs.writeFile(authorizationFile, externalAuthorizations), + ]); + } + return entries; + }) as typeof fs.readdir); + const { io, errors } = createIo(); + + try { + expect( + await runAddCli(["action", "publish-project", "--route", "/", "--cwd", appRoot], io), + ).toBe(1); + } finally { + readdir.mockRestore(); + } + + expect(errors.join("\n")).toContain("File changed before writing"); + expect(await fs.readFile(registryFile, "utf8")).toBe(externalRegistry); + expect(await fs.readFile(authorizationFile, "utf8")).toBe(externalAuthorizations); + await expect(fs.access(path.join(actionsDir, "publish-project.ts"))).rejects.toMatchObject({ + code: "ENOENT", + }); + } finally { + process.chdir(previousCwd); + vi.restoreAllMocks(); + await fs.rm(tempRoot, { recursive: true, force: true }); + } +}); + test("should ensure runSsgCli prints help without requiring config", async () => { const { io, logs, errors } = createIo(); const code = await runSsgCli(["--help"], undefined, io); From 18458a024ec52f17750fe94c45d41d75f8b6a9c1 Mon Sep 17 00:00:00 2001 From: Jeff Repanich Date: Sat, 15 Aug 2026 14:55:19 -0400 Subject: [PATCH 4/4] chore: release CLI 0.0.25 --- CHANGELOG.md | 9 ++++++++- package-lock.json | 4 ++-- package.json | 2 +- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6e2e17..bc64da9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.0.25] - 2026-08-15 + +### Fixed + +- Reject stale concurrent `askr add` transactions before they can silently lose a shared route, action registry, authorization, environment, or package-manifest edit. + ## [0.0.24] - 2026-08-15 ### Added @@ -80,7 +86,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Make database tooling work consistently across supported operating systems. -[Unreleased]: https://github.com/askrjs/askr-cli/compare/v0.0.24...HEAD +[Unreleased]: https://github.com/askrjs/askr-cli/compare/v0.0.25...HEAD +[0.0.25]: https://github.com/askrjs/askr-cli/compare/v0.0.24...v0.0.25 [0.0.24]: https://github.com/askrjs/askr-cli/compare/v0.0.23...v0.0.24 [0.0.23]: https://github.com/askrjs/askr-cli/compare/v0.0.22...v0.0.23 [0.0.22]: https://github.com/askrjs/askr-cli/compare/v0.0.21...v0.0.22 diff --git a/package-lock.json b/package-lock.json index 1ceb63b..39928bf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@askrjs/cli", - "version": "0.0.24", + "version": "0.0.25", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@askrjs/cli", - "version": "0.0.24", + "version": "0.0.25", "license": "Apache-2.0", "dependencies": { "@npmcli/config": "^11.0.1", diff --git a/package.json b/package.json index 6bfbe8c..d4e8a0e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@askrjs/cli", - "version": "0.0.24", + "version": "0.0.25", "description": "Unified CLI for the Askr platform", "homepage": "https://github.com/askrjs/askr-cli#readme", "bugs": {