From 354c4641a9608847c053ed5b209f6e0a8804602b Mon Sep 17 00:00:00 2001 From: JD Date: Thu, 8 Oct 2026 10:05:57 -0400 Subject: [PATCH] fix(session-options): keep the external flag when App Settings resyncs the CLI catalog _syncCliLaunchCatalog() rebuilt window.__codemanCliCatalog from /api/clis rows, which carry no capabilities, so after App Settings loaded the CLI list isExternalCliSession('claude') fell back to the kind check and Respawn and Ralph disappeared again until a reload. The rebuild now carries external over from the served catalog; a newly created custom CLI has no previous entry and falls back to kind, which is right since custom entries are external. Tests pin the resync and the openSessionOptions() call site, and the /api/clis comment names the page catalog as the deliberate capabilities exception. --- src/web/public/settings-ui.js | 6 ++++++ src/web/routes/cli-registry-routes.ts | 4 +++- test/run-mode-ui.test.ts | 19 +++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index e8fed034..f967a3c8 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -3160,12 +3160,18 @@ Object.assign(CodemanApp.prototype, { /** Keep the launch surfaces in sync with Settings mutations without a reload. */ _syncCliLaunchCatalog() { if (!Array.isArray(this._cliList) || this._cliList.length === 0) return; + // /api/clis rows carry no capabilities, so keep the served catalog's `external` + // (isExternalCliSession() reads it). A new custom CLI has none and falls back to `kind`. + const previous = new Map( + (Array.isArray(window.__codemanCliCatalog) ? window.__codemanCliCatalog : []).map((cli) => [cli.id, cli]) + ); window.__codemanCliCatalog = this._cliList.map((cli) => ({ id: cli.id, label: cli.label, shortBadge: cli.shortBadge, order: cli.order, kind: cli.kind, + external: previous.get(cli.id)?.external, enabled: cli.enabled, available: cli.kind === 'shell' || (cli.enabled && cli.installed), })); diff --git a/src/web/routes/cli-registry-routes.ts b/src/web/routes/cli-registry-routes.ts index a4d4b136..3b75ffda 100644 --- a/src/web/routes/cli-registry-routes.ts +++ b/src/web/routes/cli-registry-routes.ts @@ -312,7 +312,9 @@ export function registerCliRegistryRoutes(app: FastifyInstance): void { // admin/settings surface; every SPAWN-time caller elsewhere uses // enabledClis() instead). Deliberately excludes launch/env/capabilities/ // overlays/discovery — the same rule every other catalogue-export surface in - // this codebase follows. + // this codebase follows, with one deliberate exception: the page catalog + // (`window.__codemanCliCatalog`, server.ts) carries `capabilities.external`, + // which Session Options reads to keep Claude's Respawn and Ralph tabs. // // NOT gated on cliManagementEnabled: reading the list is cheap and is not // the risky part. The Settings UI section simply never fetches this while diff --git a/test/run-mode-ui.test.ts b/test/run-mode-ui.test.ts index d237b129..b6f82047 100644 --- a/test/run-mode-ui.test.ts +++ b/test/run-mode-ui.test.ts @@ -556,6 +556,25 @@ describe('Codex quick start settings', () => { expect(context.isExternalCliSession('custom-agent')).toBe(true); }); + it('keeps the served external flag when App Settings resyncs the catalog from /api/clis', () => { + const catalog = CATALOG.map((cli) => ({ ...cli, external: cli.id !== 'claude' && cli.id !== 'shell' })); + const { app, context } = loadUi(undefined, catalog); + // /api/clis rows: no capabilities, so no `external`. + app._cliList = CATALOG.map((cli) => ({ ...cli, installed: true })); + app.runMode = 'claude'; + app._syncCliLaunchCatalog(); + expect(context.isExternalCliSession('claude')).toBe(false); + expect(context.isExternalCliSession('codex')).toBe(true); + }); + + it('gates Session Options on isExternalCliSession, not the launch-path check', () => { + const src = readFileSync(resolve(import.meta.dirname, '../src/web/public/session-ui.js'), 'utf8'); + const open = src.slice(src.indexOf('\n openSessionOptions(')); + const body = open.slice(0, open.indexOf('\n },')); + expect(body).toContain('isExternalCliSession(session.mode)'); + expect(body).not.toContain('isExternalCliRunMode('); + }); + it('shows everything when the flags were never injected', () => { // A cached page from a build without the injection, or a solo popup. Hiding // every run button on a doubt would leave a working install nothing to click.