Fix Windows libuv abort in concept-seed after a successful roll (#526)

* Fix Windows libuv abort in concept-seed after a successful roll process.exit() with a live fetch keep-alive socket trips libuv's UV_HANDLE_CLOSING assertion on Windows (nodejs/node#56645), aborting the CLI with 0xC0000409 after complete output on the successful-roll path. Destroy the global undici dispatcher before the explicit exit so no socket is left to race; the hard exit stays, keeping the no-linger guarantee on blackholed networks. Fixes #504 Prepared with AI assistance (Cursor agent) under maintainer direction. * Add regression test for the successful-API dispatcher teardown The suite covered local rolls and the unreachable-API fallback but never a successful roll, the one path where a pooled keep-alive socket exists at exit (issue #504). Serve a real /api/roll from a local server and assert the CLI destroys fetch's global dispatcher before its explicit exit. Verified to fail without the fix. Prepared with AI assistance (Cursor agent) under maintainer direction. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-12 06:06:37 +03:00 · 2026-08-08 18:41:19 -07:00
parent 628aac5a40
commit ddf4526fb5
2 changed files with 104 additions and 3 deletions
@@ -692,6 +692,13 @@ if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.ur
    process.exitCode = 1;
  }
  // A raced-out fetch may still hold a socket; exit explicitly so the CLI
-  // never lingers on a dead network path after output is written.
+  // never lingers on a dead network path after output is written. Destroy
+  // fetch's global undici dispatcher first: process.exit() with a live
+  // keep-alive socket trips a libuv assertion on Windows and aborts the
+  // process after a successful roll (nodejs/node#56645).
+  const dispatcher = globalThis[Symbol.for('undici.globalDispatcher.1')];
+  if (dispatcher && typeof dispatcher.destroy === 'function') {
+    try { await dispatcher.destroy(); } catch { /* exit regardless */ }
+  }
  process.exit(process.exitCode ?? 0);
 }
@@ -1,10 +1,11 @@
 import { describe, it } from 'node:test';
 import assert from 'node:assert/strict';
-import { spawnSync } from 'node:child_process';
+import { spawn, spawnSync } from 'node:child_process';
 import { mkdtempSync, writeFileSync } from 'node:fs';
+import { createServer } from 'node:http';
 import { tmpdir } from 'node:os';
 import path from 'node:path';
-import { fileURLToPath } from 'node:url';
+import { fileURLToPath, pathToFileURL } from 'node:url';
 import {
  readConceptCatalog,
  validateConceptCatalog,
@@ -711,3 +712,96 @@ describe('init gate', () => {
    );
  });
 });
+
+// The Windows abort in issue #504 (nodejs/node#56645) needs three things at
+// once: a successful roll over Node's undici-backed fetch, the keep-alive
+// socket that success leaves pooled, and the explicit process.exit at the end
+// of the CLI. The suite's other API test exercises only the unreachable-API
+// fallback, which leaves no pooled socket and so never walked the crashing
+// path. This one serves a real roll from a local server and asserts the CLI
+// destroys fetch's global dispatcher before exiting, so the teardown cannot
+// silently regress. The teardown is Node fetch internals, so the CLI is
+// spawned with node even when the suite itself runs under bun.
+describe('API roll path', () => {
+  const NODE = process.versions.bun ? 'node' : process.execPath;
+
+  const ROLL_PAYLOAD = {
+    poolRevision: 'api-test-rev',
+    approvedCount: 6,
+    catalogCount: 9,
+    challengers: [{
+      id: 'api-test-world',
+      form: 'a letterpress print shop, where type, ink, and impression organize the page',
+      spark: 'Deep impressions hold the central promise while loose sorts wait in the case.',
+      system: ['Palette/material: dense ink black bitten into soft cotton paper'],
+      webLeverage: 'Variable-font impression depth with a keyboard-readable page structure',
+    }],
+    compositions: [],
+  };
+
+  // Wraps the global dispatcher's destroy so the parent test can observe the
+  // CLI's exit teardown. The warmup fetch makes fetch install the dispatcher
+  // before the wrap, and parks a keep-alive socket in its pool, which is the
+  // exact state the Windows crash needs at exit.
+  const PRELOAD = [
+    "const KEY = Symbol.for('undici.globalDispatcher.1');",
+    'await fetch(`${process.env.IMPECCABLE_API_URL}/warmup`).then(r => r.arrayBuffer()).catch(() => {});',
+    'const dispatcher = globalThis[KEY];',
+    "if (dispatcher && typeof dispatcher.destroy === 'function') {",
+    '  const destroy = dispatcher.destroy.bind(dispatcher);',
+    '  dispatcher.destroy = (...args) => {',
+    "    process.stderr.write('DISPATCHER_DESTROY_CALLED\\n');",
+    '    return destroy(...args);',
+    '  };',
+    '}',
+    '',
+  ].join('\n');
+
+  it('resolves a successful roll and destroys the fetch dispatcher before the explicit exit', async () => {
+    const requests = [];
+    const server = createServer((req, res) => {
+      requests.push(req.url);
+      if (req.url.startsWith('/api/roll?')) {
+        res.setHeader('Content-Type', 'application/json');
+        res.end(JSON.stringify(ROLL_PAYLOAD));
+        return;
+      }
+      res.statusCode = 404;
+      res.end('not found');
+    });
+    await new Promise(resolveListen => server.listen(0, '127.0.0.1', resolveListen));
+    try {
+      const dir = mkdtempSync(path.join(tmpdir(), 'concept-seed-api-'));
+      writeFileSync(path.join(dir, 'PRODUCT.md'), '# Test Product\n\n## Platform\n\nweb\n');
+      const preloadPath = path.join(dir, 'wrap-dispatcher.mjs');
+      writeFileSync(preloadPath, PRELOAD);
+      const result = await new Promise((resolveRun, rejectRun) => {
+        const child = spawn(NODE, [
+          '--import', pathToFileURL(preloadPath).href,
+          SCRIPT, '--scope', 'direction', '--mode', 'persuade', '--from', 'api-test',
+        ], {
+          cwd: dir,
+          env: {
+            ...process.env,
+            IMPECCABLE_CATALOG_DIR: '/nonexistent-catalog-dir',
+            IMPECCABLE_API_URL: `http://127.0.0.1:${server.address().port}/api`,
+          },
+        });
+        let stdout = '';
+        let stderr = '';
+        child.stdout.on('data', chunk => { stdout += chunk; });
+        child.stderr.on('data', chunk => { stderr += chunk; });
+        child.on('error', rejectRun);
+        child.on('close', status => resolveRun({ status, stdout, stderr }));
+      });
+      assert.equal(result.status, 0, `stderr: ${result.stderr}`);
+      assert.equal(requests.some(url => url.startsWith('/api/roll?')), true, 'the CLI must hit the roll endpoint');
+      assert.match(result.stdout, /source: api/);
+      assert.match(result.stdout, /letterpress print shop/);
+      assert.match(result.stdout, /TELEMETRY:/);
+      assert.match(result.stderr, /DISPATCHER_DESTROY_CALLED/, 'the dispatcher must be destroyed before process.exit');
+    } finally {
+      server.close();
+    }
+  });
+});