From f6b13c15f3bcc6e5aef000143e628b9a5908bd67 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:09:28 +0900 Subject: [PATCH 1/2] test(lab): verify busy-lock ownership without wall-clock timing --- .../2026-08-14-cl10-final-review-closure.md | 9 ++++++++- tests/lab/lab-community-mutation-lock.test.ts | 18 +++++++++++------- tests/lab/lab-public-surfaces.test.ts | 12 +++++++----- 3 files changed, 26 insertions(+), 13 deletions(-) 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..a281e0d3af 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,17 @@ 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; - 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 and ownership preservation +under the normal test deadline. 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..b5230846c7 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 { 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,26 @@ 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 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; try { - listCommunityEvidence(config); + withPublicEvidenceMutationLock(config, () => { ranProtectedWork = true; }); } catch (error) { failure = error; } - 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); + 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]); }); 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 () => { From 2a8ca25cba298936be3ec7d13edb69aab5657b2a Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Thu, 10 Sep 2026 13:56:31 +0900 Subject: [PATCH 2/2] test(lab): detect repeated live-owner probes without wall-clock timing --- .../2026-08-14-cl10-final-review-closure.md | 6 ++-- tests/lab/lab-community-mutation-lock.test.ts | 33 ++++++++++++------- 2 files changed, 25 insertions(+), 14 deletions(-) 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 a281e0d3af..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 @@ -23,6 +23,7 @@ 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 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; @@ -31,8 +32,9 @@ The closure is protected by focused tests that require: 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 and ownership preservation -under the normal test deadline. This changes the test oracle, not the fail-fast/no-polling runtime +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 b5230846c7..7c3132d155 100644 --- a/tests/lab/lab-community-mutation-lock.test.ts +++ b/tests/lab/lab-community-mutation-lock.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, test } from "bun:test"; +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"; @@ -70,7 +70,7 @@ describe("community mutation lock", () => { expect(existsSync(lockPath)).toBe(true); }); - test("rejects a live owner without running protected work or changing ownership", () => { + test("rejects a live owner after one check without running protected work or changing ownership", () => { const config = configDir(); const lockPath = createLiveOwnerLock(config); const lockBefore = lstatSync(lockPath); @@ -78,18 +78,27 @@ describe("community mutation lock", () => { 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 { - withPublicEvidenceMutationLock(config, () => { ranProtectedWork = true; }); - } 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(); } - - 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]); }); test("a competing reclaim claim prevents a second stale reclaimer from deleting the lock", () => {