fix(#1538): roadmap upgrade must not process.exit inside the no-throw hub (#1539)

* fix(core): roadmap upgrade must not process.exit inside the no-throw hub (#1538)

The upgrade handler called process.stderr.write + process.exit(1) on an
unsupported --convention, structurally bypassing the command-routing-hub's
no-throw contract (ADR-0012). It also parsed only the space-separated
--convention <value> form, so --convention=<value> was silently dropped and
defaulted to milestone-prefixed, running the migration the user did not request.

Throw instead of exit (the hub converts to HandlerFailure and the adapter routes
it through error()); parse both --convention forms and fail closed on any
missing/unsupported value.

Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz

* chore(changeset): Fixed fragment for #1539 (roadmap upgrade hub contract)

Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz

---------

Co-authored-by: Tom Boucher <trekkie@nomorestars.com>

.changeset/proud-sloths-wander.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 1539
+---
+`roadmap upgrade` now rejects an unsupported or malformed `--convention` value (including the `--convention=` form) instead of silently running the milestone-prefixed migration, and no longer hard-exits inside the command-routing hub.

src/roadmap-command-router.cts

@@ -181,10 +181,25 @@ function routeRoadmapCommand({ roadmap, args, cwd, raw, error }: RouteRoadmapCom
       },
       'upgrade': () => {
         const dryRun = !args.includes('--apply');
-        const convention = args.find((_a, i) => args[i - 1] === '--convention') || 'milestone-prefixed';
+        // Parse `--convention <value>` and `--convention=<value>`. When the flag is
+        // absent entirely, default to the only supported convention; when present
+        // with a missing/unsupported value, fall through to the rejection below
+        // (fail-closed — never silently run a migration the user did not request).
+        let convention = 'milestone-prefixed';
+        const conventionFlagIdx = args.findIndex(
+          (a) => a === '--convention' || a.startsWith('--convention='),
+        );
+        if (conventionFlagIdx !== -1) {
+          const token = args[conventionFlagIdx];
+          convention = token.includes('=')
+            ? token.slice(token.indexOf('=') + 1)
+            : (args[conventionFlagIdx + 1] ?? '');
+        }
         if (convention !== 'milestone-prefixed') {
-          process.stderr.write('Only --convention milestone-prefixed is supported\n');
-          process.exit(1);
+          // No-throw hub contract (ADR-0012): a hub-dispatched handler must not call
+          // process.exit. Throw instead — the hub converts this to HandlerFailure and
+          // the adapter routes it through the injected error() boundary.
+          throw new Error('Only --convention milestone-prefixed is supported');
         }
         const plan = roadmapUpgrade.computeMigrationPlan(cwd);
         roadmapUpgrade.applyMigration(cwd, plan, { dryRun });

tests/roadmap-command-router.test.cjs

@@ -1,9 +1,10 @@
 'use strict';
 
-const { describe, test, before, after } = require('node:test');
+const { describe, test, before, after, beforeEach, afterEach, mock } = require('node:test');
 const assert = require('node:assert/strict');
 
 const { routeRoadmapCommand } = require('../gsd-core/bin/lib/roadmap-command-router.cjs');
+const roadmapUpgrade = require('../gsd-core/bin/lib/roadmap-upgrade.cjs');
 
 // These tests exercise router dispatch with a deterministic runtime context.
 let _prevWorkstream;
@@ -85,3 +86,83 @@ describe('roadmap-command-router', () => {
     assert.equal(message, 'Unknown roadmap subcommand. Available: analyze, get-phase, update-plan-progress, annotate-dependencies, validate, upgrade');
   });
 });
+
+// #1538 — the `upgrade` handler must honor the no-throw hub contract (ADR-0012)
+// and parse `--convention` in both `--convention <v>` and `--convention=<v>` forms.
+describe('roadmap upgrade — hub contract + --convention parsing (#1538)', () => {
+  let exitCalls;
+  let applyCalls;
+
+  beforeEach(() => {
+    exitCalls = [];
+    applyCalls = [];
+    // A hub-dispatched handler must never call process.exit. Mock it to throw a
+    // sentinel so the test can observe an illegal exit instead of killing the runner.
+    mock.method(process, 'exit', (code) => {
+      exitCalls.push(code);
+      throw new Error('UNEXPECTED_PROCESS_EXIT');
+    });
+    // Stub the migration so the supported-convention path is observable without a real project.
+    mock.method(roadmapUpgrade, 'computeMigrationPlan', () => ({ phases: [] }));
+    mock.method(roadmapUpgrade, 'applyMigration', (_cwd, _plan, opts) => {
+      applyCalls.push({ opts });
+    });
+  });
+
+  afterEach(() => {
+    mock.restoreAll();
+  });
+
+  function runUpgrade(args) {
+    let message = null;
+    routeRoadmapCommand({
+      roadmap: {},
+      args,
+      cwd: '/tmp/proj',
+      raw: false,
+      error: (msg) => { message = msg; },
+    });
+    return message;
+  }
+
+  test('rejects an unsupported convention (space form) via error(), never process.exit', () => {
+    const message = runUpgrade(['roadmap', 'upgrade', '--convention', 'sequential']);
+    assert.equal(exitCalls.length, 0, 'a hub handler must not call process.exit');
+    assert.equal(message, 'Only --convention milestone-prefixed is supported');
+    assert.equal(applyCalls.length, 0, 'must not run the migration for an unsupported convention');
+  });
+
+  test('rejects an unsupported convention in equals form — no silent fail-open', () => {
+    const message = runUpgrade(['roadmap', 'upgrade', '--convention=sequential']);
+    assert.equal(exitCalls.length, 0, 'a hub handler must not call process.exit');
+    assert.equal(message, 'Only --convention milestone-prefixed is supported');
+    assert.equal(applyCalls.length, 0, '--convention=sequential must not silently run the milestone-prefixed migration');
+  });
+
+  test('rejects empty/malformed convention values fail-closed (never runs the migration)', () => {
+    for (const args of [
+      ['roadmap', 'upgrade', '--convention', ''],
+      ['roadmap', 'upgrade', '--convention='],
+      ['roadmap', 'upgrade', '--convention'],
+      ['roadmap', 'upgrade', '--convention==x'],
+    ]) {
+      const message = runUpgrade(args);
+      assert.equal(
+        message,
+        'Only --convention milestone-prefixed is supported',
+        `should reject ${JSON.stringify(args)}`,
+      );
+      assert.equal(exitCalls.length, 0, 'a hub handler must not call process.exit');
+    }
+    assert.equal(applyCalls.length, 0, 'no migration runs for any malformed convention');
+  });
+
+  test('accepts the supported convention in both forms and the default (reaches applyMigration, dry-run)', () => {
+    assert.equal(runUpgrade(['roadmap', 'upgrade', '--convention', 'milestone-prefixed']), null);
+    assert.equal(runUpgrade(['roadmap', 'upgrade', '--convention=milestone-prefixed']), null);
+    assert.equal(runUpgrade(['roadmap', 'upgrade']), null);
+    assert.equal(exitCalls.length, 0);
+    assert.equal(applyCalls.length, 3, 'all three supported invocations reach applyMigration');
+    assert.ok(applyCalls.every((c) => c.opts.dryRun === true), 'no --apply ⇒ dryRun');
+  });
+});