From 6ca0540dba93b0e0e9a5c47fb86cd6e48fec4ee9 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:24:19 -0400 Subject: [PATCH] fix(plan-canvas): inspect startup lock safely --- docs/testing/plan-canvas-pdf-export.tdd.md | 4 ++-- scripts/plan-canvas.js | 21 +++++++++++++++++---- tests/integration/plan-canvas-e2e.test.js | 12 ++++++++++++ 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/docs/testing/plan-canvas-pdf-export.tdd.md b/docs/testing/plan-canvas-pdf-export.tdd.md index 18ac1c295..e2bc2ca06 100644 --- a/docs/testing/plan-canvas-pdf-export.tdd.md +++ b/docs/testing/plan-canvas-pdf-export.tdd.md @@ -25,8 +25,8 @@ artifact as a real PDF file without sending the plan to an external converter. - RED: the focused server suite produced 30 passes and 2 failures because the Canvas had no Download PDF control or PDF endpoint. - GREEN: renderer unit tests pass 7/7, Plan Canvas server tests pass 34/34, - and the end-to-end review workflow passes 11/11. -- FULL SUITE: the final review-hardened implementation passes all 4,007 + and the end-to-end review workflow passes 12/12. +- FULL SUITE: the final review-hardened implementation passes all 4,008 discovered tests; hosted security reruns are recorded on PR #2894. - COVERAGE: `npm run coverage` passes 4,003/4,003 with 88.97% statements, 80.58% branches, 94.22% functions, and 88.97% lines. The Plan Canvas diff --git a/scripts/plan-canvas.js b/scripts/plan-canvas.js index fddce1471..8c919c5af 100755 --- a/scripts/plan-canvas.js +++ b/scripts/plan-canvas.js @@ -187,12 +187,25 @@ function processIsAlive(pid) { } } +function readServerStartLock(lockPath) { + let fd; + try { + fd = fs.openSync(lockPath, 'r'); + const stat = fs.fstatSync(fd); + let owner = null; + try { owner = JSON.parse(fs.readFileSync(fd, 'utf8')); } catch { /* incomplete lock owner */ } + return { mtimeMs: stat.mtimeMs, owner }; + } finally { + if (fd !== undefined) { + try { fs.closeSync(fd); } catch { /* best-effort lock inspection */ } + } + } +} + function removeStaleServerStartLock(lockPath, staleAfterMs = 60 * 1000) { try { - const stat = fs.statSync(lockPath); - let owner = null; - try { owner = JSON.parse(fs.readFileSync(lockPath, 'utf8')); } catch { /* incomplete lock owner */ } - const oldEnough = Date.now() - stat.mtimeMs > staleAfterMs; + const { mtimeMs, owner } = readServerStartLock(lockPath); + const oldEnough = Date.now() - mtimeMs > staleAfterMs; if (!oldEnough && (!owner || processIsAlive(owner.pid))) return false; fs.rmSync(lockPath, { force: true }); return true; diff --git a/tests/integration/plan-canvas-e2e.test.js b/tests/integration/plan-canvas-e2e.test.js index 4a1566c89..9e1b2f8ff 100644 --- a/tests/integration/plan-canvas-e2e.test.js +++ b/tests/integration/plan-canvas-e2e.test.js @@ -158,6 +158,18 @@ async function main() { assert.ok(!fs.readdirSync(tmp).some(name => name.endsWith('.lock'))); }); + await test('port-scoped startup lock recovers a dead owner', async () => { + const lockPort = port + 3; + const userId = typeof process.getuid === 'function' ? process.getuid() : 'user'; + const lockPath = path.join(tmp, `ecc-plan-canvas-${userId}-${lockPort}.lock`); + fs.writeFileSync(lockPath, JSON.stringify({ pid: 2147483647, token: 'dead-owner' })); + assert.strictEqual( + await withServerStartLock(lockPort, async () => 'recovered', { lockDir: tmp, timeoutMs: 2000 }), + 'recovered' + ); + assert.ok(!fs.existsSync(lockPath)); + }); + await test('concurrent opens serialize replacement of a same-version legacy server', async () => { const legacyPort = port + 1; const legacyStateDir = path.join(tmp, 'legacy-state');