From 3d09cccf4840057e863473190311dd05184c8213 Mon Sep 17 00:00:00 2001 From: Shu Chen <338845574+shu-chen-dot-ca@users.noreply.github.com> Date: Wed, 11 Nov 2026 15:36:52 -0500 Subject: [PATCH] test(config): accept a refused uninstall in the concurrent ownership race When the racing registrations keep the ownership lock busy, uninstall returns refused before it writes its tombstone, and the registrations then succeed. The race test still expected every recorded file to be removed. It now checks the refused outcome as its own case (no tombstone, every registration recorded, a rerun removes the home), and a new test forces that path by running uninstall while the lock is held. Closes #8379 --- ...config-ownership-concurrent-record.test.ts | 76 ++++++++++++++----- 1 file changed, 58 insertions(+), 18 deletions(-) diff --git a/tests/config/config-ownership-concurrent-record.test.ts b/tests/config/config-ownership-concurrent-record.test.ts index e5658c26268..bf53acc342d 100644 --- a/tests/config/config-ownership-concurrent-record.test.ts +++ b/tests/config/config-ownership-concurrent-record.test.ts @@ -11,6 +11,7 @@ import { recordOwnedConfigPath, removeOwnedConfigState, setUninstallStepHookForTests, + type ConfigRemovalResult, } from "../../src/lib/config-ownership"; import { repoPath } from "../helpers/repo-root"; import { removeTreeWithRetry } from "../helpers/remove-tree"; @@ -157,6 +158,46 @@ function uninstallElsewhere(dir: string): string { return Bun.spawnSync([process.execPath, "-e", script]).stdout.toString().trim(); } +/** + * What uninstall racing gated registrations must leave. A run that got the lock tombstoned the + * manifest first, so each child either recorded before the snapshot (its file was removed) or was + * refused and reported. A run refused because the children kept the lock busy wrote nothing: every + * child records, every file stays listed, and a rerun removes the whole home (#8379). + */ +function expectRaceOutcome(dir: string, names: string[], codes: number[], result: ConfigRemovalResult) { + if (result.status === "refused") { + expect(result.residualPaths).toEqual([dir]); + expect(manifest(dir).uninstallStartedAt).toBeUndefined(); + expect(codes).toEqual(names.map(() => 0)); + expect(manifest(dir).paths).toEqual(expect.arrayContaining(names)); + for (const name of names) expect(existsSync(join(dir, name))).toBe(true); + expect(removeOwnedConfigState(dir)).toEqual({ status: "removed", residualPaths: [] }); + expect(existsSync(dir)).toBe(false); + return; + } + expect(codes.every(code => code === 0 || code === 2 || code === 3)).toBe(true); + names.forEach((name, i) => { + const path = join(dir, name); + if (codes[i] === 0) expect(existsSync(path)).toBe(false); + else expect(result.residualPaths).toContain(path); + }); + expect(existsSync(join(dir, CONFIG_OWNERSHIP_LOCK))).toBe(false); +} + +/** Six gated children that have created their files and wait for `go` before recording them. */ +async function spawnGatedChildren(dir: string) { + expect(recordOwnedConfigPath(dir, join(dir, "seed"))).toBe(true); + const names = Array.from({ length: 6 }, (_, i) => "racing-" + i); + const signals = dirname(dir); + const gates = names.map(name => ({ ready: join(signals, name + ".ready"), go: join(signals, "go") })); + const children = names.map((name, i) => spawnChild(dir, name, gates[i])); + // Barrier: every child has created its file before uninstall starts. + const deadline = Date.now() + BARRIER_MS; + while (Date.now() < deadline && !gates.every(gate => existsSync(gate.ready))) await Bun.sleep(10); + expect(gates.every(gate => existsSync(gate.ready))).toBe(true); + return { names, children, go: gates[0]!.go }; +} + function withHome(fn: (dir: string) => void | Promise) { return async () => { const parent = mkdtempSync(join(tmpdir(), "ocx-ownership-record-")); @@ -186,30 +227,29 @@ describe("serialized ownership manifest (#8093)", () => { })); test("a registration racing uninstall is either removed or refused, never left unreported", withHome(async dir => { - expect(recordOwnedConfigPath(dir, join(dir, "seed"))).toBe(true); - const names = Array.from({ length: 6 }, (_, i) => "racing-" + i); - const signals = dirname(dir); - const gates = names.map(name => ({ ready: join(signals, name + ".ready"), go: join(signals, "go") })); - const children = names.map((name, i) => spawnChild(dir, name, gates[i])); - // Barrier: every child has created its file before uninstall starts. - const deadline = Date.now() + BARRIER_MS; - while (Date.now() < deadline && !gates.every(gate => existsSync(gate.ready))) await Bun.sleep(10); - expect(gates.every(gate => existsSync(gate.ready))).toBe(true); + const { names, children, go } = await spawnGatedChildren(dir); // Open the gate while holding the lock, so the children contend with uninstall for it. const release = holdLock(dir); try { - writeFileSync(gates[0]!.go, ""); + writeFileSync(go, ""); await Bun.sleep(50); } finally { release(); } + // The children may still hold the lock here, so uninstall may be refused as busy (#8379). const result = removeOwnedConfigState(dir); - const codes = await Promise.all(children.map(child => child.exited)); - expect(codes.every(code => code === 0 || code === 2 || code === 3)).toBe(true); - names.forEach((name, i) => { - const path = join(dir, name); - if (codes[i] === 0) expect(existsSync(path)).toBe(false); - else expect(result.residualPaths).toContain(path); - }); - expect(existsSync(join(dir, CONFIG_OWNERSHIP_LOCK))).toBe(false); + expectRaceOutcome(dir, names, await Promise.all(children.map(child => child.exited)), result); + }), 60_000); + + test("an uninstall refused for a busy lock leaves racing registrations recorded for a rerun", withHome(async dir => { + const { names, children, go } = await spawnGatedChildren(dir); + // The timing CI hit by chance in #8379, made certain: uninstall runs while the lock is busy. + const release = holdLock(dir); + let result: ConfigRemovalResult; + try { + writeFileSync(go, ""); + result = removeOwnedConfigState(dir); + } finally { release(); } + expect(result.status).toBe("refused"); + expectRaceOutcome(dir, names, await Promise.all(children.map(child => child.exited)), result); }), 60_000); test.skipIf(process.platform === "win32" || process.getuid?.() === 0)("an interrupted uninstall refuses registrations, cached paths included, and a rerun finishes it",