diff --git a/tests/scripts/plan-canvas.test.js b/tests/scripts/plan-canvas.test.js index cc3da84d2..da4746272 100644 --- a/tests/scripts/plan-canvas.test.js +++ b/tests/scripts/plan-canvas.test.js @@ -21,6 +21,11 @@ const { createPlanCanvasServer } = require('../../scripts/lib/plan-canvas/server class SkippedTest extends Error {} +function failureText(error) { + try { return error?.stack || error?.message || String(error); } + catch { return 'Unprintable thrown value'; } +} + function createTestRunner(log = console.log) { let results = { passed: 0, failed: 0, skipped: 0 }; return { @@ -36,7 +41,7 @@ function createTestRunner(log = console.log) { log(` SKIP ${name}: ${error.message}`); } else { results = { ...results, failed: results.failed + 1 }; - log(` FAIL ${name}\n Error: ${error.stack || error.message}`); + log(` FAIL ${name}\n Error: ${failureText(error)}`); } } } @@ -102,6 +107,42 @@ async function withFixtureCleanup(callback, cleanups) { return result; } +// Explicit close and finally cleanup await the same attempt, including failure. +function onceCleanup(cleanup) { + let pending; + return () => { + if (!pending) pending = Promise.resolve().then(cleanup); + return pending; + }; +} + +async function withResourceScope(callback) { + const clients = []; + const servers = []; + const roots = []; + const own = (group, cleanup) => { + const close = onceCleanup(cleanup); + group.push(close); + return close; + }; + const resources = { + client: cleanup => own(clients, cleanup), + server: cleanup => own(servers, cleanup), + root: cleanup => own(roots, cleanup), + }; + return withFixtureCleanup(() => callback(resources), () => [...clients, ...servers, ...roots]); +} + +// Requests are owned before setup writes or awaits. Destroy the response and +// request independently so one cleanup failure cannot leave the other open. +function ownHttpClient(req, getResponse, resources) { + const cleanup = () => withFixtureCleanup(() => {}, () => { + const response = getResponse(); + return [...(response ? [() => response.destroy()] : []), () => req.destroy()]; + }); + return resources ? resources.client(cleanup) : onceCleanup(cleanup); +} + // Capture fresh real dispatchers without binding sockets. Each request captures // its own filesystem facade; the fixture owns those canvases until teardown. // Artifact bodies are inert response bytes, never executed in a browser. @@ -673,10 +714,13 @@ async function fixtureIsolationTests(test) { } } -function request(port, method, requestPath, { body = null, headers = {} } = {}) { - return new Promise((resolve, reject) => { +function request(port, method, requestPath, { + body = null, headers = {}, resources, transport = http, onData = () => {} +} = {}) { + let response; + const pending = new Promise((resolve, reject) => { const payload = body === null ? null : JSON.stringify(body); - const req = http.request( + const req = transport.request( { host: '127.0.0.1', port, @@ -688,17 +732,24 @@ function request(port, method, requestPath, { body = null, headers = {} } = {}) : headers }, res => { + response = res; + res.on('error', reject); let data = ''; res.on('data', chunk => { data += chunk; + onData(chunk); }); res.on('end', () => resolve({ statusCode: res.statusCode, headers: res.headers, body: data })); } ); + ownHttpClient(req, () => response, resources); req.on('error', reject); if (payload) req.write(payload); req.end(); }); + // Cleanup can reject an abandoned long-poll; awaiters still see this rejection. + pending.catch(() => {}); + return pending; } function jsonBody(res) { @@ -706,13 +757,16 @@ function jsonBody(res) { } // Open an SSE stream and collect parsed events into `received`. -function openSse(port, key) { +function openSse(port, key, { resources, transport = http } = {}) { const received = []; let close = () => {}; + let response; const ready = new Promise((resolve, reject) => { - const req = http.get( + const req = transport.get( { host: '127.0.0.1', port, path: `/events/${key}`, agent: false }, res => { + response = res; + res.on('error', reject); let buffer = ''; res.on('data', chunk => { buffer += chunk; @@ -730,9 +784,10 @@ function openSse(port, key) { resolve(); } ); + close = ownHttpClient(req, () => response, resources); req.on('error', reject); - close = () => req.destroy(); }); + ready.catch(() => {}); return { received, ready, close: () => close() }; } @@ -751,6 +806,128 @@ function waitFor(predicate, { timeoutMs = 3000, intervalMs = 20 } = {}) { }); } +// Deterministic ownership checks: no socket/listener or shared module mutation. +async function integrationCleanupTests(test) { + async function capture(callback) { + try { return { threw: false, value: await callback() }; } + catch (error) { return { threw: true, error }; } + } + for (const primary of [Object.freeze(new Error('primary integration failure')), 0, false, null, undefined]) { + await test(`integration resource cleanup preserves ${String(primary)} and attempts all stages`, async () => { + const stages = []; + const secondary = new Error('cleanup failure'); + const result = await capture(() => withResourceScope(async resources => { + resources.root(() => { stages.push('root'); throw secondary; }); + resources.server(() => { stages.push('server'); throw secondary; }); + resources.client(() => { stages.push('client'); throw secondary; }); + throw primary; + })); + assert.strictEqual(result.threw, true); + assert.ok(Object.is(result.error, primary)); + assert.deepStrictEqual(stages, ['client', 'server', 'root']); + }); + await test(`runner records falsy/frozen failure ${String(primary)} without replacing it`, async () => { + const output = []; + const runner = createTestRunner(line => output.push(line)); + await runner.test('failure', () => { throw primary; }); + assert.deepStrictEqual(runner.results, { passed: 0, failed: 1, skipped: 0 }); + assert.strictEqual(output.length, 1); + assert.match(output[0], /FAIL failure/); + }); + } + await test('cleanup-only failure reports the first failure after every owned stage', async () => { + const first = Object.freeze(new Error('client cleanup')); + const stages = []; + const result = await capture(() => withResourceScope(resources => { + resources.client(() => { stages.push('client'); throw first; }); + resources.server(() => { stages.push('server'); throw new Error('server cleanup'); }); + resources.root(() => { stages.push('root'); throw new Error('root cleanup'); }); + })); + assert.strictEqual(result.error, first); + assert.deepStrictEqual(stages, ['client', 'server', 'root']); + }); + await test('explicit server close and automatic cleanup share one close attempt', async () => { + let attempts = 0; + await withResourceScope(async resources => { + const close = resources.server(async () => { attempts++; }); + await close(); + await close(); + }); + assert.strictEqual(attempts, 1); + }); + await test('second acquisition failure still closes the first server and roots', async () => { + const primary = new Error('second acquisition'); + const stages = []; + const result = await capture(() => withResourceScope(resources => { + resources.root(() => stages.push('root-one')); + resources.server(() => stages.push('server-one')); + resources.root(() => stages.push('root-two')); + throw primary; + })); + assert.strictEqual(result.error, primary); + assert.deepStrictEqual(stages, ['server-one', 'root-one', 'root-two']); + }); + function fakeHttp() { + const { EventEmitter } = require('events'); + const stages = []; + const request = new EventEmitter(); + const response = new EventEmitter(); + let respond; + request.write = () => {}; + request.end = () => {}; + request.destroy = () => { stages.push('request'); request.emit('error', new Error('owned request destroyed')); }; + response.destroy = () => { stages.push('response'); }; + return { + stages, request, response, + transport: { get(_options, callback) { respond = callback; return request; }, request(_options, callback) { respond = callback; return request; } }, + respond() { respond(response); }, + }; + } + for (const headers of [false, true]) { + await test(`SSE failure cleanup owns request before headers=${headers}`, async () => { + const fake = fakeHttp(); + const primary = new Error('SSE assertion'); + let sse; + const result = await capture(() => withResourceScope(async resources => { + sse = openSse(1, 'synthetic', { resources, transport: fake.transport }); + if (headers) { fake.respond(); await sse.ready; } + throw primary; + })); + assert.strictEqual(result.error, primary); + assert.deepStrictEqual(fake.stages, headers ? ['response', 'request'] : ['request']); + await sse.ready.catch(() => {}); + await sse.close(); + assert.strictEqual(fake.stages.filter(stage => stage === 'request').length, 1); + }); + } + await test('abandoned long-poll cleanup reaps its client without an unhandled rejection', async () => { + const fake = fakeHttp(); + const primary = new Error('heartbeat assertion'); + let pending; + const chunks = []; + const result = await capture(() => withResourceScope(async resources => { + pending = request(1, 'GET', '/synthetic', { resources, transport: fake.transport, onData: chunk => chunks.push(chunk.toString()) }); + fake.respond(); + fake.response.emit('data', Buffer.from(' ')); + throw primary; + })); + assert.strictEqual(result.error, primary); + assert.deepStrictEqual(chunks, [' ']); + assert.deepStrictEqual(fake.stages, ['response', 'request']); + assert.strictEqual((await capture(() => pending)).threw, true); + }); + await test('request setup failure after acquisition closes its client and keeps the original error', async () => { + const fake = fakeHttp(); + const primary = Object.freeze(new Error('write failed')); + fake.request.write = () => { throw primary; }; + const result = await capture(() => withResourceScope(resources => request(1, 'POST', '/synthetic', { + body: {}, resources, transport: fake.transport, + }))); + assert.strictEqual(result.error, primary); + assert.deepStrictEqual(fake.stages, ['request']); + }); +} + async function main() { console.log('\n=== Testing plan-canvas server ===\n'); @@ -759,425 +936,430 @@ async function main() { await artifactSecurityTests(test); await artifactRaceTests(test); await fixtureIsolationTests(test); + await integrationCleanupTests(test); if (process.argv.includes('--artifact-security-only')) { printResults(suite.results); return; } - const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-canvas-server-')); - const artifact = path.join(tmp, 'demo.plan.md'); - fs.writeFileSync(artifact, '# Plan: Demo\n\n## Files to Change\n\n| File | Action |\n|---|---|\n| `a.js` | UPDATE |\n'); - const htmlArtifact = path.join(tmp, 'report.html'); - fs.writeFileSync(htmlArtifact, '
hi