Skip to content

fix: publish and recover multiscan locks atomically #196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mldangelo-oai wants to merge 2 commits into
base: main
Choose a base branch
from
+477 −20
Draft

fix: publish and recover multiscan locks atomically #196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e816da
fix: publish multiscan locks atomically
GautamSharma99 Jul 29, 2026
8cdc1f6
fix: serialize multiscan lock recovery and preserve ownership
mldangelo-oai Aug 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 170 additions & 19 deletions sdk/typescript/src/multiscan.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { execFile as execFileCallback } from "node:child_process";
import { randomUUID } from "node:crypto";
import {
link,
lstat,
mkdir,
open,
Expand Down Expand Up @@ -28,6 +29,13 @@ const REQUIRED_ARTIFACTS = [
"coverage.json",
"report.md",
];
const MAX_LOCK_OWNER_BYTES = 4 * 1024;
const MAX_LOCK_OWNER_PID = 2_147_483_647;

interface MultiscanLockOwner {
pid: number;
token?: string;
}

interface MultiscanTask {
id: string;
Expand Down Expand Up @@ -267,30 +275,173 @@ async function appendReceipt(path: string, receipt: string): Promise<void> {

async function acquireLock(output: string): Promise<() => Promise<void>> {
const path = join(output, ".lock");
const ownerPath = join(path, "owner.json");
const recovery = join(output, ".lock.recovery");
const token = randomUUID();
const pending = join(output, `.lock.pending-${token}`);
const owner = `${JSON.stringify({ pid: process.pid, token })}\n`;
const file = await open(pending, "wx", 0o600);
try {
try {
await file.writeFile(owner, "utf8");
await file.sync();
} finally {
await file.close();
}

for (;;) {
try {
await link(pending, path);
const recovering = await readLockOwner(recovery);
if (
recovering !== null &&
recovering.token !== token &&
processIsRunning(recovering.pid)
) {
await releaseLock(path, token);
throw new Error("A multiscan supervisor is already running.");
}
return async () => await releaseLock(path, token);

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Revalidate ownership before accepting a published lock

During legacy-directory recovery, there is a gap between moving the directory at line 337 and installing the recoverer's lock at line 339. A contender can hard-link its pending file into .lock during that gap, but if the recovery marker is released between its lstat and readFile, readLockOwner(recovery) returns null and this return admits it even though the recoverer has already replaced its lock. Confirm that .lock still contains this contender's token before returning, otherwise both supervisors can run concurrently.

Useful? React with 👍 / 👎.

} catch (error) {
if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error;
}

const existing = await readLockOwner(path);
if (existing !== null && processIsRunning(existing.pid)) {
throw new Error("A multiscan supervisor is already running.");
}

try {
await link(pending, recovery);
} catch (error) {
if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error;
const recovering = await readLockOwner(recovery);
if (recovering !== null && processIsRunning(recovering.pid)) {
throw new Error("A multiscan supervisor is already running.");
}
await rm(recovery, { force: true });

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reclaim stale recovery markers without unlinking successors

When a crashed .lock.recovery exists and multiple supervisors restart concurrently, two contenders can both read the old owner; after one removes it and publishes its own recovery claim, the delayed contender can execute this unconditional removal against the new claim. Both can then enter recovery and replace .lock, allowing overlapping campaigns. Move the observed marker aside and verify its token before deleting it, as is done for lock release.

Useful? React with 👍 / 👎.

continue;
}

try {
const current = await readLockOwner(path);
if (current !== null && processIsRunning(current.pid)) {
throw new Error("A multiscan supervisor is already running.");
}
const metadata = await lstat(path).catch((error: unknown) => {
if ((error as NodeJS.ErrnoException).code === "ENOENT") return null;
throw error;
});
if (metadata === null) continue;
if (metadata.isDirectory()) {
const stale = join(output, `.lock.stale-${randomUUID()}`);
await rename(path, stale);
try {
await rename(pending, path);
} finally {
await rm(stale, { recursive: true, force: true });
}
} else {
await rename(pending, path);
}
return async () => await releaseLock(path, token);
} catch (error) {
if ((error as NodeJS.ErrnoException).code === "ENOENT") continue;
throw error;
} finally {
await releaseLock(recovery, token);
}
}
} finally {
await rm(pending, { force: true });
}
}

async function readLockOwner(path: string): Promise<MultiscanLockOwner | null> {
let metadata;
try {
await mkdir(path, { mode: 0o700 });
metadata = await lstat(path);
} catch (error) {
if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error;
const { pid } = JSON.parse(await readFile(ownerPath, "utf8")) as {
pid: number;
};
if ((error as NodeJS.ErrnoException).code === "ENOENT") return null;
throw error;
}
let ownerPath = path;
if (metadata.isDirectory()) {
ownerPath = join(path, "owner.json");
try {
process.kill(pid, 0);
throw new Error("A multiscan supervisor is already running.");
} catch (failure) {
if ((failure as NodeJS.ErrnoException).code !== "ESRCH") throw failure;
metadata = await lstat(ownerPath);
} catch (error) {
if ((error as NodeJS.ErrnoException).code === "ENOENT") return null;
throw error;
}
const stale = join(output, `.lock.stale-${randomUUID()}`);
await rename(path, stale);
await rm(stale, { recursive: true });
return await acquireLock(output);
}
await writeFile(ownerPath, `${JSON.stringify({ pid: process.pid })}\n`, {
flag: "wx",
mode: 0o600,
});
return async () => rm(path, { recursive: true });
if (
!metadata.isFile() ||
metadata.isSymbolicLink() ||
metadata.size > MAX_LOCK_OWNER_BYTES
) {
return null;
}
let value: unknown;
try {
value = JSON.parse(await readFile(ownerPath, "utf8"));
} catch (error) {
if (
error instanceof SyntaxError ||
(error as NodeJS.ErrnoException).code === "ENOENT"
) {
return null;
}
throw error;
}
if (
typeof value !== "object" ||
value === null ||
!("pid" in value) ||
!Number.isSafeInteger(value.pid) ||
(value.pid as number) <= 0 ||
(value.pid as number) > MAX_LOCK_OWNER_PID ||
("token" in value && typeof value.token !== "string")
) {
return null;
}
return {
pid: value.pid as number,
...("token" in value ? { token: value.token as string } : {}),
};
}

function processIsRunning(pid: number): boolean {
try {
process.kill(pid, 0);
return true;
} catch (error) {
const code = (error as NodeJS.ErrnoException).code;
if (code === "ESRCH") return false;
if (code === "EPERM") return true;
throw error;
}
}

async function releaseLock(path: string, token: string): Promise<void> {
const owner = await readLockOwner(path);
if (owner?.token !== token) return;

const released = `${path}.released-${randomUUID()}`;
try {
await rename(path, released);
} catch (error) {
if ((error as NodeJS.ErrnoException).code === "ENOENT") return;
throw error;
}

try {
const moved = await readLockOwner(released);
if (moved?.token !== token) {
await link(released, path);

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restore substituted legacy locks without hard-linking directories

If a legacy directory lock is substituted after the ownership check at line 426 but before the rename at line 431, this code moves that competing lock aside and then attempts to restore it with link. Hard-linking a directory fails on supported POSIX filesystems, and the non-recursive cleanup also cannot remove it, leaving the competitor's ownership stranded under .lock.released-* while .lock is vacant and allowing another supervisor to start.

Useful? React with 👍 / 👎.

}
} finally {
await rm(released, { force: true });
}
}

async function ensureManifest(
Expand Down
Loading
Loading