diff --git a/docs/superpowers/plans/2026-08-14-cl10-final-review-closure.md b/docs/superpowers/plans/2026-08-14-cl10-final-review-closure.md index 114f448c12..b5eb331317 100644 --- a/docs/superpowers/plans/2026-08-14-cl10-final-review-closure.md +++ b/docs/superpowers/plans/2026-08-14-cl10-final-review-closure.md @@ -22,10 +22,19 @@ A direct same-publisher bundle revocation whose target bundle is absent is norma The closure is protected by focused tests that require: -- live lock contention to return `community_cache_busy` in under 500 ms; +- live lock contention to throw `community_cache_busy` synchronously without running protected work; +- one signal-zero owner-liveness check on that refusal, rejecting repeated live-owner polling; - the management community endpoint to return `503` plus `Retry-After: 1` for that contention; +- both rejection paths to preserve the existing owner bytes and lock directory identity; - oversized locally-originated community copies to be removed during sensitive purge; - hardlinked locally-originated cache pathnames to be removed while a peer hardlink survives; and - missing direct revocation bundle targets to return stable `revocation_target` errors. +The contention tests originally required completion in under 500 ms. That wall-clock criterion +included filesystem and management-route work and could fail under shared CI load before checking +the actual response contract. Verification now checks synchronous refusal, one owner-liveness probe, +and ownership preservation under the normal test deadline. The probe count detects repeated owner +checks, but does not promise to detect an unrelated one-off delay. This changes the test oracle, not the fail-fast/no-polling runtime +contract above, and does not establish a new response-time SLA. + Exact-head GitHub Actions success is required before this closure is considered verified. PR #1510 must remain open and unmerged during this review cycle. diff --git a/tests/lab/lab-community-mutation-lock.test.ts b/tests/lab/lab-community-mutation-lock.test.ts index ced17392a0..7c3132d155 100644 --- a/tests/lab/lab-community-mutation-lock.test.ts +++ b/tests/lab/lab-community-mutation-lock.test.ts @@ -1,5 +1,5 @@ -import { afterEach, describe, expect, test } from "bun:test"; -import { existsSync, mkdirSync, mkdtempSync, utimesSync, writeFileSync } from "node:fs"; +import { afterEach, describe, expect, spyOn, test } from "bun:test"; +import { existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { ensureLabDirs, labCommunityDir } from "../../src/lab/paths"; @@ -7,6 +7,7 @@ import { listCommunityEvidence } from "../../src/lab/public/community"; import { publicEvidenceMutationLockIsReclaimableForTests, publicEvidenceTryReclaimMutationLockForTests, + withPublicEvidenceMutationLock, } from "../../src/lab/public/mutation-lock"; import { PublicEvidenceValidationError } from "../../src/lab/public/validate"; import { removeTreeWithRetry } from "../helpers/remove-tree"; @@ -69,23 +70,35 @@ describe("community mutation lock", () => { expect(existsSync(lockPath)).toBe(true); }); - test("fails fast when a live owner holds the mutation lock", () => { + test("rejects a live owner after one check without running protected work or changing ownership", () => { const config = configDir(); const lockPath = createLiveOwnerLock(config); - const startedAt = performance.now(); + const lockBefore = lstatSync(lockPath); + const ownerBefore = readFileSync(join(lockPath, "owner.json")); + let ranProtectedWork = false; let failure: unknown; + // Observe the real signal-zero owner check. A retry loop must not poll a + // live owner before eventually returning the same refusal. + const ownerCheck = spyOn(process, "kill"); try { - listCommunityEvidence(config); - } catch (error) { - failure = error; + try { + withPublicEvidenceMutationLock(config, () => { ranProtectedWork = true; }); + } catch (error) { + failure = error; + } + + expect(failure).toBeInstanceOf(PublicEvidenceValidationError); + expect((failure as PublicEvidenceValidationError).code).toBe("community_cache_busy"); + expect(ranProtectedWork).toBe(false); + expect(readFileSync(join(lockPath, "owner.json"))).toEqual(ownerBefore); + const lockAfter = lstatSync(lockPath); + expect([lockAfter.dev, lockAfter.ino]).toEqual([lockBefore.dev, lockBefore.ino]); + expect(ownerCheck).toHaveBeenCalledTimes(1); + expect(ownerCheck).toHaveBeenCalledWith(process.pid, 0); + } finally { + ownerCheck.mockRestore(); } - - const elapsedMs = performance.now() - startedAt; - expect(failure).toBeInstanceOf(PublicEvidenceValidationError); - expect((failure as PublicEvidenceValidationError).code).toBe("community_cache_busy"); - expect(elapsedMs).toBeLessThan(500); - expect(existsSync(lockPath)).toBe(true); }); test("a competing reclaim claim prevents a second stale reclaimer from deleting the lock", () => { diff --git a/tests/lab/lab-public-surfaces.test.ts b/tests/lab/lab-public-surfaces.test.ts index e28d601a2c..bf798e2992 100644 --- a/tests/lab/lab-public-surfaces.test.ts +++ b/tests/lab/lab-public-surfaces.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, test } from "bun:test"; -import { existsSync, mkdirSync, readFileSync, readdirSync, writeFileSync } from "node:fs"; +import { existsSync, lstatSync, mkdirSync, readFileSync, readdirSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { handleLabCommand } from "../../src/cli/lab"; @@ -286,7 +286,7 @@ describe("CL-10 management local public evidence", () => { } }); - test("busy community lock is a prompt retryable service response", async () => { + test("busy community lock returns a retryable service response and preserves ownership", async () => { const home = tempHome(); const lockPath = join(labCommunityDir(home), ".mutation-lock"); mkdirSync(lockPath, { recursive: true, mode: 0o700 }); @@ -300,16 +300,18 @@ describe("CL-10 management local public evidence", () => { { encoding: "utf8", mode: 0o600 }, ); - const startedAt = performance.now(); + const lockBefore = lstatSync(lockPath); + const ownerBefore = readFileSync(join(lockPath, "owner.json")); const response = await api(home, "/api/lab/public/community"); - const elapsedMs = performance.now() - startedAt; - expect(elapsedMs).toBeLessThan(500); expect(response.status).toBe(503); expect(response.headers.get("retry-after")).toBe("1"); expect(await response.json()).toMatchObject({ error: { code: "community_cache_busy" }, }); + expect(readFileSync(join(lockPath, "owner.json"))).toEqual(ownerBefore); + const lockAfter = lstatSync(lockPath); + expect([lockAfter.dev, lockAfter.ino]).toEqual([lockBefore.dev, lockBefore.ino]); }); test("does not expose a remote publish endpoint", async () => {