diff --git a/.changeset/proud-ants-study.md b/.changeset/proud-ants-study.md new file mode 100644 index 000000000..2a72c6298 --- /dev/null +++ b/.changeset/proud-ants-study.md @@ -0,0 +1,5 @@ +--- +"@sapiom/harness": patch +--- + +Give the workflow registry's atomic write a per-process temp file name, so two harness instances sharing the machine-wide workflows.json cannot publish a torn file. diff --git a/packages/harness/src/core/workflow-registry.test.ts b/packages/harness/src/core/workflow-registry.test.ts index 3609b23af..002d11e7f 100644 --- a/packages/harness/src/core/workflow-registry.test.ts +++ b/packages/harness/src/core/workflow-registry.test.ts @@ -351,9 +351,39 @@ describe("WorkflowRegistry", () => { expect(Array.isArray(parsed)).toBe(true); expect(parsed).toHaveLength(1); - // The .tmp file must NOT be left behind (rename completed). - const tmpPath = `${registryPath}.tmp`; - await expect(fs.access(tmpPath)).rejects.toThrow(); + // No temp artefact must be left behind (rename completed). The temp name + // carries a pid + uuid suffix, so match on the prefix rather than one + // fixed path — asserting a fixed name would pass even if a differently + // named temp file were orphaned. + const base = path.basename(registryPath); + const leftovers = (await fs.readdir(path.dirname(registryPath))).filter((name) => + name.startsWith(`${base}.tmp`), + ); + expect(leftovers).toEqual([]); + }); + + it("two registries sharing one path do not collide on the temp file (the path is machine-wide)", async () => { + // writeQueue only serializes writers inside one instance, but the default + // registry path is machine-wide (~/.sapiom/harness/workflows.json), so a + // CLI and a desktop app run two registries over the same file. With a + // fixed .tmp name each would write into the other's temp and rename a + // half-written file over the target, which load() then swallows as an + // empty list — silent loss of connected workflows. + const other = new WorkflowRegistry(registryPath); + await writeMarker(path.join(tmpRoot, "proj-a"), 1); + await writeMarker(path.join(tmpRoot, "proj-b"), 1); + + await Promise.all([registry.scan(tmpRoot), other.scan(tmpRoot)]); + + const parsed = JSON.parse(await fs.readFile(registryPath, "utf8")) as unknown[]; + expect(Array.isArray(parsed)).toBe(true); + expect(parsed).toHaveLength(2); + + const base = path.basename(registryPath); + const leftovers = (await fs.readdir(path.dirname(registryPath))).filter((name) => + name.startsWith(`${base}.tmp`), + ); + expect(leftovers).toEqual([]); }); it("a failed persist does not poison the queue — subsequent writes succeed", async () => { diff --git a/packages/harness/src/core/workflow-registry.ts b/packages/harness/src/core/workflow-registry.ts index 8474bd878..6bc6892a9 100644 --- a/packages/harness/src/core/workflow-registry.ts +++ b/packages/harness/src/core/workflow-registry.ts @@ -9,6 +9,7 @@ * shared app. */ +import { randomUUID } from "node:crypto"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; @@ -186,10 +187,18 @@ export class WorkflowRegistry { // Atomic write: write to a temp file in the same directory (so rename is // same-filesystem and thus atomic on POSIX), then rename over the target. // A crash mid-write leaves the .tmp file, not a torn workflows.json. - // Mirrors the pattern used by SessionManager.persist(). - const tmpPath = `${this.registryPath}.tmp`; - await fs.writeFile(tmpPath, JSON.stringify(this.workflows, null, 2)); - await fs.rename(tmpPath, this.registryPath); + // The temp name is per-process and unique: writeQueue only serializes + // writers inside one process, and this path is machine-wide, so a second + // harness would otherwise write into the same .tmp concurrently and the + // rename would publish a torn file. Mirrors SessionManager.persist(). + const tmpPath = `${this.registryPath}.tmp-${process.pid}-${randomUUID()}`; + try { + await fs.writeFile(tmpPath, JSON.stringify(this.workflows, null, 2)); + await fs.rename(tmpPath, this.registryPath); + } catch (err) { + await fs.rm(tmpPath, { force: true }).catch(() => {}); + throw err; + } } /** Chains `run` onto the write queue so concurrent mutations never