fix(remediation-audit/director): locked approval requests (Refs #219)

packages/core/src/ops/tools.ts

@@ -16,7 +16,7 @@ import { formatReport, runGates } from "../gates/engine.ts"
 import { factoryLayout } from "../layout.ts"
 import type { GateFinding } from "../schema/gates.ts"
 import { relativeTo } from "../trust/paths.ts"
-import { writeJson } from "../util/fs.ts"
+import { withLock, writeJson } from "../util/fs.ts"
 
 export type EsToolContext = { readonly agent: string; readonly sessionID?: string; readonly signal?: AbortSignal }
 export interface EsToolDef {
@@ -121,9 +121,14 @@ export function esTools(ops: OpsContext): EsToolDef[] {
             `Not ready for approval: ${error.message}${missing ? " Run /grill (or `es factory begin` + grill flow) to record the frontier, then request approval again." : ""}`,
           )
         })
-        const pending = (await pendingApprovals(root)).filter((item) => item.stage !== stage)
-        pending.push({ stage, requestedBy: context.agent, at: new Date().toISOString(), ...(note ? { note } : {}) })
-        await writeJson(factoryLayout(root).pending, pending)
+        const pendingFile = factoryLayout(root).pending
+        // Locked against concurrent requests and the human approve path
+        // (which clears under the same state lock): #219 lost-update.
+        await withLock(factoryLayout(root).state, async () => {
+          const pending = (await pendingApprovals(root)).filter((item) => item.stage !== stage)
+          pending.push({ stage, requestedBy: context.agent, at: new Date().toISOString(), ...(note ? { note } : {}) })
+          await writeJson(pendingFile, pending)
+        })
         await appendAuditEntry(root, {
           actor: `agent:${context.agent}`,
           action: "approval.request",

packages/core/test/pending-approvals.test.ts

@@ -0,0 +1,38 @@
+// Concurrent approval requests must not lost-update pending-approvals.json (#219).
+import { describe, expect, test } from "bun:test"
+import { Factory } from "../src/factory/index.ts"
+import { pendingApprovals } from "../src/factory/pending.ts"
+import { esTools } from "../src/ops/tools.ts"
+import { type Fixture, frontier, gitRepo, writeSpecs } from "./helpers.ts"
+
+describe("pending approvals under concurrency (#219)", () => {
+  test("two concurrent approval requests for different stages keep both entries", async () => {
+    const fx: Fixture = await gitRepo("es-pending-")
+    try {
+      const factory = new Factory(fx.root, {
+        gates: async () => ({ passed: true, findings: [], summary: "green" }),
+        stateDir: fx.state,
+      })
+      await factory.begin("human:tester")
+      await factory.writeFrontier("grill", frontier())
+      await writeSpecs(fx.root, ["alpha"])
+      const tools = esTools({ root: fx.root, factory, stateDir: fx.state })
+      const request = tools.find((tool) => tool.name === "es_request_approval")!
+      // Repeat the race several times: without a lock, the read-modify-write
+      // interleaves and one stage's entry is lost.
+      for (let round = 0; round < 8; round++) {
+        const { writeJson } = await import("../src/util/fs.ts")
+        const { factoryLayout } = await import("../src/layout.ts")
+        await writeJson(factoryLayout(fx.root).pending, [])
+        await Promise.all([
+          request.execute({ stage: "frontier", note: `r${round}` }, { agent: "factory" }),
+          request.execute({ stage: "spec", note: `r${round}` }, { agent: "factory" }),
+        ])
+        const pending = await pendingApprovals(fx.root)
+        expect(pending.map((item) => item.stage).sort()).toEqual(["frontier", "spec"])
+      }
+    } finally {
+      await fx.cleanup()
+    }
+  })
+})