mirror of
https://github.com/affaan-m/ECC.git
synced 2026-09-10 11:47:54 +02:00
fix(plan-canvas): preserve startup exclusion
This commit is contained in:
@@ -411,6 +411,16 @@ function createPlanCanvasServer({
|
||||
if (pdfMatch && (req.method === 'GET' || req.method === 'POST')) {
|
||||
const session = store.get(pdfMatch[1]);
|
||||
if (!session) return sendJson(res, 404, { error: 'unknown session' });
|
||||
const sendBusy = () => {
|
||||
res.setHeader('retry-after', '1');
|
||||
if (req.method === 'POST') res.setHeader('connection', 'close');
|
||||
return sendJson(res, 429, {
|
||||
error: 'another PDF export is already in progress',
|
||||
code: 'PDF_EXPORT_BUSY'
|
||||
});
|
||||
};
|
||||
// Reject overload before accepting a potentially slow snapshot body.
|
||||
if (pdfExportActive) return sendBusy();
|
||||
let requestedSnapshot = null;
|
||||
if (req.method === 'POST') {
|
||||
const body = await readJsonBody(req, MAX_PDF_SNAPSHOT_BYTES);
|
||||
@@ -419,13 +429,8 @@ function createPlanCanvasServer({
|
||||
}
|
||||
requestedSnapshot = { key: session.key, html: body.html };
|
||||
}
|
||||
if (pdfExportActive) {
|
||||
res.setHeader('retry-after', '1');
|
||||
return sendJson(res, 429, {
|
||||
error: 'another PDF export is already in progress',
|
||||
code: 'PDF_EXPORT_BUSY'
|
||||
});
|
||||
}
|
||||
// A renderer may have started while this request body was arriving.
|
||||
if (pdfExportActive) return sendBusy();
|
||||
pdfExportActive = true;
|
||||
try {
|
||||
pdfSnapshot = requestedSnapshot;
|
||||
|
||||
+62
-25
@@ -21,7 +21,7 @@ const fs = require('fs');
|
||||
const http = require('http');
|
||||
const os = require('os');
|
||||
const path = require('path');
|
||||
const { spawn } = require('child_process');
|
||||
const { execFileSync, spawn } = require('child_process');
|
||||
|
||||
const {
|
||||
canonicalizeArtifactPath,
|
||||
@@ -39,7 +39,6 @@ const {
|
||||
} = require('./lib/plan-canvas/server');
|
||||
|
||||
const VERSION = require('../package.json').version;
|
||||
const DEFAULT_START_LOCK_LEASE_MS = 5 * 1000;
|
||||
|
||||
const SAFE_REQUEST_PATHS = new Set([
|
||||
'/',
|
||||
@@ -183,6 +182,37 @@ function processIsAlive(pid) {
|
||||
}
|
||||
}
|
||||
|
||||
function readProcessIdentity(pid) {
|
||||
if (!Number.isInteger(pid) || pid <= 0) return null;
|
||||
try {
|
||||
if (process.platform === 'linux') {
|
||||
const stat = fs.readFileSync(`/proc/${pid}/stat`, 'utf8');
|
||||
const commandEnd = stat.lastIndexOf(')');
|
||||
if (commandEnd === -1) return null;
|
||||
const fieldsAfterCommand = stat.slice(commandEnd + 1).trim().split(/\s+/);
|
||||
const startTicks = fieldsAfterCommand[19];
|
||||
return startTicks ? `linux:${startTicks}` : null;
|
||||
}
|
||||
if (process.platform === 'win32') {
|
||||
const command = `(Get-Process -Id ${pid} -ErrorAction Stop).StartTime.ToUniversalTime().Ticks`;
|
||||
const startTicks = execFileSync(
|
||||
'powershell.exe',
|
||||
['-NoProfile', '-NonInteractive', '-Command', command],
|
||||
{ encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'], timeout: 3000, windowsHide: true }
|
||||
).trim();
|
||||
return startTicks ? `win32:${startTicks}` : null;
|
||||
}
|
||||
const startedAt = execFileSync(
|
||||
'ps',
|
||||
['-p', String(pid), '-o', 'lstart='],
|
||||
{ encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'], timeout: 1000 }
|
||||
).trim();
|
||||
return startedAt ? `${process.platform}:${startedAt}` : null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function readServerStartTicket(file) {
|
||||
let fd;
|
||||
try {
|
||||
@@ -201,9 +231,10 @@ function readServerStartTicket(file) {
|
||||
}
|
||||
}
|
||||
|
||||
function listServerStartTickets(lockDir, port, ownToken, staleAfterMs = DEFAULT_START_LOCK_LEASE_MS) {
|
||||
function listServerStartTickets(lockDir, port, ownToken, malformedStaleAfterMs = 60 * 1000) {
|
||||
const prefix = `ecc-plan-canvas-${validatePort(port)}-`;
|
||||
const entries = [];
|
||||
const identities = new Map();
|
||||
for (const name of fs.readdirSync(lockDir)) {
|
||||
if (!name.startsWith(prefix) || (!name.endsWith('.choosing') && !name.endsWith('.ticket'))) continue;
|
||||
const file = path.join(lockDir, name);
|
||||
@@ -215,18 +246,25 @@ function listServerStartTickets(lockDir, port, ownToken, staleAfterMs = DEFAULT_
|
||||
continue;
|
||||
}
|
||||
if (value.malformed) {
|
||||
if (Date.now() - value.mtimeMs > staleAfterMs) {
|
||||
if (Date.now() - value.mtimeMs > malformedStaleAfterMs) {
|
||||
try { fs.rmSync(file, { force: true }); } catch { /* already removed */ }
|
||||
}
|
||||
continue;
|
||||
}
|
||||
const stale = value.token !== ownToken && (
|
||||
!processIsAlive(value.pid) || Date.now() - value.mtimeMs > staleAfterMs
|
||||
);
|
||||
let stale = false;
|
||||
if (value.token !== ownToken) {
|
||||
if (!processIsAlive(value.pid)) {
|
||||
stale = true;
|
||||
} else if (typeof value.processIdentity === 'string') {
|
||||
if (!identities.has(value.pid)) identities.set(value.pid, readProcessIdentity(value.pid));
|
||||
const currentIdentity = identities.get(value.pid);
|
||||
stale = currentIdentity !== null && currentIdentity !== value.processIdentity;
|
||||
}
|
||||
}
|
||||
if (stale) {
|
||||
// A bounded, renewable lease prevents PID reuse from making an exited
|
||||
// owner's ticket look live forever. Ticket names contain a never-reused
|
||||
// owner token, so this cannot delete a later caller's ticket path.
|
||||
// Process start identity distinguishes an exited owner from an unrelated
|
||||
// process that later reused its PID. A live owner remains authoritative
|
||||
// even if its event loop is paused for an arbitrary amount of time.
|
||||
try { fs.rmSync(file, { force: true }); } catch { /* already removed */ }
|
||||
continue;
|
||||
}
|
||||
@@ -237,36 +275,36 @@ function listServerStartTickets(lockDir, port, ownToken, staleAfterMs = DEFAULT_
|
||||
|
||||
async function withServerStartLock(port, task, {
|
||||
timeoutMs = 15 * 1000,
|
||||
leaseMs = DEFAULT_START_LOCK_LEASE_MS,
|
||||
lockDir = path.join(os.homedir(), '.claude', 'plan-canvas', 'locks')
|
||||
} = {}) {
|
||||
const lockPort = validatePort(port);
|
||||
const processIdentity = readProcessIdentity(process.pid);
|
||||
if (!processIdentity) throw new Error('could not determine Plan Canvas startup lock process identity');
|
||||
const token = `${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}`;
|
||||
const prefix = `ecc-plan-canvas-${lockPort}-${token}`;
|
||||
const choosingFile = path.join(lockDir, `${prefix}.choosing`);
|
||||
const ticketFile = path.join(lockDir, `${prefix}.ticket`);
|
||||
const startedAt = Date.now();
|
||||
const lockLeaseMs = Number.isFinite(leaseMs) && leaseMs > 0
|
||||
? leaseMs
|
||||
: DEFAULT_START_LOCK_LEASE_MS;
|
||||
let leaseTimer = null;
|
||||
fs.mkdirSync(lockDir, { recursive: true, mode: 0o700 });
|
||||
fs.writeFileSync(choosingFile, JSON.stringify({ pid: process.pid, token }), { flag: 'wx', mode: 0o600 });
|
||||
fs.writeFileSync(
|
||||
choosingFile,
|
||||
JSON.stringify({ pid: process.pid, processIdentity, token }),
|
||||
{ flag: 'wx', mode: 0o600 }
|
||||
);
|
||||
|
||||
try {
|
||||
const existing = listServerStartTickets(lockDir, lockPort, token, lockLeaseMs)
|
||||
const existing = listServerStartTickets(lockDir, lockPort, token)
|
||||
.filter(entry => !entry.choosing && Number.isInteger(entry.number));
|
||||
const number = existing.reduce((maximum, entry) => Math.max(maximum, entry.number), 0) + 1;
|
||||
fs.writeFileSync(ticketFile, JSON.stringify({ pid: process.pid, token, number }), { flag: 'wx', mode: 0o600 });
|
||||
fs.writeFileSync(
|
||||
ticketFile,
|
||||
JSON.stringify({ pid: process.pid, processIdentity, token, number }),
|
||||
{ flag: 'wx', mode: 0o600 }
|
||||
);
|
||||
fs.rmSync(choosingFile, { force: true });
|
||||
leaseTimer = setInterval(() => {
|
||||
const now = new Date();
|
||||
try { fs.utimesSync(ticketFile, now, now); } catch { /* cleanup or lease loss */ }
|
||||
}, Math.max(25, Math.floor(lockLeaseMs / 3)));
|
||||
if (leaseTimer.unref) leaseTimer.unref();
|
||||
|
||||
while (true) {
|
||||
const entries = listServerStartTickets(lockDir, lockPort, token, lockLeaseMs);
|
||||
const entries = listServerStartTickets(lockDir, lockPort, token);
|
||||
const anotherOwnerIsChoosing = entries.some(entry => entry.choosing && entry.token !== token);
|
||||
const tickets = entries
|
||||
.filter(entry => !entry.choosing && Number.isInteger(entry.number))
|
||||
@@ -279,7 +317,6 @@ async function withServerStartLock(port, task, {
|
||||
}
|
||||
return await task();
|
||||
} finally {
|
||||
clearInterval(leaseTimer);
|
||||
fs.rmSync(choosingFile, { force: true });
|
||||
fs.rmSync(ticketFile, { force: true });
|
||||
}
|
||||
|
||||
@@ -159,7 +159,7 @@ async function main() {
|
||||
assert.strictEqual(maximumActive, 1);
|
||||
});
|
||||
|
||||
await test('port-scoped startup lock renews its lease while the owner is active', async () => {
|
||||
await test('port-scoped startup lock preserves an old ticket from the same live process', async () => {
|
||||
const lockPort = port + 6;
|
||||
const lockDir = path.join(tmp, 'startup-locks');
|
||||
let releaseFirst;
|
||||
@@ -169,16 +169,25 @@ async function main() {
|
||||
const first = withServerStartLock(lockPort, async () => {
|
||||
markFirstEntered();
|
||||
await new Promise(resolve => { releaseFirst = resolve; });
|
||||
}, { lockDir, timeoutMs: 1000, leaseMs: 100 });
|
||||
}, { lockDir, timeoutMs: 1000 });
|
||||
await firstEntered;
|
||||
const second = withServerStartLock(lockPort, async () => {
|
||||
secondEntered = true;
|
||||
}, { lockDir, timeoutMs: 1000, leaseMs: 100 });
|
||||
await new Promise(resolve => setTimeout(resolve, 250));
|
||||
assert.strictEqual(secondEntered, false);
|
||||
releaseFirst();
|
||||
await Promise.all([first, second]);
|
||||
assert.strictEqual(secondEntered, true);
|
||||
const ticketName = fs.readdirSync(lockDir).find(name => name.endsWith('.ticket'));
|
||||
assert.ok(ticketName);
|
||||
const old = new Date(Date.now() - 60 * 1000);
|
||||
fs.utimesSync(path.join(lockDir, ticketName), old, old);
|
||||
try {
|
||||
await assert.rejects(
|
||||
withServerStartLock(lockPort, async () => { secondEntered = true; }, {
|
||||
lockDir,
|
||||
timeoutMs: 200
|
||||
}),
|
||||
/timed out waiting for Plan Canvas startup lock/
|
||||
);
|
||||
assert.strictEqual(secondEntered, false);
|
||||
} finally {
|
||||
releaseFirst();
|
||||
await first;
|
||||
}
|
||||
});
|
||||
|
||||
await test('port-scoped startup lock recovers dead tickets and failed owners', async () => {
|
||||
@@ -199,20 +208,24 @@ async function main() {
|
||||
assert.ok(!fs.readdirSync(lockDir).some(name => name.startsWith(`ecc-plan-canvas-${lockPort}-`)));
|
||||
});
|
||||
|
||||
await test('port-scoped startup lock expires a stale ticket after PID reuse', async () => {
|
||||
await test('port-scoped startup lock recovers a stale ticket after PID reuse', async () => {
|
||||
const lockPort = port + 5;
|
||||
const lockDir = path.join(tmp, 'startup-locks');
|
||||
fs.mkdirSync(lockDir, { recursive: true });
|
||||
const reusedPidTicket = path.join(lockDir, `ecc-plan-canvas-${lockPort}-reused-pid.ticket`);
|
||||
fs.writeFileSync(
|
||||
reusedPidTicket,
|
||||
JSON.stringify({ pid: process.pid, token: 'reused-pid', number: 1 })
|
||||
JSON.stringify({
|
||||
pid: process.pid,
|
||||
processIdentity: 'an-exited-process-instance',
|
||||
token: 'reused-pid',
|
||||
number: 1
|
||||
})
|
||||
);
|
||||
assert.strictEqual(
|
||||
await withServerStartLock(lockPort, async () => 'recovered', {
|
||||
lockDir,
|
||||
timeoutMs: 1000,
|
||||
leaseMs: 100
|
||||
timeoutMs: 1000
|
||||
}),
|
||||
'recovered'
|
||||
);
|
||||
|
||||
@@ -307,6 +307,44 @@ async function main() {
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (await test('an active PDF export rejects incomplete snapshot uploads before reading them', async () => {
|
||||
holdPdfExport = true;
|
||||
const first = request(port, 'GET', `/api/session/${key}/pdf`);
|
||||
let stalled = null;
|
||||
try {
|
||||
await waitFor(() => typeof releasePdfExport === 'function');
|
||||
const competing = new Promise((resolve, reject) => {
|
||||
stalled = http.request({
|
||||
host: '127.0.0.1',
|
||||
port,
|
||||
method: 'POST',
|
||||
path: `/api/session/${key}/pdf`,
|
||||
agent: false,
|
||||
headers: { 'content-type': 'application/json', 'content-length': 1024 }
|
||||
}, res => {
|
||||
let data = '';
|
||||
res.on('data', chunk => { data += chunk; });
|
||||
res.on('end', () => resolve({ statusCode: res.statusCode, body: data }));
|
||||
});
|
||||
stalled.on('error', reject);
|
||||
stalled.write('{"html":"partial');
|
||||
});
|
||||
const timeout = new Promise((_, reject) => {
|
||||
setTimeout(() => reject(new Error('incomplete PDF upload was not rejected')), 500);
|
||||
});
|
||||
const overloaded = await Promise.race([competing, timeout]);
|
||||
assert.strictEqual(overloaded.statusCode, 429);
|
||||
assert.strictEqual(jsonBody(overloaded).code, 'PDF_EXPORT_BUSY');
|
||||
} finally {
|
||||
if (stalled) stalled.destroy();
|
||||
const release = releasePdfExport;
|
||||
holdPdfExport = false;
|
||||
releasePdfExport = null;
|
||||
if (release) release();
|
||||
}
|
||||
assert.strictEqual((await first).statusCode, 200);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (await test('PDF failures log diagnostics without disclosing local paths', async () => {
|
||||
try {
|
||||
const rendererError = new Error('Chromium failed at /Users/private/browser-profile');
|
||||
|
||||
Reference in New Issue
Block a user