fix: release stranded bootstrap locks and handle re-entrant reacquire (#1352)
Release session locks on bootstrap abort paths and reset same-process lock state before re-acquiring so stale proper-lockfile callbacks cannot poison a fresh auto-mode session. Adds regression coverage for bootstrap cleanup and re-entrant lock acquisition.
This commit is contained in:
parent
eaf0538150
commit
f2657e1ba0
4 changed files with 134 additions and 10 deletions
|
|
@ -103,13 +103,20 @@ export async function bootstrapAutoSession(
|
|||
return false;
|
||||
}
|
||||
|
||||
// Ensure git repo exists
|
||||
if (!nativeIsRepo(base)) {
|
||||
const mainBranch = loadEffectiveGSDPreferences()?.preferences?.git?.main_branch || "main";
|
||||
nativeInit(base, mainBranch);
|
||||
function releaseLockAndReturn(): false {
|
||||
releaseSessionLock(base);
|
||||
clearLock(base);
|
||||
return false;
|
||||
}
|
||||
|
||||
// Ensure .gitignore has baseline patterns
|
||||
try {
|
||||
// Ensure git repo exists
|
||||
if (!nativeIsRepo(base)) {
|
||||
const mainBranch = loadEffectiveGSDPreferences()?.preferences?.git?.main_branch || "main";
|
||||
nativeInit(base, mainBranch);
|
||||
}
|
||||
|
||||
// Ensure .gitignore has baseline patterns
|
||||
const gitPrefs = loadEffectiveGSDPreferences()?.preferences?.git;
|
||||
const manageGitignore = gitPrefs?.manage_gitignore;
|
||||
ensureGitignore(base, { manageGitignore });
|
||||
|
|
@ -251,10 +258,10 @@ export async function bootstrapAutoSession(
|
|||
"Discussion completed but no milestone context was written. Run /gsd to try the discussion again, or /gsd auto after creating the milestone manually.",
|
||||
"warning",
|
||||
);
|
||||
return false;
|
||||
return releaseLockAndReturn();
|
||||
}
|
||||
} else {
|
||||
return false;
|
||||
return releaseLockAndReturn();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -276,7 +283,7 @@ export async function bootstrapAutoSession(
|
|||
"Discussion completed but milestone context is still missing. Run /gsd to try again.",
|
||||
"warning",
|
||||
);
|
||||
return false;
|
||||
return releaseLockAndReturn();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -286,7 +293,7 @@ export async function bootstrapAutoSession(
|
|||
if (!state.activeMilestone) {
|
||||
const { showSmartEntry } = await import("./guided-flow.js");
|
||||
await showSmartEntry(ctx, pi, base, { step: requestedStepMode });
|
||||
return false;
|
||||
return releaseLockAndReturn();
|
||||
}
|
||||
|
||||
// ── Initialize session state ──
|
||||
|
|
@ -479,5 +486,10 @@ export async function bootstrapAutoSession(
|
|||
}
|
||||
} catch { /* non-fatal */ }
|
||||
|
||||
return true;
|
||||
return true;
|
||||
} catch (err) {
|
||||
releaseSessionLock(base);
|
||||
clearLock(base);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -149,6 +149,16 @@ function ensureExitHandler(gsdDir: string): void {
|
|||
export function acquireSessionLock(basePath: string): SessionLockResult {
|
||||
const lp = lockPath(basePath);
|
||||
|
||||
// Re-entrant acquire on the same path: release our current OS lock first so
|
||||
// proper-lockfile clears its update timer before we acquire a fresh lock.
|
||||
if (_releaseFunction && _lockedPath === basePath) {
|
||||
try { _releaseFunction(); } catch { /* may already be released */ }
|
||||
_releaseFunction = null;
|
||||
_lockedPath = null;
|
||||
_lockPid = 0;
|
||||
_lockCompromised = false;
|
||||
}
|
||||
|
||||
// Ensure the directory exists
|
||||
mkdirSync(dirname(lp), { recursive: true });
|
||||
|
||||
|
|
@ -234,6 +244,7 @@ export function acquireSessionLock(basePath: string): SessionLockResult {
|
|||
_releaseFunction = release;
|
||||
_lockedPath = basePath;
|
||||
_lockPid = process.pid;
|
||||
_lockCompromised = false;
|
||||
|
||||
// Safety net — uses centralized handler to avoid double-registration
|
||||
ensureExitHandler(gsdDir);
|
||||
|
|
|
|||
|
|
@ -1,10 +1,25 @@
|
|||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdirSync, mkdtempSync, writeFileSync, existsSync, readFileSync, rmSync } from "node:fs";
|
||||
import { createRequire } from "node:module";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
|
||||
import { writeLock, readCrashLock, clearLock, isLockProcessAlive } from "../crash-recovery.ts";
|
||||
import { acquireSessionLock, releaseSessionLock } from "../session-lock.ts";
|
||||
|
||||
const require = createRequire(import.meta.url);
|
||||
|
||||
function hasProperLockfile(): boolean {
|
||||
try {
|
||||
require("proper-lockfile");
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
const properLockfileAvailable = hasProperLockfile();
|
||||
|
||||
// ─── writeLock creates auto.lock in .gsd/ ────────────────────────────────
|
||||
|
||||
|
|
@ -95,6 +110,28 @@ test("clearLock is safe when no lock file exists", () => {
|
|||
rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("bootstrap cleanup releases session lock artifacts", () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "gsd-lock-test-"));
|
||||
mkdirSync(join(dir, ".gsd"), { recursive: true });
|
||||
|
||||
try {
|
||||
const result = acquireSessionLock(dir);
|
||||
assert.equal(result.acquired, true, "session lock should be acquired");
|
||||
assert.ok(existsSync(join(dir, ".gsd", "auto.lock")), "auto.lock should exist while lock is held");
|
||||
if (properLockfileAvailable) {
|
||||
assert.ok(existsSync(join(dir, ".gsd.lock")), ".gsd.lock should exist while lock is held");
|
||||
}
|
||||
|
||||
releaseSessionLock(dir);
|
||||
clearLock(dir);
|
||||
|
||||
assert.ok(!existsSync(join(dir, ".gsd", "auto.lock")), "auto.lock should be removed by bootstrap cleanup");
|
||||
assert.ok(!existsSync(join(dir, ".gsd.lock")), ".gsd.lock should be removed by bootstrap cleanup");
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
// ─── isLockProcessAlive detects live vs dead PIDs ────────────────────────
|
||||
|
||||
test("isLockProcessAlive returns false for dead PID", () => {
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@
|
|||
*/
|
||||
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, rmSync, existsSync, readFileSync } from 'node:fs';
|
||||
import { createRequire } from 'node:module';
|
||||
import { join } from 'node:path';
|
||||
import { tmpdir } from 'node:os';
|
||||
|
||||
|
|
@ -26,6 +27,18 @@ import { gsdRoot } from '../paths.ts';
|
|||
import { createTestContext } from './test-helpers.ts';
|
||||
|
||||
const { assertEq, assertTrue, report } = createTestContext();
|
||||
const require = createRequire(import.meta.url);
|
||||
|
||||
function hasProperLockfile(): boolean {
|
||||
try {
|
||||
require("proper-lockfile");
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
const properLockfileAvailable = hasProperLockfile();
|
||||
|
||||
async function main(): Promise<void> {
|
||||
|
||||
|
|
@ -207,6 +220,57 @@ async function main(): Promise<void> {
|
|||
}
|
||||
}
|
||||
|
||||
// ─── 9. Re-entrant acquisition without explicit release ───────────────
|
||||
console.log('\n=== 9. re-entrant acquire without explicit release ===');
|
||||
{
|
||||
const base = mkdtempSync(join(tmpdir(), 'gsd-session-lock-'));
|
||||
mkdirSync(join(base, '.gsd'), { recursive: true });
|
||||
|
||||
try {
|
||||
const r1 = acquireSessionLock(base);
|
||||
assertTrue(r1.acquired, 'first acquisition succeeds');
|
||||
|
||||
const r2 = acquireSessionLock(base);
|
||||
assertTrue(r2.acquired, 're-entrant acquisition succeeds');
|
||||
|
||||
const valid = validateSessionLock(base);
|
||||
assertTrue(valid, 're-entrant acquisition does not corrupt validation state');
|
||||
|
||||
releaseSessionLock(base);
|
||||
} finally {
|
||||
rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
// ─── 10. Re-entrant acquisition refreshes lock artifacts ──────────────
|
||||
console.log('\n=== 10. re-entrant acquire refreshes lock artifacts ===');
|
||||
{
|
||||
const base = mkdtempSync(join(tmpdir(), 'gsd-session-lock-'));
|
||||
mkdirSync(join(base, '.gsd'), { recursive: true });
|
||||
|
||||
try {
|
||||
const r1 = acquireSessionLock(base);
|
||||
assertTrue(r1.acquired, 'first acquisition succeeds');
|
||||
|
||||
const lockDir = gsdRoot(base) + '.lock';
|
||||
if (properLockfileAvailable) {
|
||||
assertTrue(existsSync(lockDir), '.gsd.lock/ exists after first acquisition');
|
||||
}
|
||||
|
||||
const r2 = acquireSessionLock(base);
|
||||
assertTrue(r2.acquired, 'second acquisition succeeds');
|
||||
if (properLockfileAvailable) {
|
||||
assertTrue(existsSync(lockDir), '.gsd.lock/ exists after re-entrant acquisition');
|
||||
}
|
||||
assertTrue(validateSessionLock(base), 'lock remains valid after re-entrant acquisition');
|
||||
|
||||
releaseSessionLock(base);
|
||||
assertTrue(!existsSync(lockDir), '.gsd.lock/ is removed after release');
|
||||
} finally {
|
||||
rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
report();
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue