From d0875653e310b1ec87cf580929c47c40d1d9451d Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 00:35:28 +0300 Subject: [PATCH 01/17] chore(deps): bump subswitch 0.2.0 -> 0.3.0 0.2.0's connect timer capped time-to-first-byte and synthesized 504s during upstream slow periods, killing Claude Code sessions machine-wide (subswitch#42). 0.3.0 (ADR-010 relay hardening) removes the TTFB cap and other proxy-side policy. Relay on this machine already restarted onto 0.3.0. --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9283dc93..9a30b2f7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,7 @@ "@clack/prompts": "^0.9.1", "commander": "^12.0.0", "picocolors": "^1.1.1", - "subswitch": "0.2.0" + "subswitch": "0.3.0" }, "bin": { "devflow": "dist/cli.js" @@ -1522,9 +1522,9 @@ "license": "MIT" }, "node_modules/subswitch": { - "version": "0.2.0", - "resolved": "https://registry.npmjs.org/subswitch/-/subswitch-0.2.0.tgz", - "integrity": "sha512-x07ACfkE495sBTZ6bV/O9jdARWdPcE09Jtaq0rg1o0vHgFKx+zc1nv4tZvxIWRyfZPH1f0XgKf+0njBi1Ylkcw==", + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/subswitch/-/subswitch-0.3.0.tgz", + "integrity": "sha512-tmXMR8K3kCXgNVGPohvUE4vNPX4wPrSD/RotO57Lb9lk44VsYhDbQXF/2Bd8iGwimvGks+ixsrVRUIiu6s/aew==", "license": "MIT", "dependencies": { "@clack/prompts": "^1.7.0", diff --git a/package.json b/package.json index 6d6760c5..b19b2d8f 100644 --- a/package.json +++ b/package.json @@ -63,7 +63,7 @@ "@clack/prompts": "^0.9.1", "commander": "^12.0.0", "picocolors": "^1.1.1", - "subswitch": "0.2.0" + "subswitch": "0.3.0" }, "devDependencies": { "@mdscript/mds": "0.2.0", From b547db3d83d15e2cb0eb5f63849e1a38b1f74541 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 00:56:06 +0300 Subject: [PATCH 02/17] fix(post-install): merge settings.json instead of overwriting on init (#313) installSettings now uses a merge strategy (D-SETTINGS-1): hook entries are added idempotently to an existing settings.json by exact command string; statusLine and attribution are set only when absent; user-owned keys (env, permissions, model, apiKeyHelper) are never touched. Parse failure emits a warning and leaves the file byte-identical. Adds exported mergeDevflowSettingsTemplate helper for testing. --- src/targets/claude-code/post-install.ts | 110 +++++++++-- tests/post-install-merge.test.ts | 234 ++++++++++++++++++++++++ 2 files changed, 325 insertions(+), 19 deletions(-) create mode 100644 tests/post-install-merge.test.ts diff --git a/src/targets/claude-code/post-install.ts b/src/targets/claude-code/post-install.ts index 0c8bda58..f951ec20 100644 --- a/src/targets/claude-code/post-install.ts +++ b/src/targets/claude-code/post-install.ts @@ -7,6 +7,7 @@ import { getManagedSettingsPath } from './claude-paths.js'; import { getGitignoreEntries, getDocsDir } from '../../core/project-paths.js'; import { writeFileAtomicExclusive } from '../../core/fs-atomic.js'; import type { SecurityMode } from '../../core/manifest.js'; +import type { HookMatcher } from './hooks.js'; /** * Type guard for Node.js system errors with error codes. @@ -791,10 +792,65 @@ export async function stripUserSecurityDenyList( return { removed }; } +/** + * Merge Devflow's template hook entries and top-level fields into an existing + * parsed settings object. Idempotent by exact command string — a hook that is + * already present is skipped. Sets `statusLine` and `attribution` only when the + * user has no existing value for them. Mutates `existing` in place. + * + * D-SETTINGS-1: merge strategy — never replace; only add devflow entries that are absent. + * Returns { changed: true } when any field was added. + * + * Exported for testing. + */ +export function mergeDevflowSettingsTemplate( + existing: Record, + template: Record, +): { changed: boolean } { + let changed = false; + + // Merge hook entries — idempotent by exact command string + const tmplHooks = (template.hooks ?? {}) as Record; + for (const [event, matchers] of Object.entries(tmplHooks)) { + const existingHooksObj = (existing.hooks as Record | undefined) ?? {}; + existing.hooks = existingHooksObj; + existingHooksObj[event] ??= []; + const eventArr = existingHooksObj[event]; + for (const matcher of matchers) { + const cmd = matcher.hooks[0]?.command; + if (!cmd) continue; + const alreadyPresent = eventArr.some((m) => m.hooks.some((h) => h.command === cmd)); + if (!alreadyPresent) { + eventArr.push(matcher); + changed = true; + } + } + } + + // Set statusLine only if the user has none + if (existing.statusLine === undefined && template.statusLine !== undefined) { + existing.statusLine = template.statusLine; + changed = true; + } + + // Set attribution only if the user has none + if (existing.attribution === undefined && template.attribution !== undefined) { + existing.attribution = template.attribution; + changed = true; + } + + return { changed }; +} + /** * Install or update settings.json with Devflow configuration. - * Prompts interactively in TTY mode when settings already exist. - * In non-TTY mode, skips override (safe default). + * + * Strategy (D-SETTINGS-1: merge, never overwrite wholesale): + * - Fresh file: write the template directly. + * - Existing file: MERGE devflow hook entries and fields into the parsed object. + * Idempotent by exact command string — existing hooks are never duplicated or removed. + * Preserves every user key (env, permissions, apiKeyHelper, model, etc.) untouched. + * - Parse failure: warn and skip; file left byte-identical (never clobber a broken file). * * The deny list is handled by init's dedicated security step * (applyUserSecurityDenyList / installManagedSettings) after installSettings completes. @@ -828,22 +884,38 @@ export async function installSettings( return; } - // Settings exist — check if they already have hooks - let hasHooks = false; + // Settings exist — parse and merge (never overwrite wholesale) + let existingParsed: Record; try { - const existing = JSON.parse(await fs.readFile(settingsPath, 'utf-8')); - hasHooks = !!existing.hooks; - } catch { /* parse error = treat as no hooks */ } + existingParsed = JSON.parse(await fs.readFile(settingsPath, 'utf-8')) as Record; + } catch { + // Parse failure — warn and skip; file left byte-identical (D-SETTINGS-1) + if (verbose) { + p.log.warn( + 'settings.json could not be parsed — Devflow hooks not added. ' + + 'Fix the JSON manually and re-run devflow init.', + ); + } + return; + } - if (hasHooks) { - // Settings already configured with hooks — nothing to do + const templateParsed = JSON.parse(settingsContent) as Record; + + // Merge template into existing (mutates existingParsed in place). + // If nothing needs to be added the write is skipped entirely. + const { changed } = mergeDevflowSettingsTemplate(existingParsed, templateParsed); + + if (!changed) { + // Already fully configured — nothing to do return; } - // Settings exist without hooks — prompt in TTY, warn in non-TTY + // Settings need Devflow hooks added. + // In TTY mode: ask before writing so the user knows what's happening. + // In non-TTY mode: merge silently (operation is non-destructive — D-SETTINGS-1). if (process.stdin.isTTY) { const confirmed = await p.confirm({ - message: 'settings.json exists without hooks (Working Memory needs hooks). Override?', + message: 'settings.json exists without some Devflow hooks. Add Working Memory hooks and HUD configuration?', initialValue: true, }); @@ -852,15 +924,15 @@ export async function installSettings( process.exit(0); } - if (confirmed) { - await fs.writeFile(settingsPath, settingsContent, 'utf-8'); - p.log.success('Settings overridden'); - } else { - p.log.info('Keeping existing settings'); + if (!confirmed) { + p.log.info('Keeping existing settings unchanged'); + return; } - } else { - p.log.warn('Settings exist without hooks. Working Memory requires hooks.'); - p.log.info('Re-run interactively to configure, or manually add hooks to settings.json'); + } + + await writeFileAtomicExclusive(settingsPath, JSON.stringify(existingParsed, null, 2) + '\n'); + if (verbose) { + p.log.success('Settings updated with Devflow hooks and HUD'); } } catch (error: unknown) { if (verbose) { diff --git a/tests/post-install-merge.test.ts b/tests/post-install-merge.test.ts new file mode 100644 index 00000000..13a621f6 --- /dev/null +++ b/tests/post-install-merge.test.ts @@ -0,0 +1,234 @@ +/** + * Tests for FIX 1 (issue #313): mergeDevflowSettingsTemplate in post-install.ts. + * + * D-SETTINGS-1: installSettings must MERGE devflow hook entries into an existing + * settings.json rather than replacing it. A user's env block, permissions, apiKeyHelper, + * or model assignment must never be destroyed. Parse failure must leave the file + * byte-identical. Idempotent by exact command string. + * + * Coverage: + * - Hooks already present → changed:false (no duplicate insertion) + * - Hooks absent → changed:true (added) + * - Preserves user-owned keys untouched (env, permissions, model, etc.) + * - statusLine set only when absent + * - attribution set only when absent + * - statusLine preserved when already set by the user + * - Idempotent — double merge is the same as single merge + * - Template hook with no command string is skipped silently + * - Empty template → changed:false + */ + +import { describe, it, expect } from 'vitest'; +import { mergeDevflowSettingsTemplate } from '../src/targets/claude-code/post-install.js'; +import type { HookMatcher } from '../src/targets/claude-code/hooks.js'; + +// ─── Helpers ───────────────────────────────────────────────────────────────── + +function makeHookMatcher(command: string, timeout = 10): HookMatcher { + return { hooks: [{ type: 'command', command, timeout }] }; +} + +function makeTemplate(commands: string[], statusLine = 'devflow: {branch}', attribution = 'Devflow'): Record { + const matchers = commands.map((cmd) => makeHookMatcher(cmd)); + return { + hooks: { + 'SessionStart': matchers, + }, + statusLine, + attribution, + }; +} + +// ─── mergeDevflowSettingsTemplate ──────────────────────────────────────────── + +describe('mergeDevflowSettingsTemplate — FIX 1 (issue #313)', () => { + + it('returns changed:true and adds hooks when existing has none', () => { + const existing: Record = {}; + const template = makeTemplate(['/devflow/scripts/hooks/run-hook memory-worker']); + const { changed } = mergeDevflowSettingsTemplate(existing, template); + expect(changed).toBe(true); + const hooks = (existing.hooks as Record)['SessionStart'] ?? []; + expect(hooks.length).toBe(1); + expect(hooks[0]?.hooks[0]?.command).toBe('/devflow/scripts/hooks/run-hook memory-worker'); + }); + + it('returns changed:false when all template hooks are already present', () => { + const cmd = '/devflow/scripts/hooks/run-hook memory-worker'; + const existing: Record = { + hooks: { + 'SessionStart': [makeHookMatcher(cmd)], + }, + statusLine: 'devflow: {branch}', + attribution: 'Devflow', + }; + const template = makeTemplate([cmd]); + const { changed } = mergeDevflowSettingsTemplate(existing, template); + expect(changed).toBe(false); + }); + + it('does not duplicate a hook that already exists (exact command match)', () => { + const cmd = '/devflow/scripts/hooks/run-hook capture-turn'; + const existing: Record = { + hooks: { + 'SessionStart': [makeHookMatcher(cmd)], + }, + }; + const template = makeTemplate([cmd]); + mergeDevflowSettingsTemplate(existing, template); + mergeDevflowSettingsTemplate(existing, template); // second pass + const hooks = (existing.hooks as Record)['SessionStart'] ?? []; + const count = hooks.filter((m) => m.hooks.some((h) => h.command === cmd)).length; + expect(count).toBe(1); + }); + + it('preserves user keys: env block is never touched', () => { + const existing: Record = { + env: { ANTHROPIC_API_KEY: 'sk-test-keep-me', CUSTOM_VAR: 'value' }, + }; + const template = makeTemplate(['/devflow/scripts/hooks/run-hook preamble']); + mergeDevflowSettingsTemplate(existing, template); + // env block must survive unchanged + const env = existing.env as Record; + expect(env.ANTHROPIC_API_KEY).toBe('sk-test-keep-me'); + expect(env.CUSTOM_VAR).toBe('value'); + }); + + it('preserves user keys: permissions block is never touched', () => { + const existing: Record = { + permissions: { deny: ['Bash(*)', 'Edit(*)'] }, + }; + const template = makeTemplate(['/devflow/scripts/hooks/run-hook preamble']); + mergeDevflowSettingsTemplate(existing, template); + const perms = existing.permissions as Record; + expect(perms.deny).toEqual(['Bash(*)', 'Edit(*)']); + }); + + it('preserves user keys: model field is never touched', () => { + const existing: Record = { + model: 'claude-opus-4-5', + }; + const template = makeTemplate(['/devflow/scripts/hooks/run-hook preamble']); + mergeDevflowSettingsTemplate(existing, template); + expect(existing.model).toBe('claude-opus-4-5'); + }); + + it('preserves user keys: apiKeyHelper is never touched', () => { + const existing: Record = { + apiKeyHelper: '/usr/local/bin/get-api-key', + }; + const template = makeTemplate(['/devflow/scripts/hooks/run-hook preamble']); + mergeDevflowSettingsTemplate(existing, template); + expect(existing.apiKeyHelper).toBe('/usr/local/bin/get-api-key'); + }); + + it('sets statusLine from template when absent on existing', () => { + const existing: Record = {}; + const template = makeTemplate([], 'devflow v2'); + mergeDevflowSettingsTemplate(existing, template); + expect(existing.statusLine).toBe('devflow v2'); + }); + + it('does NOT overwrite statusLine when user already has one', () => { + const existing: Record = { statusLine: 'my-custom-status-line' }; + const template = makeTemplate([], 'devflow v2'); + mergeDevflowSettingsTemplate(existing, template); + expect(existing.statusLine).toBe('my-custom-status-line'); + }); + + it('sets attribution from template when absent on existing', () => { + const existing: Record = {}; + const template = { statusLine: 's', attribution: 'Devflow' }; + mergeDevflowSettingsTemplate(existing, template); + expect(existing.attribution).toBe('Devflow'); + }); + + it('does NOT overwrite attribution when user already has one', () => { + const existing: Record = { attribution: 'my-org' }; + const template = { statusLine: 's', attribution: 'Devflow' }; + mergeDevflowSettingsTemplate(existing, template); + expect(existing.attribution).toBe('my-org'); + }); + + it('adds only the missing hooks when some are present and some are not', () => { + const cmd1 = '/devflow/scripts/hooks/run-hook memory-worker'; + const cmd2 = '/devflow/scripts/hooks/run-hook capture-turn'; + const existing: Record = { + hooks: { 'SessionStart': [makeHookMatcher(cmd1)] }, // cmd1 present, cmd2 absent + }; + const template: Record = { + hooks: { 'SessionStart': [makeHookMatcher(cmd1), makeHookMatcher(cmd2)] }, + }; + const { changed } = mergeDevflowSettingsTemplate(existing, template); + expect(changed).toBe(true); + const hooks = (existing.hooks as Record)['SessionStart'] ?? []; + const cmds = hooks.flatMap((m) => m.hooks.map((h) => h.command)); + expect(cmds).toContain(cmd1); + expect(cmds).toContain(cmd2); + // cmd1 appears exactly once — no duplicate + expect(cmds.filter((c) => c === cmd1).length).toBe(1); + }); + + it('handles multiple hook events independently', () => { + const cmd = '/devflow/scripts/hooks/run-hook ensure-proxy'; + const existing: Record = {}; + const template: Record = { + hooks: { + 'SessionStart': [makeHookMatcher(cmd)], + 'UserPromptSubmit': [makeHookMatcher(cmd)], + }, + }; + mergeDevflowSettingsTemplate(existing, template); + const ss = (existing.hooks as Record)['SessionStart'] ?? []; + const up = (existing.hooks as Record)['UserPromptSubmit'] ?? []; + expect(ss.some((m) => m.hooks.some((h) => h.command === cmd))).toBe(true); + expect(up.some((m) => m.hooks.some((h) => h.command === cmd))).toBe(true); + }); + + it('skips a template hook entry that has no command string', () => { + const existing: Record = {}; + const template: Record = { + hooks: { + 'SessionStart': [{ hooks: [] } as unknown as HookMatcher], // no command + }, + }; + const { changed } = mergeDevflowSettingsTemplate(existing, template); + // No command → nothing to add → not changed from hooks (hooks key initialized but no entry pushed) + expect(changed).toBe(false); + }); + + it('empty template results in changed:false', () => { + const existing: Record = { model: 'claude-opus-4-5' }; + const template: Record = {}; + const { changed } = mergeDevflowSettingsTemplate(existing, template); + expect(changed).toBe(false); + expect(existing.model).toBe('claude-opus-4-5'); + }); + + it('is idempotent — double merge produces same result as single merge', () => { + const cmd = '/devflow/scripts/hooks/run-hook memory-worker'; + const existing: Record = {}; + const template = makeTemplate([cmd], 'devflow v2', 'Devflow'); + + mergeDevflowSettingsTemplate(existing, template); + const snapshotAfterFirst = JSON.stringify(existing); + + // D-SETTINGS-1: second merge must be a no-op + const { changed: changedOnSecond } = mergeDevflowSettingsTemplate(existing, template); + expect(changedOnSecond).toBe(false); + expect(JSON.stringify(existing)).toBe(snapshotAfterFirst); + }); + + it('preserves existing hook order — devflow hooks appended, not prepended', () => { + const existingCmd = '/user/custom-hook'; + const devflowCmd = '/devflow/scripts/hooks/run-hook memory-worker'; + const existing: Record = { + hooks: { 'SessionStart': [makeHookMatcher(existingCmd)] }, + }; + const template = makeTemplate([devflowCmd]); + mergeDevflowSettingsTemplate(existing, template); + const hooks = (existing.hooks as Record)['SessionStart'] ?? []; + expect(hooks[0]?.hooks[0]?.command).toBe(existingCmd); // user hook comes first + expect(hooks[1]?.hooks[0]?.command).toBe(devflowCmd); // devflow hook appended + }); +}); From fa1e28ec1d2e02eb06075d0d3f319b8e92a62bc9 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 00:56:15 +0300 Subject: [PATCH 03/17] fix(proxy): gate env-var stripping on proxy.json existence (#313) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds proxyJsonExists() to proxy-state.ts — a discriminator helper that distinguishes "file absent" from readProxyState()'s tolerant Ok(defaultState) on ENOENT, which was indistinguishable from a file present with DEFAULT_PROXY_PORT. Gates ANTHROPIC_BASE_URL and CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT stripping in init.ts and uninstall.ts on proxyJsonExists() (D-STRIP-1): strip only when proxy.json exists, proving devflow previously wrote those env vars. Prevents destroying a user's independently-managed ANTHROPIC_BASE_URL during devflow init. --- src/cli/commands/init.ts | 16 ++++++---- src/cli/commands/uninstall.ts | 30 ++++++++++++------- src/core/proxy-state.ts | 23 ++++++++++++++ tests/proxy-state.test.ts | 56 +++++++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 16 deletions(-) diff --git a/src/cli/commands/init.ts b/src/cli/commands/init.ts index e89d0fca..a1fa1c4c 100644 --- a/src/cli/commands/init.ts +++ b/src/cli/commands/init.ts @@ -35,7 +35,7 @@ import { addCaptureHooks, removeCaptureHooks } from './capture.js'; import { removeDreamHook } from './legacy-hooks.js'; import { addProxyHooks, removeProxyHooks, applyProxyEnv, stripProxyEnv, runProxyPreflight, buildRealPreflightDeps } from './proxy.js'; import { reapplyAgentMapping, readAgentMapping } from '../../core/agent-models.js'; -import { readProxyState, writeProxyState, buildProxyState, buildRoutingConfigJson, DEFAULT_PROXY_PORT } from '../../core/proxy-state.js'; +import { readProxyState, writeProxyState, buildProxyState, buildRoutingConfigJson, DEFAULT_PROXY_PORT, proxyJsonExists } from '../../core/proxy-state.js'; import type { Settings } from '../../targets/claude-code/hooks.js'; import { stripDevflowTeammateModeFromJson } from '../../core/teammate-mode-cleanup.js'; // Settings/HookMatcher types used by hook utilities — each in their own module @@ -1635,14 +1635,18 @@ export const initCommand = new Command('init') content = JSON.stringify(parsedSettings, null, 2) + '\n'; } // Proxy env: ANTHROPIC_BASE_URL strip-then-add, scoped to managed port. - // Read proxy.json to learn which port we own — only that URL is stripped. - // A user's own localhost gateway on any other port is preserved. + // D-STRIP-1: only strip when proxy.json exists — evidence that Devflow previously + // wrote ANTHROPIC_BASE_URL. Without this gate, a fresh init on a machine where + // DEFAULT_PROXY_PORT (4141) happens to be a user's own gateway (LiteLLM etc.) + // would silently delete both their URL and the window-enforcement var. // Invariant: proxy.json always reflects the final settled state after the // preflight block above — all paths that force proxyEnabled=false also write // proxy.json enabled:false (avoids PF-015), so managedPort == effectivePort. - const proxyStateForStrip = await readProxyState(devflowDir); - const managedPort = proxyStateForStrip.ok ? proxyStateForStrip.value.port : DEFAULT_PROXY_PORT; - content = stripProxyEnv(content, managedPort); + if (await proxyJsonExists(devflowDir)) { + const proxyStateForStrip = await readProxyState(devflowDir); + const managedPort = proxyStateForStrip.ok ? proxyStateForStrip.value.port : DEFAULT_PROXY_PORT; + content = stripProxyEnv(content, managedPort); + } if (proxyEnabled) content = applyProxyEnv(content, effectivePort); if (content !== original) { diff --git a/src/cli/commands/uninstall.ts b/src/cli/commands/uninstall.ts index c4c5b210..14a161c7 100644 --- a/src/cli/commands/uninstall.ts +++ b/src/cli/commands/uninstall.ts @@ -16,7 +16,7 @@ import { removeDreamHook } from './legacy-hooks.js'; import { removeHudStatusLine } from './hud.js'; import { removeContextHook } from './context.js'; import { applyDisableToSettings } from './proxy.js'; -import { readProxyState, DEFAULT_PROXY_PORT } from '../../core/proxy-state.js'; +import { readProxyState, proxyJsonExists } from '../../core/proxy-state.js'; import { hudCacheDir } from '../../core/cache.js'; import { revertExternalAgents } from '../../core/agent-models.js'; import type { Settings } from '../../targets/claude-code/hooks.js'; @@ -814,12 +814,14 @@ export async function runCleanupPhase(opts: { settingsContent = stripFlags(settingsContent); // also strips viewMode via view-mode registry entry settingsContent = stripDevflowTeammateModeFromJson(settingsContent); // Remove proxy hooks and ANTHROPIC_BASE_URL env in a single parse-mutate-serialize pass. - // REG-1: scope the URL strip to the port Devflow manages — use the pre-captured port - // (from opts.managedProxyPorts) because proxy.json is already deleted by - // removeDevFlowInstallArtifacts before this phase runs. A user's own localhost - // gateway on any other port is left in settings untouched. - { - const managedPort = opts.managedProxyPorts?.get(scope) ?? DEFAULT_PROXY_PORT; + // D-STRIP-1: only strip when we have devflow-managed evidence (proxy.json existed + // before artifact removal). managedProxyPorts is populated only when the file was + // present — if the scope has no entry, proxy was never managed and we leave + // ANTHROPIC_BASE_URL / CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT untouched. + // REG-1: the port-scoped URL strip protects any foreign localhost gateway on a + // different port — the managedPort value is from the pre-captured proxy.json port. + if (opts.managedProxyPorts?.has(scope)) { + const managedPort = opts.managedProxyPorts.get(scope)!; const parsedSettings = JSON.parse(settingsContent) as Settings; applyDisableToSettings(parsedSettings, managedPort); settingsContent = JSON.stringify(parsedSettings, null, 2) + '\n'; @@ -1047,14 +1049,22 @@ export const uninstallCommand = new Command('uninstall') // Pre-capture managed proxy ports for each scope BEFORE artifact removal. // proxy.json is deleted by removeDevFlowInstallArtifacts (inside runFullPhaseForScope), // so reading it after the full phase would always yield DEFAULT_PROXY_PORT. (F6) + // + // D-STRIP-1: only populate managedProxyPorts when proxy.json exists — evidence that + // Devflow has managed the proxy on this machine. runCleanupPhase gates the + // applyDisableToSettings call on the port being present in the map, so when the file + // is absent (never managed), no env strip is attempted and a user's own gateway vars + // are left untouched. const managedProxyPorts = new Map(); if (!isSelectiveUninstall) { for (const scope of scopesToUninstall) { try { const paths = await getInstallationPaths(scope); - const proxyState = await readProxyState(paths.devflowDir); - if (proxyState.ok) managedProxyPorts.set(scope, proxyState.value.port); - } catch { /* non-fatal: if we can't read, we fall back to DEFAULT_PROXY_PORT */ } + if (await proxyJsonExists(paths.devflowDir)) { + const proxyState = await readProxyState(paths.devflowDir); + if (proxyState.ok) managedProxyPorts.set(scope, proxyState.value.port); + } + } catch { /* non-fatal */ } } } diff --git a/src/core/proxy-state.ts b/src/core/proxy-state.ts index 9ff4861e..f5a91c65 100644 --- a/src/core/proxy-state.ts +++ b/src/core/proxy-state.ts @@ -267,6 +267,29 @@ export function proxyBaseUrl(port: number): string { // isProxyEnabled — the primary contract other modules consume // --------------------------------------------------------------------------- +/** + * Returns true when ~/.devflow/proxy.json exists on disk — i.e. Devflow has + * previously managed the proxy on this machine. + * + * This is the evidence gate used by init and uninstall before stripping + * ANTHROPIC_BASE_URL / CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT: + * we only strip those vars when we can prove Devflow wrote them. + * + * D-STRIP-1: gate proxy env stripping on devflow-managed evidence. + * readProxyState() returns Ok(defaultState) on ENOENT — it cannot distinguish + * "file absent" from "file present with DEFAULT_PROXY_PORT". Callers that need + * to differentiate must use proxyJsonExists() rather than checking the result + * of readProxyState(). + */ +export async function proxyJsonExists(devflowDir: string): Promise { + try { + await fs.access(join(devflowDir, 'proxy.json')); + return true; + } catch { + return false; + } +} + /** * Check whether the Devflow proxy is currently enabled. * Returns false when the proxy state file is missing, unreadable, or malformed. diff --git a/tests/proxy-state.test.ts b/tests/proxy-state.test.ts index fa4cb4b7..47be33eb 100644 --- a/tests/proxy-state.test.ts +++ b/tests/proxy-state.test.ts @@ -24,6 +24,7 @@ import { writeProxyState, buildRoutingConfigJson, buildProxyState, + proxyJsonExists, DEFAULT_PROXY_PORT, RUNTIME_VERSION_RE, DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS, @@ -406,3 +407,58 @@ describe('RUNTIME_VERSION_RE — version string validation (AC-S4)', () => { expect(RUNTIME_VERSION_RE.test(v)).toBe(expected); }); }); + +// --------------------------------------------------------------------------- +// FIX 2 (issue #313): proxyJsonExists — D-STRIP-1 gate discriminator +// +// readProxyState() returns Ok(defaultState) on ENOENT — callers cannot use its +// success to prove proxy.json exists. proxyJsonExists() is the correct gate for +// stripping managed env vars: strip only when the file exists (proving devflow +// previously wrote it). +// --------------------------------------------------------------------------- + +describe('proxyJsonExists — FIX 2 (issue #313)', () => { + it('returns false when proxy.json does not exist', async () => { + const result = await proxyJsonExists(tmpDir); + expect(result).toBe(false); + }); + + it('returns true when proxy.json exists (even with only a port)', async () => { + await fs.writeFile(path.join(tmpDir, 'proxy.json'), JSON.stringify({ port: DEFAULT_PROXY_PORT })); + const result = await proxyJsonExists(tmpDir); + expect(result).toBe(true); + }); + + it('returns true for a full proxy.json written by writeProxyState', async () => { + const state = buildProxyState(DEFAULT_PROXY_PORT, '/path/relay.js', '/path/config.json', '0.2.0'); + await writeProxyState(tmpDir, state); + const result = await proxyJsonExists(tmpDir); + expect(result).toBe(true); + }); + + it('returns true even when the file contains malformed JSON (file-exists ≠ valid JSON)', async () => { + await fs.writeFile(path.join(tmpDir, 'proxy.json'), 'not-valid-json{{'); + const result = await proxyJsonExists(tmpDir); + expect(result).toBe(true); + }); + + it('returns false when devflowDir itself does not exist', async () => { + const nonexistent = path.join(tmpDir, 'does-not-exist'); + const result = await proxyJsonExists(nonexistent); + expect(result).toBe(false); + }); + + it('discriminates absent-file from Ok(defaultState) returned by readProxyState', async () => { + // readProxyState() returns Ok({enabled:false,port:DEFAULT_PROXY_PORT,...}) on ENOENT, + // which is indistinguishable from a file present with those exact values. + // proxyJsonExists() must correctly report false for the absent-file case. + const readResult = await readProxyState(tmpDir); + expect(readResult.ok).toBe(true); // readProxyState returns Ok even on ENOENT + if (readResult.ok) { + expect(readResult.value.port).toBe(DEFAULT_PROXY_PORT); // same as a real file with default port + } + // proxyJsonExists is the correct discriminator — file is absent + const exists = await proxyJsonExists(tmpDir); + expect(exists).toBe(false); + }); +}); From f5edf41a752ed072f2a1e6470a1b1995e473146e Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 00:56:22 +0300 Subject: [PATCH 04/17] fix(proxy): evaluate foreign-env check before adopted early-return (#313) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In runProxyPreflight, the foreign ANTHROPIC_BASE_URL refusal (check ④) now runs BEFORE the adopted early-return (check ③). Previously, a healthy relay on the target port triggered an early-return that silently skipped the foreign-gateway guard entirely — enabling devflow's proxy while a user's custom gateway URL was set. D-EFR-5: readSettingsJson is called in the healthy-relay branch before returning Ok(adopted:true). swallowSettingsReadError semantics preserved: when that flag is true, readSettingsJson returns '{}' on I/O failure instead of throwing. --- src/cli/commands/proxy.ts | 27 +++++++++-- tests/proxy.test.ts | 95 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 3 deletions(-) diff --git a/src/cli/commands/proxy.ts b/src/cli/commands/proxy.ts index ddb91b83..a41f8e80 100644 --- a/src/cli/commands/proxy.ts +++ b/src/cli/commands/proxy.ts @@ -187,9 +187,10 @@ function _stripProxyEnvFromObject(settings: Settings, managedPort: number): bool const env = s.env as Record | undefined; if (!env) return false; - // Devflow is the only producer of this var — always remove it, regardless of whether - // the URL is still ours. Port-scoping protects a FOREIGN url value; there is no - // foreign value of this key to protect. (applies PF-015, ADR-003) + // D-STRIP-1: callers gate this function on devflow-managed evidence (proxy.json exists), + // so by the time we get here we know Devflow wrote this var. Always remove it + // unconditionally — there is no foreign value of this key to protect. Port-scoping + // protects ANTHROPIC_BASE_URL (see below), but not this var. (applies PF-015, ADR-003) const hadWindowVar = env[UNKNOWN_MODEL_WINDOW_ENV] !== undefined; delete env[UNKNOWN_MODEL_WINDOW_ENV]; @@ -453,6 +454,26 @@ export async function runProxyPreflight( PROBE_TIMEOUT_MS, ); if (healthResult.ok && isOurRelayBody(healthResult.value)) { + // D-EFR-5: evaluate the foreign-env refusal BEFORE the adopted early-return. + // The old order (adopted-return first, foreign-check only when port is free) + // allowed enabling while a foreign ANTHROPIC_BASE_URL was set — any healthy + // relay on the port caused an early-return that silently skipped check ④. + // Now we read settings and refuse if a foreign URL is present, regardless of + // port state. swallowSettingsReadError semantics are preserved: when that flag + // is true, readSettingsJson() returns '{}' on I/O failure instead of throwing, + // so the catch here is only hit in the strict (runEnable) path. + let settingsJsonForForeignCheck: string; + try { + settingsJsonForForeignCheck = await deps.readSettingsJson(); + } catch { + return Err('Could not read settings.json — check file permissions'); + } + const envStateForAdopt = readProxyEnvState(settingsJsonForForeignCheck, port); + if (envStateForAdopt === 'foreign') { + return Err( + 'An existing ANTHROPIC_BASE_URL in settings.json points to a different gateway — Devflow will not overwrite it', + ); + } return Ok({ binPath, npxWarning, adopted: true }); } // Port accepting but health timed out, failed, or not our relay diff --git a/tests/proxy.test.ts b/tests/proxy.test.ts index 8fc4534f..68fb2580 100644 --- a/tests/proxy.test.ts +++ b/tests/proxy.test.ts @@ -1755,3 +1755,98 @@ describe('Phase 4 / T7-extended: fully-enabled state includes UNKNOWN_MODEL_WIND expect(env[WINDOW_ENV]).toBe('1'); }); }); + +// ─── FIX 3 (issue #313): runProxyPreflight — D-EFR-5 ordering ─────────────── +// +// Foreign-env check (check ④) must evaluate BEFORE the adopted early-return +// (check ③). Previously, a healthy relay on the port caused an early-return that +// silently skipped the foreign-gateway refusal entirely. +// +// Regression tests: +// REG-EFR-1: foreign URL + healthy relay → Err (refusal), not adopted +// REG-EFR-2: no foreign URL + healthy relay → Ok(adopted:true) (normal adopt) +// REG-EFR-3: foreign URL + port free → Err (check ④ on the free-port path, unchanged) + +describe('runProxyPreflight — FIX 3 D-EFR-5 foreign-env check ordering (issue #313)', () => { + const port = DEFAULT_PORT; + const codexAuthPath = '/home/test/.codex/auth.json'; + const configPath = '/home/test/.devflow/proxy-routing.json'; + const logPath = '/home/test/.devflow/logs/proxy.log'; + + const ourHealthBody = '{"name":"subswitch","version":"0.2.0","providers":[{"id":"anthropic","configured":true,"modelCount":0}]}'; + + // REG-EFR-1: healthy relay + foreign URL → must be refused (not adopted silently) + it('REG-EFR-1: returns Err when relay is healthy but ANTHROPIC_BASE_URL is a foreign gateway', async () => { + const deps = makeDeps({ + tcpConnectable: vi.fn().mockResolvedValue(true), // port up + httpGet: vi.fn().mockResolvedValue({ ok: true, value: ourHealthBody }), // our relay + readSettingsJson: vi.fn().mockResolvedValue(JSON.stringify({ + env: { ANTHROPIC_BASE_URL: 'https://litellm.mycompany.internal' }, // foreign URL + })), + }); + const result = await runProxyPreflight(port, codexAuthPath, configPath, logPath, deps); + // Must be refused — not adopted silently + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain('ANTHROPIC_BASE_URL'); + } + }); + + // REG-EFR-2: healthy relay + no foreign URL → normal adopt + it('REG-EFR-2: returns Ok(adopted:true) when relay is healthy and no foreign URL', async () => { + const deps = makeDeps({ + tcpConnectable: vi.fn().mockResolvedValue(true), + httpGet: vi.fn().mockResolvedValue({ ok: true, value: ourHealthBody }), + readSettingsJson: vi.fn().mockResolvedValue('{}'), // no foreign URL + }); + const result = await runProxyPreflight(port, codexAuthPath, configPath, logPath, deps); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.value.adopted).toBe(true); + } + }); + + // REG-EFR-3: port free + foreign URL → check ④ on the free-port path (already tested above, + // reconfirmed here to show both paths refuse a foreign URL) + it('REG-EFR-3: returns Err when port is free but ANTHROPIC_BASE_URL is a foreign gateway', async () => { + const deps = makeDeps({ + tcpConnectable: vi.fn().mockResolvedValue(false), // port free + readSettingsJson: vi.fn().mockResolvedValue(JSON.stringify({ + env: { ANTHROPIC_BASE_URL: 'https://litellm.mycompany.internal' }, + })), + }); + const result = await runProxyPreflight(port, codexAuthPath, configPath, logPath, deps); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain('ANTHROPIC_BASE_URL'); + } + }); + + // REG-EFR-1b: healthy relay + our own relay URL on the port → adopted (not foreign) + it('REG-EFR-1b: adopted when relay is healthy and ANTHROPIC_BASE_URL is already our relay URL', async () => { + const deps = makeDeps({ + tcpConnectable: vi.fn().mockResolvedValue(true), + httpGet: vi.fn().mockResolvedValue({ ok: true, value: ourHealthBody }), + readSettingsJson: vi.fn().mockResolvedValue(JSON.stringify({ + env: { ANTHROPIC_BASE_URL: `http://127.0.0.1:${port}` }, // our URL → not foreign + })), + }); + const result = await runProxyPreflight(port, codexAuthPath, configPath, logPath, deps); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.value.adopted).toBe(true); + } + }); + + // Verify that readSettingsJson is called in the adopt path (D-EFR-5 gate is wired) + it('calls readSettingsJson in the adopted path to evaluate foreign-env state', async () => { + const readSettingsJson = vi.fn().mockResolvedValue('{}'); + const deps = makeDeps({ + tcpConnectable: vi.fn().mockResolvedValue(true), + httpGet: vi.fn().mockResolvedValue({ ok: true, value: ourHealthBody }), + readSettingsJson, + }); + await runProxyPreflight(port, codexAuthPath, configPath, logPath, deps); + expect(readSettingsJson).toHaveBeenCalledTimes(1); + }); +}); From 236392e523ca8185334bbb4b40cbfa8b3db0e729 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 00:56:32 +0300 Subject: [PATCH 05/17] fix(ensure-proxy): attempt binPath re-resolution before warning (#313) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the persisted binPath is missing or stale (e.g. npx cache GC cleared the subswitch package dir), the ensure-proxy hook now attempts dependency-light re-resolution before emitting the "relay binary not found" warning: Strategy a: walk up from `command -v devflow` → find node_modules/subswitch → read bin.subswitch from package.json via node (already present). Strategy b: `command -v subswitch` — globally installed CLI. Both strategies are best-effort; the hook falls through to the existing warning and always exits 0 when re-resolution fails (avoids PF-001, PF-009). The healed path is used for the current session only; `devflow proxy --enable` persists it. Loop is bounded at 6 iterations (avoids PF-017). --- src/assets/scripts/hooks/ensure-proxy | 69 +++++++++++++-- tests/shell-hooks.test.ts | 122 ++++++++++++++++++++++++++ 2 files changed, 186 insertions(+), 5 deletions(-) diff --git a/src/assets/scripts/hooks/ensure-proxy b/src/assets/scripts/hooks/ensure-proxy index 856add49..eb245c40 100644 --- a/src/assets/scripts/hooks/ensure-proxy +++ b/src/assets/scripts/hooks/ensure-proxy @@ -181,12 +181,71 @@ PROXY_BIN=$(json_field_file "$PROXY_STATE_FILE" "binPath" "") PROXY_CONFIG=$(json_field_file "$PROXY_STATE_FILE" "configPath" "") dbg "PROXY_BIN=$PROXY_BIN PROXY_CONFIG=$PROXY_CONFIG" -# Validate prerequisites before spawning +# Validate prerequisites before spawning. +# D-FIX4: when the persisted binPath is missing or stale (e.g. npx cache GC, upgrade), +# attempt dependency-light re-resolution before warning. Two strategies: +# a) Walk up from `devflow` CLI to find devflow's node_modules/subswitch. +# b) `command -v subswitch` — globally installed CLI. +# Re-resolution is best-effort: on failure the existing warning is emitted and the +# hook exits 0 as before. The healed path is used for this session only; the next +# successful `devflow proxy --enable` persists it to proxy.json. (avoids PF-001, +# avoids PF-009: non-fatal on failure, always exits 0) if [ -z "$PROXY_BIN" ] || [ ! -f "$PROXY_BIN" ]; then - log "prereq fail: binPath missing or not a file: $PROXY_BIN" - CONTEXT="[Devflow proxy] Warning: relay binary not found. Run 'devflow proxy --enable' to restore external model routing." - json_session_output "$CONTEXT" - exit 0 + log "prereq: binPath missing or not a file (${PROXY_BIN:-}) — attempting re-resolution" + _RESOLVED_BIN="" + + # Resolve node early for use in re-resolution (moved here from below) + _NODE_FOR_RESOLVE=$(command -v node 2>/dev/null || true) + + # Strategy a: locate devflow CLI → walk up to find node_modules/subswitch + if [ -n "$_NODE_FOR_RESOLVE" ]; then + _DF_CMD=$(command -v devflow 2>/dev/null || true) + if [ -n "$_DF_CMD" ]; then + _DF_REAL=$(realpath "$_DF_CMD" 2>/dev/null || readlink -f "$_DF_CMD" 2>/dev/null || echo "$_DF_CMD") + _WALK_DIR=$(dirname "$_DF_REAL") + _WALK_GUARD=0 + while [ "$_WALK_GUARD" -lt 6 ] && [ -n "$_WALK_DIR" ] && [ "$_WALK_DIR" != "/" ]; do + _SW_PKG="$_WALK_DIR/node_modules/subswitch/package.json" + if [ -f "$_SW_PKG" ]; then + # Read bin field from package.json using node (already confirmed present). + # Env-var pass avoids shell-quoting issues with paths containing spaces. + _BIN_REL=$(SUBSWITCH_PKG_JSON="$_SW_PKG" \ + "$_NODE_FOR_RESOLVE" -p \ + "try{var p=JSON.parse(require('fs').readFileSync(process.env.SUBSWITCH_PKG_JSON,'utf-8'));var b=p.bin;typeof b==='string'?b:b&&b.subswitch?b.subswitch:''}catch(e){''}" \ + 2>/dev/null) || _BIN_REL="" + if [ -n "$_BIN_REL" ] && [ "$_BIN_REL" != "undefined" ]; then + _CAND="$(dirname "$_SW_PKG")/$_BIN_REL" + if [ -f "$_CAND" ]; then + _RESOLVED_BIN="$_CAND" + log "re-resolved binPath via devflow walk: $_RESOLVED_BIN" + fi + fi + break # found subswitch dir — stop walking regardless of bin result + fi + _WALK_DIR=$(dirname "$_WALK_DIR") + _WALK_GUARD=$(( _WALK_GUARD + 1 )) + done + fi + fi + + # Strategy b: subswitch globally installed as a CLI + if [ -z "$_RESOLVED_BIN" ]; then + _SW_GLOBAL=$(command -v subswitch 2>/dev/null || true) + if [ -n "$_SW_GLOBAL" ] && [ -f "$_SW_GLOBAL" ]; then + _RESOLVED_BIN="$_SW_GLOBAL" + log "re-resolved binPath via command -v subswitch: $_RESOLVED_BIN" + fi + fi + + if [ -n "$_RESOLVED_BIN" ]; then + PROXY_BIN="$_RESOLVED_BIN" + log "binPath healed for this session: $PROXY_BIN" + else + log "re-resolution failed — binPath not found" + CONTEXT="[Devflow proxy] Warning: relay binary not found. Run 'devflow proxy --enable' to restore external model routing." + json_session_output "$CONTEXT" + exit 0 + fi fi NODE_BIN=$(command -v node 2>/dev/null || true) diff --git a/tests/shell-hooks.test.ts b/tests/shell-hooks.test.ts index 9ac2d485..ada2f360 100644 --- a/tests/shell-hooks.test.ts +++ b/tests/shell-hooks.test.ts @@ -2553,4 +2553,126 @@ describe('ensure-proxy behavioral tests', () => { ).toBe(false); }); }); + + // ── FIX 4 (issue #313): stale binPath re-resolution ───────────────────────── + // + // When proxy.json exists with enabled:true but the stored binPath is absent + // (e.g., npx cache GC cleared the subswitch package), the hook must attempt + // re-resolution before emitting the "relay binary not found" warning. + // + // Strategy b (command -v subswitch) is the easiest to test in a controlled env: + // create a fake subswitch script in a shadow PATH directory, set binPath to a + // non-existent path, and verify the hook heals silently (exits 0 with no warning). + // + // Strategy a (devflow walk) is exercised implicitly by the presence of a real + // devflow binary + node_modules/subswitch in the running test environment — + // but we can't rely on that structure in CI, so we keep strategy-b tests here. + + describe('FIX 4 — stale binPath re-resolution (issue #313)', () => { + + it('always exits 0 when binPath is stale (regression: no crash)', async () => { + // Base case: stale path + re-resolution fails (no devflow/subswitch in PATH) + // → hook falls through to warning and exits 0 (same as before fix). + writeProxyJson({ + enabled: true, + port: await allocateFreePort(), + binPath: '/this/stale/path/does/not/exist/relay.js', + }); + const result = spawnSync('bash', [PROXY_HOOK], { + input: JSON.stringify(SESSION_INPUT), + env: { + ...process.env, + HOME: homeDir, + PATH: '/usr/bin:/bin', // restrict PATH so devflow/subswitch not found + }, + encoding: 'utf-8', + }); + expect(result.status).toBe(0); // must always exit 0 + }); + + it('emits relay-binary-not-found warning when re-resolution fails', async () => { + writeProxyJson({ + enabled: true, + port: await allocateFreePort(), + binPath: '/stale/relay.js', + }); + const result = spawnSync('bash', [PROXY_HOOK], { + input: JSON.stringify(SESSION_INPUT), + env: { + ...process.env, + HOME: homeDir, + PATH: '/usr/bin:/bin', + }, + encoding: 'utf-8', + }); + expect(result.status).toBe(0); + // Warning emitted for SessionStart + if (result.stdout) { + try { + const parsed = JSON.parse(result.stdout) as Record; + const output = parsed['hookSpecificOutput'] as Record | undefined; + if (output) { + expect((output['additionalContext'] as string)).toContain('[Devflow proxy]'); + } + } catch { + // stdout not JSON (e.g., empty) — still acceptable as long as exit=0 + } + } + }); + + it('heals silently (exits 0, no warning) when strategy-b finds subswitch via PATH', async () => { + // Create a fake subswitch binary in a shadow bin directory + const fakeBinDir = path.join(tmpDir, 'shadow-bin'); + fs.mkdirSync(fakeBinDir, { recursive: true }); + + // The hook will call `subswitch serve` on the resolved binary — we need a + // script that does not actually start a relay (it will exit immediately). + // The hook only spawns in the UserPromptSubmit+port-up path, which doesn't + // trigger here since port is down. So we just need the binary to exist. + const fakeSubswitch = path.join(fakeBinDir, 'subswitch'); + fs.writeFileSync(fakeSubswitch, '#!/bin/sh\nexec true\n'); + fs.chmodSync(fakeSubswitch, '0755'); + + writeProxyJson({ + enabled: true, + port: await allocateFreePort(), + binPath: '/stale/relay.js', // non-existent stored path + }); + + // Build a PATH that includes fakeBinDir (for `command -v subswitch`) plus + // the binaries the hook needs to run (bash, node, cat, etc.) + const hookPath = `/usr/bin:/bin${fakeBinDir ? ':' + fakeBinDir : ''}`; + + const result = spawnSync('bash', [PROXY_HOOK], { + input: JSON.stringify(SESSION_INPUT), + env: { + ...process.env, + HOME: homeDir, + PATH: `${fakeBinDir}:${process.env.PATH}`, + }, + encoding: 'utf-8', + }); + + // After re-resolution, binPath is healed — the hook continues past the + // binPath check. On SessionStart with port down, it tries to spawn the relay. + // The fake subswitch exits immediately → relay fails to start → hook emits a + // different warning (port not accepting) and exits 0. The key assertion is + // that exit code is 0 and the "relay binary not found" warning is NOT emitted. + expect(result.status).toBe(0); + + // Should NOT emit the "relay binary not found" message + if (result.stdout) { + try { + const parsed = JSON.parse(result.stdout) as Record; + const output = parsed['hookSpecificOutput'] as Record | undefined; + if (output) { + const ctx = output['additionalContext'] as string ?? ''; + expect(ctx).not.toContain('relay binary not found'); + } + } catch { + // Not JSON — also acceptable (empty stdout = healed past binPath check) + } + } + }); + }); }); From e3cea2135116bad11f69c998fadf917baa7f24be Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 00:56:37 +0300 Subject: [PATCH 06/17] chore(test): update subswitch version pin to 0.3.0 in packaging guard packaging.test.ts hardcoded SUBSWITCH_VERSION='0.2.0'; the dependency was bumped to 0.3.0 in d087565. Align the guard constant with the current lockfile. --- tests/packaging.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/packaging.test.ts b/tests/packaging.test.ts index dc3d4567..be444d18 100644 --- a/tests/packaging.test.ts +++ b/tests/packaging.test.ts @@ -25,7 +25,7 @@ const ROOT = path.resolve(import.meta.dirname, '..'); * Expected exact-pinned version of the routing runtime. * Hoisted so the next bump is a one-line change. */ -const SUBSWITCH_VERSION = '0.2.0'; +const SUBSWITCH_VERSION = '0.3.0'; // --------------------------------------------------------------------------- // Guard 3: dependency pin integrity From 4a9c32380234d4b20e1452b94e65f9677484a8c2 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 01:02:04 +0300 Subject: [PATCH 07/17] refactor: extract loop-invariant hook obj and drop tombstone comment (#313) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - post-install.ts: hoist existingHooksObj and existing.hooks assignment out of the mergeDevflowSettingsTemplate loop — was re-evaluated on every iteration but always yielded the same reference after the first - ensure-proxy: remove "(moved here from below)" tombstone from node re-resolution comment; describe current state, not the transition --- src/assets/scripts/hooks/ensure-proxy | 2 +- src/targets/claude-code/post-install.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/assets/scripts/hooks/ensure-proxy b/src/assets/scripts/hooks/ensure-proxy index eb245c40..be3125c8 100644 --- a/src/assets/scripts/hooks/ensure-proxy +++ b/src/assets/scripts/hooks/ensure-proxy @@ -194,7 +194,7 @@ if [ -z "$PROXY_BIN" ] || [ ! -f "$PROXY_BIN" ]; then log "prereq: binPath missing or not a file (${PROXY_BIN:-}) — attempting re-resolution" _RESOLVED_BIN="" - # Resolve node early for use in re-resolution (moved here from below) + # Resolve node for re-resolution strategies. _NODE_FOR_RESOLVE=$(command -v node 2>/dev/null || true) # Strategy a: locate devflow CLI → walk up to find node_modules/subswitch diff --git a/src/targets/claude-code/post-install.ts b/src/targets/claude-code/post-install.ts index f951ec20..1846936f 100644 --- a/src/targets/claude-code/post-install.ts +++ b/src/targets/claude-code/post-install.ts @@ -811,9 +811,9 @@ export function mergeDevflowSettingsTemplate( // Merge hook entries — idempotent by exact command string const tmplHooks = (template.hooks ?? {}) as Record; + const existingHooksObj = (existing.hooks as Record | undefined) ?? {}; + existing.hooks = existingHooksObj; for (const [event, matchers] of Object.entries(tmplHooks)) { - const existingHooksObj = (existing.hooks as Record | undefined) ?? {}; - existing.hooks = existingHooksObj; existingHooksObj[event] ??= []; const eventArr = existingHooksObj[event]; for (const matcher of matchers) { From f73c956410412eac931b8e5168591636a4a6cdf5 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 01:17:47 +0300 Subject: [PATCH 08/17] fix(proxy): never gate proxy-hook removal on the env-strip evidence (#313) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The D-STRIP-1 gate was applied to the whole teardown instead of to the env strip alone, and it was missing entirely from `devflow proxy --disable`. - uninstall: with no proxy.json, `removeProxyHooks` stopped running too, so an uninstall could leave SessionStart/UserPromptSubmit entries pointing at a deleted hook script — and a re-run of an interrupted uninstall (proxy.json already removed by the artifact loop) could never clean them up. - `runDisable`: `applyDisableToSettings` still ran unconditionally with the DEFAULT_PROXY_PORT fallback, so `devflow proxy --disable` on a machine where Devflow never managed the proxy deleted a user's own ANTHROPIC_BASE_URL on 4141 plus CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT — the exact hazard #313 set out to fix. Both paths now route through `applyProxyTeardownToSettings(settings, port?)`: hooks always come out, the env strip runs only against a port the caller read from an existing proxy.json. `applyDisableToSettings` keeps its both-operations invariant and is still what runs on the managed path. Also folds the duplicated D-EFR-5 foreign-env block into a shared `checkSettingsEnv` helper, so the adopted-relay path and the free-port path apply the same check ④ — the adopted path was skipping the malformed-JSON refusal and the ANTHROPIC_API_KEY warning. --- src/cli/commands/proxy.ts | 165 ++++++++++++++++++++++------------ src/cli/commands/uninstall.ts | 21 +++-- tests/proxy.test.ts | 99 ++++++++++++++++++++ 3 files changed, 218 insertions(+), 67 deletions(-) diff --git a/src/cli/commands/proxy.ts b/src/cli/commands/proxy.ts index a41f8e80..c1ab9271 100644 --- a/src/cli/commands/proxy.ts +++ b/src/cli/commands/proxy.ts @@ -29,6 +29,7 @@ import { buildProxyState, buildRoutingConfigJson, proxyBaseUrl, + proxyJsonExists, resolveProxyBin, DEFAULT_PROXY_PORT, } from '../../core/proxy-state.js'; @@ -187,10 +188,11 @@ function _stripProxyEnvFromObject(settings: Settings, managedPort: number): bool const env = s.env as Record | undefined; if (!env) return false; - // D-STRIP-1: callers gate this function on devflow-managed evidence (proxy.json exists), - // so by the time we get here we know Devflow wrote this var. Always remove it - // unconditionally — there is no foreign value of this key to protect. Port-scoping - // protects ANTHROPIC_BASE_URL (see below), but not this var. (applies PF-015, ADR-003) + // D-STRIP-1: every REMOVAL caller (init, uninstall, runDisable) first proves Devflow + // managed the proxy — `proxyJsonExists()` — and the enable caller is taking ownership + // of the key anyway, so reaching this line means the value is Devflow's to remove. + // Removal is therefore unconditional: unlike ANTHROPIC_BASE_URL (port-scoped below), + // this key has no foreign value to protect. (applies PF-015, ADR-003) const hadWindowVar = env[UNKNOWN_MODEL_WINDOW_ENV] !== undefined; delete env[UNKNOWN_MODEL_WINDOW_ENV]; @@ -336,6 +338,34 @@ export function applyDisableToSettings(settings: Settings, managedPort: number): return removedHooks || strippedEnv; } +/** + * Settings teardown for every path that turns the proxy off: `devflow proxy --disable` + * and `devflow uninstall`. + * + * D-STRIP-1: the two removals answer to different evidence, so they are gated + * separately. + * - Hooks are Devflow's own artifacts — removed unconditionally. Leaving them behind + * points later sessions at a hook script that no longer exists, and makes a re-run + * of an interrupted uninstall unable to clean up. + * - The env vars (ANTHROPIC_BASE_URL, CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT) + * are stripped only against `managedPort`, which the caller supplies from an existing + * `proxy.json` — the one piece of evidence that Devflow ever wrote them. Pass + * `undefined` when that file is absent: the values then belong to a gateway Devflow + * never managed (a user's own LiteLLM on 4141, say) and must survive untouched. + * + * When `managedPort` is defined this delegates to `applyDisableToSettings`, so the + * both-operations invariant documented there still holds. + * + * Mutates settings in place. Returns true when any change was made. + */ +export function applyProxyTeardownToSettings( + settings: Settings, + managedPort: number | undefined, +): boolean { + if (managedPort === undefined) return removeProxyHooks(settings); + return applyDisableToSettings(settings, managedPort); +} + /** * Check whether the ensure-proxy hook is registered on at least one event. * Returns true if present on either SessionStart or UserPromptSubmit. @@ -411,6 +441,59 @@ export interface PreflightResult { adopted: boolean; } +/** + * Preflight check ④ — settings.json readable and parseable, no foreign + * ANTHROPIC_BASE_URL, plus the non-fatal ANTHROPIC_API_KEY warning. + * + * Extracted so the adopted-relay path and the free-port path run the identical + * check (D-EFR-5): a healthy relay on the port must not buy an exemption from the + * foreign-gateway refusal. Keeping it in one function is what stops the two paths + * from drifting on which sub-checks they apply or which message they return. + * + * `swallowSettingsReadError` semantics are preserved by the caller-supplied dep: + * when that flag is set (init), readSettingsJson() resolves to '{}' on I/O failure + * instead of rejecting, so the read-failure branch is reachable only from runEnable. + */ +async function checkSettingsEnv( + deps: ProxyPreflightDeps, + port: number, +): Promise> { + let settingsJson: string; + try { + settingsJson = await deps.readSettingsJson(); + } catch { + return Err('Could not read settings.json — check file permissions'); + } + + let parsedSettings: Record; + try { + parsedSettings = JSON.parse(settingsJson) as Record; + } catch { + return Err('settings.json is malformed — fix it before enabling the proxy'); + } + + if (readProxyEnvState(settingsJson, port) === 'foreign') { + return Err( + 'An existing ANTHROPIC_BASE_URL in settings.json points to a different gateway — Devflow will not overwrite it', + ); + } + + // API key warning (non-fatal) + const envBlock = parsedSettings.env; + if ( + typeof envBlock === 'object' && + envBlock !== null && + !Array.isArray(envBlock) && + typeof (envBlock as Record).ANTHROPIC_API_KEY === 'string' + ) { + deps.onWarn?.( + 'ANTHROPIC_API_KEY is set in settings.json — requests will use that key through the local relay', + ); + } + + return Ok(undefined); +} + /** * Run preflight checks before enabling the Devflow proxy. * @@ -419,6 +502,8 @@ export interface PreflightResult { * ② ~/.codex/auth.json exists. * ③ Port probe: free → OK; accepting → health check → adopt or fail. * ④ settings.json parseable; ANTHROPIC_BASE_URL not pointing elsewhere; API key warn. + * Runs on BOTH outcomes of ③ that can proceed — adopted relay and free port + * (D-EFR-5) — via the shared `checkSettingsEnv` helper. * * Doctor is deliberately excluded from preflight: the relay's doctor subcommand * probes the relay port — a not-yet-started relay makes that probe fail (exit 1). A @@ -454,26 +539,11 @@ export async function runProxyPreflight( PROBE_TIMEOUT_MS, ); if (healthResult.ok && isOurRelayBody(healthResult.value)) { - // D-EFR-5: evaluate the foreign-env refusal BEFORE the adopted early-return. - // The old order (adopted-return first, foreign-check only when port is free) - // allowed enabling while a foreign ANTHROPIC_BASE_URL was set — any healthy - // relay on the port caused an early-return that silently skipped check ④. - // Now we read settings and refuse if a foreign URL is present, regardless of - // port state. swallowSettingsReadError semantics are preserved: when that flag - // is true, readSettingsJson() returns '{}' on I/O failure instead of throwing, - // so the catch here is only hit in the strict (runEnable) path. - let settingsJsonForForeignCheck: string; - try { - settingsJsonForForeignCheck = await deps.readSettingsJson(); - } catch { - return Err('Could not read settings.json — check file permissions'); - } - const envStateForAdopt = readProxyEnvState(settingsJsonForForeignCheck, port); - if (envStateForAdopt === 'foreign') { - return Err( - 'An existing ANTHROPIC_BASE_URL in settings.json points to a different gateway — Devflow will not overwrite it', - ); - } + // D-EFR-5: run check ④ BEFORE the adopted early-return. The old order + // (adopted-return first, settings check only when the port was free) let a + // healthy relay on the port silently skip the foreign-gateway refusal entirely. + const settingsCheck = await checkSettingsEnv(deps, port); + if (!settingsCheck.ok) return Err(settingsCheck.error); return Ok({ binPath, npxWarning, adopted: true }); } // Port accepting but health timed out, failed, or not our relay @@ -484,39 +554,8 @@ export async function runProxyPreflight( // Port refused — free to proceed // ④ Settings.json check - let settingsJson: string; - try { - settingsJson = await deps.readSettingsJson(); - } catch { - return Err('Could not read settings.json — check file permissions'); - } - - let parsedSettings: Record; - try { - parsedSettings = JSON.parse(settingsJson) as Record; - } catch { - return Err('settings.json is malformed — fix it before enabling the proxy'); - } - - const envState = readProxyEnvState(settingsJson, port); - if (envState === 'foreign') { - return Err( - 'An existing ANTHROPIC_BASE_URL in settings.json points to a different gateway — Devflow will not overwrite it', - ); - } - - // API key warning (non-fatal) - const envBlock = parsedSettings.env; - if ( - typeof envBlock === 'object' && - envBlock !== null && - !Array.isArray(envBlock) && - typeof (envBlock as Record).ANTHROPIC_API_KEY === 'string' - ) { - deps.onWarn?.( - 'ANTHROPIC_API_KEY is set in settings.json — requests will use that key through the local relay', - ); - } + const settingsCheck = await checkSettingsEnv(deps, port); + if (!settingsCheck.ok) return Err(settingsCheck.error); return Ok({ binPath, npxWarning, adopted: false }); } @@ -1730,6 +1769,13 @@ async function runDisable(): Promise { // applyDisableToSettings strips ANTHROPIC_BASE_URL only when the URL // port matches the port Devflow manages — callers must supply it. Reading // proxy.json here also consolidates state for Step 2 below. + // + // D-STRIP-1: readProxyState() cannot distinguish "file absent" from "file present + // with the default port", so the env strip is gated on the file's existence — the + // only evidence that Devflow ever wrote those vars. Must be read before Step 2 + // creates the file. Hook removal is NOT gated: our hooks are ours to remove on + // every path. + const proxyManaged = await proxyJsonExists(devflowDir); const priorStateResult = await readProxyState(devflowDir); const priorState = priorStateResult.ok ? priorStateResult.value : null; const managedPort = priorState?.port ?? DEFAULT_PROXY_PORT; @@ -1751,7 +1797,10 @@ async function runDisable(): Promise { return; } - const changed = applyDisableToSettings(parsedSettings, managedPort); + const changed = applyProxyTeardownToSettings( + parsedSettings, + proxyManaged ? managedPort : undefined, + ); if (changed) { // Guard ENOSPC/EACCES — unhandled rejection leaves proxy in partial state try { diff --git a/src/cli/commands/uninstall.ts b/src/cli/commands/uninstall.ts index 14a161c7..7e7a293b 100644 --- a/src/cli/commands/uninstall.ts +++ b/src/cli/commands/uninstall.ts @@ -15,7 +15,7 @@ import { removeCaptureHooks } from './capture.js'; import { removeDreamHook } from './legacy-hooks.js'; import { removeHudStatusLine } from './hud.js'; import { removeContextHook } from './context.js'; -import { applyDisableToSettings } from './proxy.js'; +import { applyProxyTeardownToSettings } from './proxy.js'; import { readProxyState, proxyJsonExists } from '../../core/proxy-state.js'; import { hudCacheDir } from '../../core/cache.js'; import { revertExternalAgents } from '../../core/agent-models.js'; @@ -717,7 +717,10 @@ export async function runCleanupPhase(opts: { /** Pre-captured managed proxy ports per scope, read BEFORE artifact removal. * proxy.json is deleted by removeDevFlowInstallArtifacts (inside runFullPhaseForScope), * so reading it inside this phase would always yield the DEFAULT_PROXY_PORT fallback - * and leave a non-default ANTHROPIC_BASE_URL (e.g. port 4200) unstripped. (F6) */ + * and leave a non-default ANTHROPIC_BASE_URL (e.g. port 4200) unstripped. (F6) + * + * A scope with no entry means proxy.json did not exist — the env vars are not + * Devflow's to remove and only the hooks come out (D-STRIP-1). */ managedProxyPorts?: ReadonlyMap; }): Promise { const { scopesToUninstall, keepDocs, verbose, cwd, isTTY } = opts; @@ -814,16 +817,16 @@ export async function runCleanupPhase(opts: { settingsContent = stripFlags(settingsContent); // also strips viewMode via view-mode registry entry settingsContent = stripDevflowTeammateModeFromJson(settingsContent); // Remove proxy hooks and ANTHROPIC_BASE_URL env in a single parse-mutate-serialize pass. - // D-STRIP-1: only strip when we have devflow-managed evidence (proxy.json existed - // before artifact removal). managedProxyPorts is populated only when the file was - // present — if the scope has no entry, proxy was never managed and we leave - // ANTHROPIC_BASE_URL / CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT untouched. + // D-STRIP-1: an absent entry in managedProxyPorts means proxy.json did not exist + // before artifact removal — no evidence Devflow ever wrote the env vars, so + // applyProxyTeardownToSettings removes only our hooks and leaves the user's + // ANTHROPIC_BASE_URL / CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT alone. + // Hook removal is never gated: our hooks must not survive an uninstall. // REG-1: the port-scoped URL strip protects any foreign localhost gateway on a // different port — the managedPort value is from the pre-captured proxy.json port. - if (opts.managedProxyPorts?.has(scope)) { - const managedPort = opts.managedProxyPorts.get(scope)!; + { const parsedSettings = JSON.parse(settingsContent) as Settings; - applyDisableToSettings(parsedSettings, managedPort); + applyProxyTeardownToSettings(parsedSettings, opts.managedProxyPorts?.get(scope)); settingsContent = JSON.stringify(parsedSettings, null, 2) + '\n'; } diff --git a/tests/proxy.test.ts b/tests/proxy.test.ts index 68fb2580..e1516a26 100644 --- a/tests/proxy.test.ts +++ b/tests/proxy.test.ts @@ -22,6 +22,7 @@ import { removeProxyHooks, hasProxyHooks, applyDisableToSettings, + applyProxyTeardownToSettings, runProxyPreflight, runPostSpawnVerification, isOurRelayBody, @@ -1850,3 +1851,101 @@ describe('runProxyPreflight — FIX 3 D-EFR-5 foreign-env check ordering (issue expect(readSettingsJson).toHaveBeenCalledTimes(1); }); }); + +// ─── FIX 2 (issue #313): applyProxyTeardownToSettings — D-STRIP-1 gate ──────── +// +// Every proxy-off path (devflow proxy --disable, devflow uninstall) routes its +// settings teardown through this one function so the two removals cannot drift: +// - hooks → always removed (they are Devflow's own artifacts) +// - ANTHROPIC_BASE_URL + window var → removed only when the caller supplies a +// managedPort, which it may do only when proxy.json exists (the sole evidence +// that Devflow ever wrote them). +// +// Falsification: gating the whole call on the evidence (the shape this replaced) +// leaves Devflow's hooks in settings.json after uninstall — REG-TEARDOWN-1 fails. + +describe('applyProxyTeardownToSettings — FIX 2 D-STRIP-1 (issue #313)', () => { + const DEVFLOW_DIR = '/home/test/.devflow'; + + /** Settings in the fully-enabled shape: proxy hooks + both env vars. */ + function enabledSettings(port: number): Settings { + const s = { + env: { + ANTHROPIC_BASE_URL: `http://127.0.0.1:${port}`, + [WINDOW_ENV]: '1', + MY_VAR: 'keep', + }, + } as unknown as Settings; + addProxyHooks(s, DEVFLOW_DIR); + return s; + } + + // REG-TEARDOWN-1: hooks go regardless of evidence — an uninstall that leaves them + // behind points every later session at a hook script that no longer exists. + it('REG-TEARDOWN-1: removes proxy hooks even with no managed port (unmanaged)', () => { + const settings = enabledSettings(DEFAULT_PORT); + expect(hasProxyHooks(settings)).toBe(true); + + const changed = applyProxyTeardownToSettings(settings, undefined); + + expect(changed).toBe(true); + expect(hasProxyHooks(settings)).toBe(false); + }); + + it('leaves both env vars untouched when no managed port is supplied', () => { + const settings = enabledSettings(DEFAULT_PORT); + + applyProxyTeardownToSettings(settings, undefined); + + const env = (settings as unknown as { env: Record }).env; + // Not ours to remove: without proxy.json there is no evidence Devflow wrote these. + expect(env.ANTHROPIC_BASE_URL).toBe(`http://127.0.0.1:${DEFAULT_PORT}`); + expect(env[WINDOW_ENV]).toBe('1'); + expect(env.MY_VAR).toBe('keep'); + }); + + it('removes hooks AND both env vars when a managed port is supplied', () => { + const settings = enabledSettings(DEFAULT_PORT); + + const changed = applyProxyTeardownToSettings(settings, DEFAULT_PORT); + + expect(changed).toBe(true); + expect(hasProxyHooks(settings)).toBe(false); + const env = (settings as unknown as { env?: Record }).env; + expect(env?.ANTHROPIC_BASE_URL).toBeUndefined(); + expect(env?.[WINDOW_ENV]).toBeUndefined(); + expect(env?.MY_VAR).toBe('keep'); + }); + + it('keeps the both-operations invariant: env is stripped even when hooks are present', () => { + // The regression applyDisableToSettings guards against — hooks present must not + // short-circuit the env strip. Re-pinned through the teardown entry point. + const settings = enabledSettings(DEFAULT_PORT); + + applyProxyTeardownToSettings(settings, DEFAULT_PORT); + + const env = (settings as unknown as { env?: Record }).env; + expect(env?.ANTHROPIC_BASE_URL).toBeUndefined(); + expect(env?.[WINDOW_ENV]).toBeUndefined(); + }); + + it('REG-1 preserved: a foreign gateway on another port survives a managed teardown', () => { + const settings = { + env: { ANTHROPIC_BASE_URL: 'http://127.0.0.1:9999' }, + } as unknown as Settings; + addProxyHooks(settings, DEVFLOW_DIR); + + applyProxyTeardownToSettings(settings, DEFAULT_PORT); + + const env = (settings as unknown as { env: Record }).env; + expect(env.ANTHROPIC_BASE_URL).toBe('http://127.0.0.1:9999'); + expect(hasProxyHooks(settings)).toBe(false); + }); + + it('returns false when there is nothing to remove (unmanaged, no hooks)', () => { + const settings = { env: { ANTHROPIC_BASE_URL: 'https://gw.example.com' } } as unknown as Settings; + expect(applyProxyTeardownToSettings(settings, undefined)).toBe(false); + const env = (settings as unknown as { env: Record }).env; + expect(env.ANTHROPIC_BASE_URL).toBe('https://gw.example.com'); + }); +}); From ae57dd3e63a55da5dacf724a3cd7076e81003776 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 01:17:56 +0300 Subject: [PATCH 09/17] fix(proxy): strip routing-config keys the 0.3.0 relay rejects (#313) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The subswitch 0.2.0 -> 0.3.0 bump on this branch retires two keys that were valid under 0.2.0 and are now registered legacy keys — a hard startup error, not a warning: `anthropic.streamIdleTimeoutMs` and `limits.maxConcurrentRequests`. `buildRoutingConfigJson` preserves a user's existing `anthropic` and `limits` blocks wholesale, so anyone who hand-tuned ~/.devflow/proxy-routing.json under 0.2.0 would have had those keys written straight back into the config the 0.3.0 relay reads — the relay refuses to boot, and the ensure-proxy hook just reports "relay failed to start" every session. Both keys join `limits.connectTimeoutMs` in a named ROUTING_CONFIG_REJECTED_SUBKEYS table so the two strip sites cannot drift. The list is scoped to keys a config that worked against the version Devflow actually shipped could contain — keys retired before 0.2.0 are unreachable. Also refreshes the @D-EFR-4 contract comment: in 0.3.0 anthropic.connectTimeoutMs is a genuine DNS+TCP connect budget (armed on the socket, disarmed on 'connect'), not the socket-inactivity timeout that motivated the 120s override under 0.2.0. The `preserves other anthropic fields` fixture used streamIdleTimeoutMs — a shape the pinned runtime now rejects — so it pinned behaviour that would break the relay at startup (avoids PF-043). Retargeted at maxUpstreamSockets. --- src/core/proxy-state.ts | 61 +++++++++++++++++++++++++++------------ tests/proxy-state.test.ts | 35 ++++++++++++++++++++-- 2 files changed, 76 insertions(+), 20 deletions(-) diff --git a/src/core/proxy-state.ts b/src/core/proxy-state.ts index f5a91c65..6b76ed7d 100644 --- a/src/core/proxy-state.ts +++ b/src/core/proxy-state.ts @@ -126,30 +126,50 @@ export async function writeProxyState( // --------------------------------------------------------------------------- /** - * Accepted top-level keys for the subswitch 0.2.0 FileConfigSchema (z.strictObject). + * Accepted top-level keys for the subswitch 0.3.0 FileConfigSchema (z.strictObject). * Unknown top-level keys cause a hard relay startup error — never emit them. * - * @D-EFR-4 Subswitch 0.2.0 routing config contract: + * @D-EFR-4 Subswitch 0.3.0 routing config contract: * FileConfigSchema is a z.strictObject with exactly 5 accepted top-level keys: * port, logLevel, anthropic, providers, limits. Unknown keys cause a hard startup * error — the relay refuses to start. anthropic and limits are themselves * strictObject + prefault({}), so they may be partially specified. * - * connectTimeoutMs default: subswitch 0.2.0 applies anthropic.connectTimeoutMs - * as upstream.setTimeout() at anthropic-passthrough.js:86 — a socket *inactivity* - * timeout, not a connect timeout. The 10s default kills any Anthropic request - * whose upstream takes >10s to emit the first response byte — routine for long - * opus requests (confirmed: 99 spurious 504s clustered at 10004-10098ms while - * successful requests ran 25s-99s). Default overridden to 120 000ms; a - * user-specified value always wins. - * - * limits.connectTimeoutMs is a registered LEGACY KEY that causes a hard startup - * error. Strip it from the limits block when preserving existing config. + * connectTimeoutMs: in 0.3.0 this is a genuine DNS+TCP connect budget, armed on + * the socket and disarmed on 'connect' — neither the headers phase nor the stream + * phase is bounded. The devflow override to 120 000ms dates from 0.2.0, where the + * same key was applied as an upstream socket *inactivity* timeout whose 10s default + * killed any Anthropic request taking >10s to emit its first byte (confirmed: 99 + * spurious 504s clustered at 10004-10098ms while successful requests ran 25s-99s). + * The override is now only a wider connect budget; a user-specified value always wins. */ const ROUTING_CONFIG_ALLOWED_TOP_KEYS = new Set([ 'port', 'logLevel', 'anthropic', 'providers', 'limits', ]); +/** + * Sub-keys of a preserved `anthropic` / `limits` block that the pinned relay rejects + * outright — each is a registered LEGACY KEY, and a legacy key is a hard startup + * error naming its replacement, not a warning. They are stripped when carrying a + * user's existing config forward so an upgrade cannot leave the relay unable to boot. + * + * Every entry was valid under a version devflow previously pinned, which is what makes + * it reachable in a real user's file: + * anthropic.streamIdleTimeoutMs — valid in 0.2.0, removed in 0.3.0 (relay no longer + * bounds the stream-idle phase on a connected client). + * limits.connectTimeoutMs — moved to anthropic.connectTimeoutMs, which this + * builder sets itself, so dropping it loses nothing. + * limits.maxConcurrentRequests — valid in 0.2.0, removed in 0.3.0 (admission gate + * removed). + * + * Keys retired before 0.2.0 are deliberately absent: a config that worked against the + * version devflow shipped cannot contain them. + */ +const ROUTING_CONFIG_REJECTED_SUBKEYS: Readonly> = { + anthropic: ['streamIdleTimeoutMs'], + limits: ['connectTimeoutMs', 'maxConcurrentRequests'], +}; + /** Default anthropic.connectTimeoutMs injected when not specified by the user (ms). */ export const DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS = 120_000; @@ -157,9 +177,9 @@ export const DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS = 120_000; * Build the routing config JSON for ~/.devflow/proxy-routing.json. * * Authoritatively sets `port`. Preserves any existing valid `anthropic`, - * `limits`, `logLevel`, and `providers` blocks from `existingContent`, - * filtering out unknown top-level keys and the legacy `limits.connectTimeoutMs` - * field (which causes a hard relay startup error in 0.2.0). + * `limits`, `logLevel`, and `providers` blocks from `existingContent`, filtering + * out unknown top-level keys and every sub-key in ROUTING_CONFIG_REJECTED_SUBKEYS + * (each one a hard relay startup error). * * Injects a default `anthropic.connectTimeoutMs` of 120 000ms when the user * has not set one — see @D-EFR-4 for the rationale. @@ -204,6 +224,9 @@ export function buildRoutingConfigJson(port: number, existingContent?: string): !Array.isArray(preserved.anthropic) ? { ...(preserved.anthropic as Record) } : {}; + for (const key of ROUTING_CONFIG_REJECTED_SUBKEYS.anthropic) { + delete existingAnthropic[key]; + } if (!Object.prototype.hasOwnProperty.call(existingAnthropic, 'connectTimeoutMs')) { existingAnthropic.connectTimeoutMs = DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS; } @@ -214,8 +237,8 @@ export function buildRoutingConfigJson(port: number, existingContent?: string): config.providers = preserved.providers; } - // Preserve limits block if present; strip legacy limits.connectTimeoutMs that - // causes a hard startup error in 0.2.0 (use anthropic.connectTimeoutMs instead). + // Preserve limits block if present, minus the sub-keys the relay hard-errors on + // (see ROUTING_CONFIG_REJECTED_SUBKEYS). if (preserved.limits !== undefined) { if ( typeof preserved.limits === 'object' && @@ -223,7 +246,9 @@ export function buildRoutingConfigJson(port: number, existingContent?: string): !Array.isArray(preserved.limits) ) { const limitsObj = { ...(preserved.limits as Record) }; - delete limitsObj['connectTimeoutMs']; + for (const key of ROUTING_CONFIG_REJECTED_SUBKEYS.limits) { + delete limitsObj[key]; + } config.limits = limitsObj; } else { config.limits = preserved.limits; diff --git a/tests/proxy-state.test.ts b/tests/proxy-state.test.ts index 47be33eb..f3371f08 100644 --- a/tests/proxy-state.test.ts +++ b/tests/proxy-state.test.ts @@ -269,10 +269,13 @@ describe('buildRoutingConfigJson — existing config preservation', () => { }); it('preserves other anthropic fields alongside injected default', () => { - const existing = JSON.stringify({ port: 4141, anthropic: { streamIdleTimeoutMs: 60_000 } }); + // maxUpstreamSockets is a live key in the pinned runtime's AnthropicSchema — + // a preservation fixture has to use a shape the relay actually accepts, or it + // pins behaviour that would break the relay at startup (avoids PF-043). + const existing = JSON.stringify({ port: 4141, anthropic: { maxUpstreamSockets: 64 } }); const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; const anthropic = obj.anthropic as Record; - expect(anthropic.streamIdleTimeoutMs).toBe(60_000); + expect(anthropic.maxUpstreamSockets).toBe(64); expect(anthropic.connectTimeoutMs).toBe(DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS); }); @@ -297,6 +300,34 @@ describe('buildRoutingConfigJson — existing config preservation', () => { expect(limits.maxConcurrent).toBe(10); }); + // Keys that were valid under the previously pinned 0.2.0 runtime and are registered + // legacy keys in 0.3.0 — a hard startup error, not a warning. Carrying a user's own + // proxy-routing.json forward across the upgrade must drop them, or the relay that the + // ensure-proxy hook spawns dies on boot every session with no route back. + it('strips anthropic.streamIdleTimeoutMs (removed in the pinned runtime)', () => { + const existing = JSON.stringify({ + port: 4141, + anthropic: { streamIdleTimeoutMs: 60_000, maxUpstreamSockets: 64 }, + }); + const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; + const anthropic = obj.anthropic as Record; + expect(Object.prototype.hasOwnProperty.call(anthropic, 'streamIdleTimeoutMs')).toBe(false); + // Neighbouring valid keys survive the strip. + expect(anthropic.maxUpstreamSockets).toBe(64); + expect(anthropic.connectTimeoutMs).toBe(DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS); + }); + + it('strips limits.maxConcurrentRequests (removed in the pinned runtime)', () => { + const existing = JSON.stringify({ + port: 4141, + limits: { maxConcurrentRequests: 8, maxConcurrent: 10 }, + }); + const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; + const limits = obj.limits as Record; + expect(Object.prototype.hasOwnProperty.call(limits, 'maxConcurrentRequests')).toBe(false); + expect(limits.maxConcurrent).toBe(10); + }); + it('preserves providers block from existing config', () => { const existing = JSON.stringify({ port: 4141, providers: { openai: { baseUrl: 'https://api.openai.com' } } }); const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; From 83db71e5099ac8f2f891963909fe56811c56a657 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 01:18:05 +0300 Subject: [PATCH 10/17] fix(post-install): drop the vestigial settings prompt, guard merge shapes (#313) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The confirm in `installSettings` was harmless while the old `hasHooks` check made it nearly unreachable. With the template merge it fires on every re-init for anyone with a feature turned off (HUD off -> no statusLine, memory off -> no memory hooks), and it fires from inside init's running spinner — the same place init deliberately calls `s.stop()` before prompting for sudo. Declining protected nothing either: init's own settings pass rewrites the whole hook set immediately afterwards. A prompt whose answer changes nothing is worse than no prompt, so the merge (additive only) now runs silently in both TTY and non-TTY. This also removes a `process.exit(0)` that would have fired under that spinner. `mergeDevflowSettingsTemplate` also walked a hand-editable file's shape unguarded: `hooks: []` silently swallowed the devflow entries (JSON.stringify drops keys attached to an array), and a matcher without a `hooks` array threw, which the outer catch turned into "could not configure settings" — hooks never installed. Every branch now validates shape at the sink and leaves foreign shapes untouched (applies PF-023). An empty `hooks` key is no longer introduced into a settings.json that had none. --- src/targets/claude-code/post-install.ts | 83 +++++++++++++++---------- tests/post-install-merge.test.ts | 53 ++++++++++++++++ tests/shell-hooks.test.ts | 7 +-- 3 files changed, 106 insertions(+), 37 deletions(-) diff --git a/src/targets/claude-code/post-install.ts b/src/targets/claude-code/post-install.ts index 1846936f..83650e0f 100644 --- a/src/targets/claude-code/post-install.ts +++ b/src/targets/claude-code/post-install.ts @@ -7,7 +7,6 @@ import { getManagedSettingsPath } from './claude-paths.js'; import { getGitignoreEntries, getDocsDir } from '../../core/project-paths.js'; import { writeFileAtomicExclusive } from '../../core/fs-atomic.js'; import type { SecurityMode } from '../../core/manifest.js'; -import type { HookMatcher } from './hooks.js'; /** * Type guard for Node.js system errors with error codes. @@ -792,6 +791,23 @@ export async function stripUserSecurityDenyList( return { removed }; } +/** True for a non-null, non-array object literal. */ +function isPlainObject(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** + * Every `command` string carried by a hook matcher, tolerating foreign shapes. + * Returns an empty array for anything that is not `{ hooks: [{ command: string }] }`. + */ +function hookCommandsOf(matcher: unknown): string[] { + if (!isPlainObject(matcher) || !Array.isArray(matcher.hooks)) return []; + return matcher.hooks + .filter(isPlainObject) + .map((h) => h.command) + .filter((c): c is string => typeof c === 'string'); +} + /** * Merge Devflow's template hook entries and top-level fields into an existing * parsed settings object. Idempotent by exact command string — a hook that is @@ -801,6 +817,11 @@ export async function stripUserSecurityDenyList( * D-SETTINGS-1: merge strategy — never replace; only add devflow entries that are absent. * Returns { changed: true } when any field was added. * + * `existing` comes from a hand-editable file, so every branch is shape-guarded: + * a `hooks` value (or per-event value) that is not the expected object/array shape + * is left untouched rather than overwritten or thrown on (applies PF-023 — validate + * at the sink that mutates). + * * Exported for testing. */ export function mergeDevflowSettingsTemplate( @@ -810,21 +831,31 @@ export function mergeDevflowSettingsTemplate( let changed = false; // Merge hook entries — idempotent by exact command string - const tmplHooks = (template.hooks ?? {}) as Record; - const existingHooksObj = (existing.hooks as Record | undefined) ?? {}; - existing.hooks = existingHooksObj; - for (const [event, matchers] of Object.entries(tmplHooks)) { - existingHooksObj[event] ??= []; - const eventArr = existingHooksObj[event]; - for (const matcher of matchers) { - const cmd = matcher.hooks[0]?.command; - if (!cmd) continue; - const alreadyPresent = eventArr.some((m) => m.hooks.some((h) => h.command === cmd)); - if (!alreadyPresent) { + const tmplHooks = isPlainObject(template.hooks) ? template.hooks : {}; + const existingHooksRaw = existing.hooks; + if (existingHooksRaw === undefined || isPlainObject(existingHooksRaw)) { + const existingHooks: Record = existingHooksRaw ?? {}; + let hooksChanged = false; + for (const [event, matchers] of Object.entries(tmplHooks)) { + if (!Array.isArray(matchers)) continue; + for (const matcher of matchers) { + const cmd = hookCommandsOf(matcher)[0]; + if (!cmd) continue; + const current = existingHooks[event]; + if (current !== undefined && !Array.isArray(current)) break; // foreign shape — leave the event alone + const eventArr: unknown[] = current ?? []; + if (eventArr.some((m) => hookCommandsOf(m).includes(cmd))) continue; eventArr.push(matcher); - changed = true; + existingHooks[event] = eventArr; + hooksChanged = true; } } + // Attach only when something was actually added — never introduce an empty + // `hooks` key into a settings.json that had none. + if (hooksChanged) { + existing.hooks = existingHooks; + changed = true; + } } // Set statusLine only if the user has none @@ -852,6 +883,12 @@ export function mergeDevflowSettingsTemplate( * Preserves every user key (env, permissions, apiKeyHelper, model, etc.) untouched. * - Parse failure: warn and skip; file left byte-identical (never clobber a broken file). * + * The merge is additive only, so it runs without a confirmation prompt: init is called + * from inside an active spinner (a prompt would render on top of it), and declining + * could not protect the file anyway — init's own settings pass rewrites the hook set + * unconditionally right afterwards. A prompt whose answer changes nothing is worse + * than no prompt. + * * The deny list is handled by init's dedicated security step * (applyUserSecurityDenyList / installManagedSettings) after installSettings completes. */ @@ -910,26 +947,6 @@ export async function installSettings( return; } - // Settings need Devflow hooks added. - // In TTY mode: ask before writing so the user knows what's happening. - // In non-TTY mode: merge silently (operation is non-destructive — D-SETTINGS-1). - if (process.stdin.isTTY) { - const confirmed = await p.confirm({ - message: 'settings.json exists without some Devflow hooks. Add Working Memory hooks and HUD configuration?', - initialValue: true, - }); - - if (p.isCancel(confirmed)) { - p.cancel('Installation cancelled.'); - process.exit(0); - } - - if (!confirmed) { - p.log.info('Keeping existing settings unchanged'); - return; - } - } - await writeFileAtomicExclusive(settingsPath, JSON.stringify(existingParsed, null, 2) + '\n'); if (verbose) { p.log.success('Settings updated with Devflow hooks and HUD'); diff --git a/tests/post-install-merge.test.ts b/tests/post-install-merge.test.ts index 13a621f6..ef1308dc 100644 --- a/tests/post-install-merge.test.ts +++ b/tests/post-install-merge.test.ts @@ -219,6 +219,59 @@ describe('mergeDevflowSettingsTemplate — FIX 1 (issue #313)', () => { expect(JSON.stringify(existing)).toBe(snapshotAfterFirst); }); + // ── Shape guards (PF-023): `existing` is a hand-editable file ────────────── + // + // The merge mutates a user-authored object, so every branch validates shape at + // the sink. A settings.json that is valid JSON but structurally odd must neither + // throw (init would swallow it and skip hook installation entirely) nor lose the + // user's own entries. + + it('leaves a non-object hooks value untouched instead of throwing', () => { + const existing: Record = { hooks: 'not-an-object' }; + const template = makeTemplate(['/devflow/scripts/hooks/run-hook memory-worker']); + expect(() => mergeDevflowSettingsTemplate(existing, template)).not.toThrow(); + expect(existing.hooks).toBe('not-an-object'); + }); + + it('leaves an array hooks value untouched instead of writing keys onto it', () => { + const existing: Record = { hooks: [] }; + const template: Record = { + hooks: { SessionStart: [makeHookMatcher('/devflow/scripts/hooks/run-hook memory-worker')] }, + }; + const { changed } = mergeDevflowSettingsTemplate(existing, template); + // JSON.stringify would silently drop keys attached to an array — never touch it. + expect(Array.isArray(existing.hooks)).toBe(true); + expect((existing.hooks as unknown[]).length).toBe(0); + expect(changed).toBe(false); + }); + + it('leaves a non-array per-event value untouched instead of throwing', () => { + const existing: Record = { hooks: { SessionStart: 'oops' } }; + const template = makeTemplate(['/devflow/scripts/hooks/run-hook memory-worker']); + expect(() => mergeDevflowSettingsTemplate(existing, template)).not.toThrow(); + expect((existing.hooks as Record).SessionStart).toBe('oops'); + }); + + it('tolerates malformed existing matcher entries when deduping', () => { + const cmd = '/devflow/scripts/hooks/run-hook memory-worker'; + const existing: Record = { + hooks: { SessionStart: ['a-string', { noHooksKey: true }, null] }, + }; + const template = makeTemplate([cmd]); + expect(() => mergeDevflowSettingsTemplate(existing, template)).not.toThrow(); + const arr = (existing.hooks as Record).SessionStart; + // Foreign entries survive; the devflow hook is appended after them. + expect(arr.length).toBe(4); + expect(arr[0]).toBe('a-string'); + }); + + it('does not introduce an empty hooks key when the template adds no hooks', () => { + const existing: Record = { model: 'claude-opus-4-5' }; + const template: Record = { statusLine: 's' }; + mergeDevflowSettingsTemplate(existing, template); + expect('hooks' in existing).toBe(false); + }); + it('preserves existing hook order — devflow hooks appended, not prepended', () => { const existingCmd = '/user/custom-hook'; const devflowCmd = '/devflow/scripts/hooks/run-hook memory-worker'; diff --git a/tests/shell-hooks.test.ts b/tests/shell-hooks.test.ts index ada2f360..f5b99f82 100644 --- a/tests/shell-hooks.test.ts +++ b/tests/shell-hooks.test.ts @@ -2639,10 +2639,9 @@ describe('ensure-proxy behavioral tests', () => { binPath: '/stale/relay.js', // non-existent stored path }); - // Build a PATH that includes fakeBinDir (for `command -v subswitch`) plus - // the binaries the hook needs to run (bash, node, cat, etc.) - const hookPath = `/usr/bin:/bin${fakeBinDir ? ':' + fakeBinDir : ''}`; - + // PATH must find the fake subswitch (for `command -v subswitch`) as well as + // every binary the hook itself needs (bash, node, cat, ...), so prepend the + // shadow dir to the inherited PATH rather than replacing it. const result = spawnSync('bash', [PROXY_HOOK], { input: JSON.stringify(SESSION_INPUT), env: { From 0d118086bce9fb077af922bb0b62ab73cd2d2468 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 01:29:48 +0300 Subject: [PATCH 11/17] test(post-install): pin installSettings parse-failure and wiring behavior (#313) Add two tmpdir integration tests to tests/post-install-merge.test.ts: (a) parse-failure bail: write invalid JSON settings.json (trailing comma), call installSettings, assert file bytes identical and no .tmp.* residue. (b) merge wiring: write valid settings.json with env+permissions but no hooks, call installSettings, assert hooks are added and env/permissions survive byte-for-byte. Also correct a stale comment in src/cli/commands/uninstall.ts:1058 that still named applyDisableToSettings for a call that is now applyProxyTeardownToSettings. Co-Authored-By: Claude --- src/cli/commands/uninstall.ts | 2 +- tests/post-install-merge.test.ts | 79 +++++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/cli/commands/uninstall.ts b/src/cli/commands/uninstall.ts index 7e7a293b..66245d12 100644 --- a/src/cli/commands/uninstall.ts +++ b/src/cli/commands/uninstall.ts @@ -1055,7 +1055,7 @@ export const uninstallCommand = new Command('uninstall') // // D-STRIP-1: only populate managedProxyPorts when proxy.json exists — evidence that // Devflow has managed the proxy on this machine. runCleanupPhase gates the - // applyDisableToSettings call on the port being present in the map, so when the file + // applyProxyTeardownToSettings call on the port being present in the map, so when the file // is absent (never managed), no env strip is attempted and a user's own gateway vars // are left untouched. const managedProxyPorts = new Map(); diff --git a/tests/post-install-merge.test.ts b/tests/post-install-merge.test.ts index ef1308dc..223ee4bd 100644 --- a/tests/post-install-merge.test.ts +++ b/tests/post-install-merge.test.ts @@ -18,9 +18,16 @@ * - Empty template → changed:false */ -import { describe, it, expect } from 'vitest'; -import { mergeDevflowSettingsTemplate } from '../src/targets/claude-code/post-install.js'; +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mergeDevflowSettingsTemplate, installSettings } from '../src/targets/claude-code/post-install.js'; import type { HookMatcher } from '../src/targets/claude-code/hooks.js'; +import * as os from 'node:os'; +import * as fsp from 'node:fs/promises'; +import * as path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +// Resolved repo root — installSettings needs it to locate the settings template. +const REPO_ROOT = path.join(path.dirname(fileURLToPath(import.meta.url)), '..'); // ─── Helpers ───────────────────────────────────────────────────────────────── @@ -285,3 +292,71 @@ describe('mergeDevflowSettingsTemplate — FIX 1 (issue #313)', () => { expect(hooks[1]?.hooks[0]?.command).toBe(devflowCmd); // devflow hook appended }); }); + +// ─── installSettings — parse-failure and merge wiring ──────────────────────── +// +// These tests exercise installSettings(claudeDir, rootDir, devflowDir, verbose) +// against the real filesystem via real tmpdirs. They complement the pure +// mergeDevflowSettingsTemplate unit tests above by pinning the I/O wiring and +// the parse-failure bail path (post-install.ts:926-935). + +describe('installSettings — parse-failure and wiring (issue #313)', () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await fsp.mkdtemp(path.join(os.tmpdir(), 'devflow-settings-test-')); + }); + + afterEach(async () => { + await fsp.rm(tmpDir, { recursive: true, force: true }); + }); + + it('(a) leaves settings.json byte-identical on invalid JSON and leaves no .tmp.* residue', async () => { + // Arrange — write a syntactically invalid JSON (trailing comma before closing brace) + const invalidJson = '{ "hooks": { "SessionStart": [] } , }'; + const settingsPath = path.join(tmpDir, 'settings.json'); + await fsp.writeFile(settingsPath, invalidJson, 'utf-8'); + const bytesBefore = await fsp.readFile(settingsPath); + + // Act — parse failure must bail without touching the file + await installSettings(tmpDir, REPO_ROOT, '/fake/devflow', false); + + // Assert — file bytes are identical (no byte was changed) + const bytesAfter = await fsp.readFile(settingsPath); + expect(bytesAfter.equals(bytesBefore)).toBe(true); + + // Assert — writeFileAtomicExclusive writes to a `.tmp.` sibling then renames; + // on the parse-failure path it must never be created. + const entries = await fsp.readdir(tmpDir); + const tmpFiles = entries.filter((e) => e.includes('.tmp.')); + expect(tmpFiles).toHaveLength(0); + }); + + it('(b) adds hooks while preserving env and permissions blocks byte-for-byte (wiring pin)', async () => { + // Arrange — valid settings with env + permissions but no hooks + const existingSettings: Record = { + env: { ANTHROPIC_BASE_URL: 'https://my-gateway.example.com', MY_VAR: 'keep-me' }, + permissions: { allow: ['Read(*)'], deny: ['Bash(rm -rf *)'] }, + }; + const settingsPath = path.join(tmpDir, 'settings.json'); + await fsp.writeFile(settingsPath, JSON.stringify(existingSettings, null, 2) + '\n', 'utf-8'); + + // Act — installSettings should merge the template hooks in + await installSettings(tmpDir, REPO_ROOT, '/fake/devflow', false); + + // Assert — hooks were added (template carries hooks for SessionStart, Stop, etc.) + const result = JSON.parse(await fsp.readFile(settingsPath, 'utf-8')) as Record; + expect(result.hooks).toBeDefined(); + const hooks = result.hooks as Record; + const hookEventCount = Object.values(hooks).filter( + (arr) => Array.isArray(arr) && arr.length > 0, + ).length; + expect(hookEventCount).toBeGreaterThan(0); + + // Assert — env block survived byte-for-byte + expect(result.env).toEqual(existingSettings.env); + + // Assert — permissions block survived byte-for-byte + expect(result.permissions).toEqual(existingSettings.permissions); + }); +}); From da49bff304b7dd7c0ae9954e41c2bf52c41e09c9 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 01:48:19 +0300 Subject: [PATCH 12/17] docs(knowledge): update external-model-routing feature knowledge base Reflects proxy lifecycle changes from issue #313 / PR #314: proxyJsonExists() evidence discriminator, applyProxyTeardownToSettings unified teardown, D-STRIP-1 init.ts env-strip gating, checkSettingsEnv D-EFR-5 shared helper, ensure-proxy binPath re-resolution, post-install mergeDevflowSettingsTemplate merge strategy, and subswitch 0.3.0 routing config contract. --- .../external-model-routing/KNOWLEDGE.md | 122 +++++++++++++----- .devflow/features/index.md | 2 +- 2 files changed, 94 insertions(+), 30 deletions(-) diff --git a/.devflow/features/external-model-routing/KNOWLEDGE.md b/.devflow/features/external-model-routing/KNOWLEDGE.md index a5a9e202..f6b17607 100644 --- a/.devflow/features/external-model-routing/KNOWLEDGE.md +++ b/.devflow/features/external-model-routing/KNOWLEDGE.md @@ -1,11 +1,11 @@ --- feature: external-model-routing name: External Model Routing & Per-Agent Model Config -description: "Use when working on the proxy lifecycle (enable/disable/status/preflight), the ensure-proxy hook, per-agent model mapping, agent frontmatter rewriting, or the agents TUI. Keywords: proxy, external-model-routing, GPT, agent-models, ensure-proxy, frontmatter, devflow proxy, devflow agents, subswitch, ANTHROPIC_BASE_URL, CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT, dormancy, reapplyAgentMapping, runTui, flags-view, tui." +description: "Use when working on the proxy lifecycle (enable/disable/status/preflight), the ensure-proxy hook, per-agent model mapping, agent frontmatter rewriting, or the agents TUI. Keywords: proxy, external-model-routing, GPT, agent-models, ensure-proxy, frontmatter, devflow proxy, devflow agents, subswitch, ANTHROPIC_BASE_URL, CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT, dormancy, reapplyAgentMapping, runTui, flags-view, tui, proxyJsonExists, applyProxyTeardownToSettings, D-STRIP-1, mergeDevflowSettingsTemplate." category: architecture directories: [src/core/proxy-state.ts, src/core/external-models.ts, src/core/agent-models.ts, src/core/agent-state.ts, src/core/agent-frontmatter.ts, src/core/codex-auth-inspect.ts, src/core/model-discovery.ts, src/core/cache.ts, src/core/proxy-log.ts, src/cli/commands/proxy.ts, src/cli/commands/agents.ts, src/cli/agents-view, src/cli/tui, src/assets/scripts/hooks/ensure-proxy] created: 2026-07-24 -updated: 2026-08-25 +updated: 2026-09-01 --- # External Model Routing & Per-Agent Model Config @@ -18,7 +18,7 @@ Two authority sources govern the proxy at different points in its lifecycle. `ma ## System Context -The routing runtime is an internal package (`subswitch@0.2.0`, exact-pinned in `package.json`). Its name is a **hard branding constraint** — it must never appear in user-visible strings, error messages, CLI output, or agent context injections. User-facing vocabulary is always "external model routing" / "Devflow proxy". The one exception is internal code: health-check body comparisons (`body['name'] === 'subswitch'`), `SUBSWITCH_CONFIG` env var, and hook log lines are fine. +The routing runtime is an internal package (`subswitch@0.3.0`, exact-pinned in `package.json`). Its name is a **hard branding constraint** — it must never appear in user-visible strings, error messages, CLI output, or agent context injections. User-facing vocabulary is always "external model routing" / "Devflow proxy". The one exception is internal code: health-check body comparisons (`body['name'] === 'subswitch'`), `SUBSWITCH_CONFIG` env var, and hook log lines are fine. ## Proxy Lifecycle @@ -27,15 +27,17 @@ The routing runtime is an internal package (`subswitch@0.2.0`, exact-pinned in ` | File | Role | |------|------| | `~/.devflow/proxy.json` | Runtime authority. Tolerant-parsed by `readProxyState()`. ENOENT → default disabled state (not an error). Fields: `enabled`, `port`, `binPath`, `configPath`, `resolvedAt`, `devflowVersion`. | -| `~/.devflow/proxy-routing.json` | Routing config written by `buildRoutingConfigJson(port)`. Shape: bare `{port}` plus trailing newline. 0.2.0 rejects unrecognised keys — including a `codex` block breaks the runtime. Written before preflight runs on enable. | +| `~/.devflow/proxy-routing.json` | Routing config written by `buildRoutingConfigJson(port, existingContent?)`. Strict 5-key shape accepted by subswitch 0.3.0: `port`, `logLevel`, `anthropic`, `providers`, `limits`. Always injects `anthropic.connectTimeoutMs: 120000` when absent (D-EFR-4). Strips 0.2.0-era sub-keys that are hard startup errors in 0.3.0: `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, `limits.maxConcurrentRequests` (see `ROUTING_CONFIG_REJECTED_SUBKEYS`). Written before preflight runs on enable. | | `manifest.features.proxy` | Init/uninstall authority. Seeds from prior manifest on re-init (ADR-014). Never in `config.json` — manifest-group by design, same as `ambient`/`hud`/`rules`. | **`isProxyEnabled()` is the sole dormancy authority**: it reads only `proxy.json` (never manifest). This is load-bearing: on preflight failure, `init.ts` converges `proxy.json` to `enabled:false` alongside manifest, hooks, and env — all four artifacts must agree (avoids PF-015). Any path that forces `proxyEnabled=false` must write `proxy.json enabled:false` so that a subsequent `isProxyEnabled()` call in the same process returns false correctly. +**`proxyJsonExists()` is the evidence discriminator** (D-STRIP-1): `readProxyState()` returns `Ok(defaultState)` on ENOENT — it cannot distinguish "file absent" from "file present with `DEFAULT_PROXY_PORT`". Callers that must gate on Devflow-managed evidence (init.ts env strip, uninstall cleanup phase) use `proxyJsonExists()` instead of inferring file presence from `readProxyState()`. + ### Enable path (crash-safe) 1. Read `proxy.json` for the remembered port; `resolvePort(portOption, priorPort)` picks the effective port. `--port` has **no commander default** — omission leaves `portOption` as `undefined` and the remembered port from `proxy.json` wins (TS-1 fix). -2. Write `proxy-routing.json` with the effective port (bare `{port}` JSON, no model list). +2. Write `proxy-routing.json` with the effective port via `buildRoutingConfigJson(port, existingContent)`, which reads the existing file and merges user customisations (logLevel, providers, anthropic sub-keys) while stripping rejected 0.3.0 sub-keys and injecting the connectTimeoutMs default. 3. Run `runProxyPreflight()` (4 ordered checks — ①–④: bin, codex auth, port probe/adoption, settings — see Preflight section). Doctor excluded: a pre-spawn gate is always unsatisfiable on a cold path (D-EFR-2; see Anti-Patterns). 4. On success: write `proxy.json` `enabled:true`. 5. Spawn relay via `spawnRelayAndWaitForPort()` (exported): bounded ≤50×100ms probe loop (5s max). `SpawnRelayResult.spawnedPid` is set when this process spawned the relay; absent on the adopted path. If relay never accepts, `rollbackProxyState` closure writes `proxy.json enabled:false` and returns error. @@ -51,7 +53,7 @@ Hard failures at any step (steps 1–9) set `process.exitCode = 1` and return The relay process is intentionally left running on `--disable` for any live Claude Code sessions. The disable path: 1. Read `proxy.json` first to determine `managedPort` for the URL strip. -2. `applyDisableToSettings(parsedSettings, managedPort)` — removes hooks AND strips `ANTHROPIC_BASE_URL` (port-scoped) and `CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT` (unconditional) (see invariant below). +2. `applyProxyTeardownToSettings(parsedSettings, managedPort)` — the unified teardown point: removes hooks always; when `managedPort` is defined delegates to `applyDisableToSettings` (hooks + port-scoped URL strip + unconditional window-var strip). 3. Writes `proxy.json` `enabled:false` — **keeps** `port`, `binPath`, `configPath`, `resolvedAt`, `devflowVersion` for the next enable. 4. Syncs manifest to `proxy: false`. 5. `revertExternalAgents()` — rewrites installed agent files to shipped default models. @@ -59,6 +61,30 @@ The relay process is intentionally left running on `--disable` for any live Clau Hard failures (e.g., malformed `settings.json`) set `process.exitCode = 1` and return early. +### `applyProxyTeardownToSettings` — unified teardown point + +```typescript +// Single teardown decision point for disable and uninstall. +// D-STRIP-1: hooks are always removed (Devflow's own artifacts); +// env vars are stripped only when managedPort is defined — the caller +// derives managedPort from an existing proxy.json, which is evidence +// that Devflow wrote those vars. When proxy.json is absent, passing +// undefined skips the env strip rather than clobbering a foreign gateway. +export function applyProxyTeardownToSettings( + settings: Settings, + managedPort: number | undefined, +): boolean { + if (managedPort === undefined) return removeProxyHooks(settings); + return applyDisableToSettings(settings, managedPort); +} +``` + +Uninstall's cleanup phase calls `proxyJsonExists()` to decide whether to pass a port or `undefined`: +- `proxy.json` present → read `proxy.json.port`, pass it as `managedPort` (strip env). +- `proxy.json` absent → pass `undefined` (hooks only). + +`applyDisableToSettings`'s **both-operations invariant** is unchanged when called via the managed path: `removeProxyHooks(s)` and `_stripProxyEnvFromObject(s, port)` both always evaluate — no short-circuit (avoids PF-015). + ### `applyDisableToSettings` — both-operations invariant ```typescript @@ -85,11 +111,17 @@ The regression that this guards against: `removeProxyHooks(s) || _stripProxyEnvF ④ readSettingsJson parseable; ANTHROPIC_BASE_URL not 'foreign'; API key warn (non-fatal) ``` +**Check ④ is extracted as `checkSettingsEnv(deps, port)` (D-EFR-5)**, shared by the adopted-relay path and the free-port path. The old ordering ran ④ only on the free-port branch — a healthy relay on the target port was erroneously buying an exemption from the foreign-gateway refusal. Now both branches call `checkSettingsEnv` before any early-return on adoption. + `spawnDoctor` is on `ProxyPreflightDeps` and built by `buildRealPreflightDeps` so `runEnable` can pass it into `runPostSpawnVerification` (step 6) via the same deps instance (`spawnDoctor: preflightDeps.spawnDoctor`). Preflight itself never calls `spawnDoctor`. **Init never runs doctor and never spawns.** `devflow init` calls `runProxyPreflight` (the same 4-check function) then writes `proxy.json enabled:true`. The relay is started by the first session's `ensure-proxy` hook. Deeper diagnostics (doctor, spawn) live in `devflow proxy --enable` and `devflow proxy --status`. -All four checks are injectable via `ProxyPreflightDeps`. **`buildRealPreflightDeps(opts)`** (exported) builds the production implementation and is shared between `runEnable` and `init.ts`. Key option: `swallowSettingsReadError: true` for init.ts (which writes `settings.json` itself); `false` for `runEnable` (propagates read errors to the user). +All four checks are injectable via `ProxyPreflightDeps`. **`buildRealPreflightDeps(opts)`** (exported) builds the production implementation and is shared between `runEnable` and `init.ts`. Key option: `swallowSettingsReadError: true` for init.ts (which writes `settings.json` itself); `false` for `runEnable` (propagates read errors to the user). `swallowSettingsReadError` semantics are preserved by the caller-supplied dep: when set, `readSettingsJson()` resolves to `'{}'` on I/O failure instead of rejecting. + +### init.ts proxy strip gating (D-STRIP-1) + +`init.ts` gates the proxy env strip on `proxyJsonExists()` — not on the state returned by `readProxyState()`. The guard is explicit in the source as a `D-STRIP-1` comment. When the file is absent on a machine that never had the proxy, no env stripping runs and no foreign gateway is clobbered. When the file is present, the port from `proxy.json` is passed to `_stripProxyEnvFromObject` as `managedPort`. ### Exported seams in proxy.ts @@ -101,7 +133,8 @@ All four checks are injectable via `ProxyPreflightDeps`. **`buildRealPreflightDe | `resolvePort(portOption, priorPort)` | Port resolution with remembered-port fallback | | `isOurRelayBody(body)` | Health-check identity check (`name === 'subswitch'`) | | `applyProxyEnv`, `stripProxyEnv` | Settings JSON string transforms (pure, no mutation) | -| `applyDisableToSettings` | Unconditional hooks-remove + URL-strip on parsed Settings object | +| `applyDisableToSettings` | Both-operations invariant: unconditional hooks-remove + URL-strip on parsed Settings object | +| `applyProxyTeardownToSettings` | Unified teardown for disable + uninstall: hooks always removed; env stripped only when `managedPort` defined (D-STRIP-1) | | `addProxyHooks`, `removeProxyHooks`, `hasProxyHooks` | Hook mutation helpers | | `runProxyPreflight`, `ProxyPreflightDeps`, `PreflightResult` | Preflight contract (4 checks) | | `PostSpawnDoctorDeps` | Injectable interface for post-spawn doctor verification | @@ -111,7 +144,7 @@ All four checks are injectable via `ProxyPreflightDeps`. **`buildRealPreflightDe | `formatExternalModelsLine(catalog, logPath)` | External models `--status` line; renders selectable model names or `unavailable` with the log path | | `realHttpGet(url, timeoutMs)` | Exported HTTP GET with three bounds: wall-clock deadline, 64KB body cap, `res.on('error')` handler. | -Internal named functions (not exported): `applyEnableSettingsPass`, `resolveProcessState`, `formatProcessLine`, `readPidFile`, `PROBE_TIMEOUT_MS`, `DOCTOR_TIMEOUT_MS`, `RELAY_SPAWN_*` constants. +Internal named functions (not exported): `applyEnableSettingsPass`, `checkSettingsEnv`, `resolveProcessState`, `formatProcessLine`, `readPidFile`, `PROBE_TIMEOUT_MS`, `DOCTOR_TIMEOUT_MS`, `RELAY_SPAWN_*` constants. ## ensure-proxy Hook Contract @@ -129,7 +162,7 @@ esac | UserPromptSubmit | any | **immediate exit 0 before any proxy-state reads** (silent; SessionStart handles all state) | | SessionStart | UP + correct identity | exit 0, no output | | SessionStart | UP + wrong identity | exit 0 + `json_session_output` warning ("port occupied by another application") | -| SessionStart | DOWN + missing bin/config | exit 0 + `json_session_output` warning ("relay binary not found" / "routing config not found") | +| SessionStart | DOWN + missing bin/config after re-resolution fails | exit 0 + `json_session_output` warning ("relay binary not found" / "routing config not found") | | SessionStart | DOWN + prerequisites ok | acquire spawn lock → nohup spawn → write `proxy.pid` (best-effort) → wait 80×0.1s = 8s → exit 0 [+warning if never up] | **`proxy.pid` is written immediately after spawn (best-effort)**, mirroring the CLI enable path. `devflow proxy --status` reads this file to display the process line for hook-started relays. A stale pid from a relay that never came up is harmless — `--status` liveness-checks it before display (`process.kill(pid, 0)`). @@ -138,6 +171,12 @@ esac **binPath/configPath are read only in the SessionStart-down branch** — deferred so the enabled+port check path pays zero additional json_field_file cost. +**binPath re-resolution on stale/missing path (D-FIX4)**: when the persisted `binPath` is absent or the file no longer exists (e.g., npx cache GC, devflow upgrade), the hook attempts two resolution strategies before emitting a warning: +- **Strategy (a)**: bounded walk from the resolved `devflow` CLI up to its nearest `node_modules/subswitch/package.json` ancestor. Walk guard: `_WALK_GUARD < 6` — at most 6 `dirname` steps. The `bin` field is read via `node -p` (env-var pass avoids quoting issues). Stops at the first `subswitch` dir found regardless of bin result. +- **Strategy (b)**: `command -v subswitch` — for globally-installed CLI installations. + +The healed path is used for the current session only and is never written back to `proxy.json` from the hook. The next `devflow proxy --enable` persists the corrected path. On re-resolution failure the original "relay binary not found" warning is emitted (avoids PF-009 — always exits 0). + **curl is guarded** with `command -v curl >/dev/null 2>&1` before the health-check identity call. When curl is absent, the hook assumes the relay is ours and exits 0 (no spurious warning). The CLI `--status` command is the authoritative identity check. **Health body parsed via `json_field`** — key-order-independent. The old substring match `*'"name":"subswitch"'*` was order-dependent; the current code pipes `$HEALTH_BODY` into `json_field "name" ""` (sourced from json-parse) and compares the extracted value. `json_field` is always available at this call site because line 33 exits the hook if `_JSON_AVAILABLE=false`. @@ -154,6 +193,32 @@ The spawn wait uses **80×0.1s = 8s** (hook) vs the CLI's **50×100ms = 5s**. Th **Hook spawn path is covered by tests** (tests/shell-hooks.test.ts): a stub relay reads `SUBSWITCH_CONFIG` and binds the port, asserting silent exit (exit 0, no stdout/stderr), a live pid recorded in `proxy.pid`, and spawn lock released. The failure branch (full 8s wait) is intentionally not unit-tested for duration reasons. +## post-install.ts: Settings Merge (D-SETTINGS-1) + +`installSettings` in `src/targets/claude-code/post-install.ts` **merges** the Devflow template into an existing `settings.json` rather than overriding it wholesale: + +- **Fresh file**: write the template directly. +- **Existing file**: call `mergeDevflowSettingsTemplate(existingParsed, templateParsed)` — adds Devflow hook entries absent from the file (idempotent by exact command string), sets `statusLine`/`attribution` only when the user has no existing value. +- **Parse failure**: warn and skip — the file is left byte-identical. A broken `settings.json` is never clobbered. +- **Foreign/unexpected hook shapes**: a per-event value that is not an array is left entirely untouched rather than overwritten or thrown on (applies PF-023). + +The old "override confirm" prompt is gone. The merge is purely additive (Devflow entries only), so no prompt is needed — declining could not protect user keys that the merge never touches, and the prompt rendered on top of the init spinner. + +`mergeDevflowSettingsTemplate` is exported for unit testing. Every shape check is at the mutation sink (not upstream) per PF-023. + +## subswitch 0.3.0 Routing Config Contract (D-EFR-4) + +`buildRoutingConfigJson(port, existingContent?)` builds `proxy-routing.json` with a strict 5-key shape (`port`, `logLevel`, `anthropic`, `providers`, `limits`) accepted by subswitch 0.3.0's `z.strictObject` schema. Unknown top-level keys cause a hard relay startup error — never emit them. + +**`ROUTING_CONFIG_REJECTED_SUBKEYS`** strips sub-keys that were valid in prior pinned versions but are hard startup errors in 0.3.0: +- `anthropic.streamIdleTimeoutMs` — valid in 0.2.0, removed in 0.3.0 +- `limits.connectTimeoutMs` — moved to `anthropic.connectTimeoutMs` in 0.3.0 +- `limits.maxConcurrentRequests` — valid in 0.2.0, removed in 0.3.0 + +**`anthropic.connectTimeoutMs`** (injected at `DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS = 120_000`ms when absent): in 0.3.0 this is a genuine DNS+TCP connect budget — armed on the socket, disarmed on the `'connect'` event. It no longer bounds the stream/headers phase. A user-specified value always wins. The 120s override dates from 0.2.0, where the same key was an inactivity cap whose 10s default killed long opus requests. + +`buildRoutingConfigJson` preserves user customisations from the existing file (logLevel, providers, anthropic sub-keys except rejected ones, limits sub-keys except rejected ones) so an upgrade from 0.2.0 cannot leave the relay unable to boot. + ## Model Discovery (model-discovery.ts) `src/core/model-discovery.ts` provides live model catalog access via the relay. @@ -266,16 +331,7 @@ type AgentState = 'active' | 'saved-inactive' | 'not-installed' | 'unknown'; **`AGENT_STATE_LABELS`** — `Readonly>` mapping each state to its bare display text (no color applied; color is the call site's responsibility). Shared by `render.ts` (TUI STATE column) and `formatListOutput` in `agents.ts` (`--list` STATE column) so the two surfaces cannot drift from each other. -**`classifyAgentState(opts)`** — pure function with no I/O: -```typescript -interface ClassifyAgentStateOptions { - configured: string; // 'default' or a model name - proxyEnabled: boolean; - installed: boolean; // agent .md present in install dir - inRegistry: boolean; // agent name in plugin registry -} -``` -Classification rules (evaluated in order): +**`classifyAgentState(opts)`** — pure function with no I/O. Classification rules (evaluated in order): 1. `!inRegistry` → `'unknown'` (orphan rows from `agent-models.json` not in the registry) 2. `!installed` → `'not-installed'` 3. `isDormantExternalModel(configured, proxyEnabled)` → `'saved-inactive'` @@ -321,7 +377,7 @@ The TUI follows a pure-reducer / pure-renderer / thin-terminal-shell split (appl ## writeFileAtomicExclusive — Mode Preservation -`writeFileAtomicExclusive` (in `src/core/fs-atomic.ts`) now preserves the target file's permission mode across atomic replace: +`writeFileAtomicExclusive` (in `src/core/fs-atomic.ts`) preserves the target file's permission mode across atomic replace: 1. Write to `.tmp` with O_EXCL (crash-safe). 2. `stat(filePath)` to read the existing mode (permission bits only, masked with `0o777`). @@ -342,25 +398,30 @@ A user who hardened `settings.json` to `0600` (to protect `ANTHROPIC_API_KEY`) n - **D-EFR-3: Never mock the routing-runtime subprocess without a paired real-binary test**: any test that mocks the routing-runtime subprocess must be paired with at least one CI-executed test that does not. The specific trap (PF-016 reproduced exactly): `tests/integration/**` is excluded from `npm test` by `vitest.config.ts` while CI runs only `npm run build && npm test` — a real-binary test placed in `tests/integration/` would never execute in CI. Place real-binary tests in `tests/` (not `tests/integration/`). - **Calling model discovery from `validateSetArgs` or `--list`/`--reset`**: `validateSetArgs` uses `isValidModelName` (from agent-frontmatter.ts, pure regex) — not model discovery. The zero-spawn constraint for `--list`, `--set`, and `--reset` is a firm requirement pinned by module-boundary spy tests in `tests/agents-command.test.ts`. Importing or calling `discoverExternalModels`/`getExternalModelsCached` from the validation path breaks these tests and violates the configure-first-then-enable flow. - **Putting the agent-state classifier in external-models.ts**: `external-models.ts` is a leaf module with a single responsibility (dormancy predicate + Claude alias set). Agent state classification belongs in `src/core/agent-state.ts` (applies ADR-013). Adding classifier logic to external-models.ts would give it two reasons to change and break its leaf-module contract. +- **Emitting 0.2.0-era sub-keys into proxy-routing.json**: `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, and `limits.maxConcurrentRequests` are hard startup errors in subswitch 0.3.0. Always build the config via `buildRoutingConfigJson` — never construct the JSON manually. +- **Gating env strip on `isProxyEnabled()` instead of `proxyJsonExists()`**: `readProxyState()` returns `Ok(defaultState)` on ENOENT — it cannot distinguish absence from a present file at the default port. Env strip must be gated on `proxyJsonExists()` (D-STRIP-1) to avoid clobbering a foreign gateway on machines that never had the proxy. +- **Skipping check ④ on the adopted-relay path**: a healthy relay on the port does not exempt the caller from the foreign-gateway refusal. `checkSettingsEnv(deps, port)` must run before any early-return on adoption (D-EFR-5). ## Gotchas - **`proxy.json` ENOENT is not an error**: `readProxyState()` returns a default disabled state when the file is missing. Callers that treat ENOENT as an error will get a false negative on fresh installs. +- **`proxyJsonExists()` vs `readProxyState()`**: `readProxyState()` returns `Ok(defaultState)` on ENOENT — it cannot differentiate "file absent" from "file present with `DEFAULT_PROXY_PORT`". Callers that must gate on managed-evidence (init env strip, uninstall cleanup) must call `proxyJsonExists()` separately (D-STRIP-1). - **Port adoption path**: if a relay is already accepting connections on the target port and the health check confirms our identity (`name === 'subswitch'`), preflight returns `adopted: true` and `spawnRelayAndWaitForPort` skips spawning. `spawnedPid` will be absent from `SpawnRelayResult` on this path — `runPostSpawnVerification` must never kill an adopted relay. -- **`stripProxyEnv` is port-scoped for the URL, unconditional for the window var (REG-1)**: `stripProxyEnv(settingsJson, managedPort)` removes `ANTHROPIC_BASE_URL` **only when its value exactly matches `http://127.0.0.1:`** (protecting foreign gateways on any other port), but removes `CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT` unconditionally — Devflow is its sole producer, so there is no foreign value to protect. A localhost URL on any other port classifies as `'ours-other-port'` or `'foreign'` and is never touched. Callers must pass the port Devflow owns (from `proxy.json.port` or `DEFAULT_PROXY_PORT`). `readProxyEnvState` uses the pattern `^http://127\.0\.0\.1:\d+$` to classify any localhost URL as `'ours-other-port'` for display purposes only — the strip never uses that broad pattern. +- **`stripProxyEnv` window var is unconditional when managedPort is defined (REG-1)**: `_stripProxyEnvFromObject(settings, managedPort)` removes `CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT` unconditionally (Devflow is its sole producer) and removes `ANTHROPIC_BASE_URL` only when its value exactly matches `http://127.0.0.1:`. This unconditional removal only runs when the caller has verified `proxyJsonExists()` — the evidence gate is the outer guard (D-STRIP-1), not the inner strip. - **Remembered port on re-enable**: `--port` has no commander default. When `--port` is omitted, `portOption` is `undefined` and `resolvePort(undefined, priorPort)` returns the remembered port from `proxy.json`. - **Dormant TUI rows**: when proxy is off and an agent has a saved GPT model, `buildRow()` calls `isDormantExternalModel()` and sets `configuredModel='default'` with the GPT name in `dormantModel`. `persistedModelFor(row)` returns `dormantModel` for an untouched dormant row, so `mergeTuiRowsIntoMapping` preserves the GPT mapping entry byte-identical on save even though `configuredModel` shows `'default'`. - **`binPath` must be spawned with `node `**: npm does not guarantee executable bits on installed package binaries. Always spawn as `node `, never `` directly. - **Leaked stub relays**: proxy tests that spawn stub relays must reap them on the failure path too — not only the happy path. Use `afterEach`/`onTestFinished` with SIGTERM→SIGKILL escalation and confirm death via `process.kill(pid, 0)`. Real incident: three orphaned stub relays accumulated over ~3 weeks; a full run stretched from ~24 seconds to 40+ minutes and produced 13–21 spurious failures in unrelated files (memory pipeline, capture hooks) that were repeatedly misdiagnosed as product defects. - **`resolveProxyBin()` uses `createRequire(import.meta.url)`**: ESM-safe way to resolve CommonJS package paths. The `require.resolve('subswitch/package.json')` approach finds the package relative to devflow's own `node_modules`, not the user's project. -- **env -i corporate-TLS**: `NODE_EXTRA_CA_CERTS` is now included in both the hook's `_RELAY_ENV` array and `scrubChildEnv()` in `proxy-log.ts`, so corporate-TLS deployments that supply a CA bundle via this var will have it forwarded to the relay. It is included only when non-empty (an empty path causes TLS errors). `NODE_OPTIONS` remains excluded from both allowlists — it permits arbitrary code execution via `--require`/`--import`. A drift-guard test in `tests/proxy-log.test.ts` asserts the hook's conditional append exists, so a future one-sided change is caught early rather than silently breaking one spawn path. +- **env -i corporate-TLS**: `NODE_EXTRA_CA_CERTS` is included in both the hook's `_RELAY_ENV` array and `scrubChildEnv()` in `proxy-log.ts`, so corporate-TLS deployments that supply a CA bundle via this var will have it forwarded to the relay. It is included only when non-empty (an empty path causes TLS errors). `NODE_OPTIONS` remains excluded from both allowlists — it permits arbitrary code execution via `--require`/`--import`. A drift-guard test in `tests/proxy-log.test.ts` asserts the hook's conditional append exists, so a future one-sided change is caught early rather than silently breaking one spawn path. - **`classifyCodexAuthReadError` is directly testable**: the ENOENT→`{kind:'absent'}` vs other-error→`{kind:'unreadable'}` classification that used to live inside `runStatus` is now exported from `codex-auth-inspect.ts`. Tests can import and call it with a synthetic error object without needing to mock the filesystem. - **`isProxyEnabled()` is the sole dormancy authority**: it reads only `proxy.json`. On preflight failure in `init.ts`, `proxy.json` is explicitly written to `enabled:false` so `isProxyEnabled()` returns the correct value for the `reapplyAgentMapping` call that follows. Any new code path that forces the proxy off must write this file — relying on manifest alone is insufficient. - **`readInstalledAgentNames` degrades on any error, not just ENOENT**: the catch block is bare (`catch {}`) — EPERM, ENOTDIR, and any other OS error all return an empty set rather than throwing (avoids PF-009). A misconfigured install path must not crash the TUI or `--list`. +- **subswitch 0.3.0 `connectTimeoutMs` semantics changed**: in 0.2.0 it was an inactivity/socket timeout that killed long requests; in 0.3.0 it is strictly a DNS+TCP connect budget (armed on socket, disarmed on `'connect'`). The 120s devflow default is now only a generous connect window, not a keep-alive guard. ## Key Files -- `src/core/proxy-state.ts` — ProxyState schema, read/write, `isProxyEnabled()`, `resolveProxyBin()`, `buildRoutingConfigJson()` +- `src/core/proxy-state.ts` — ProxyState schema, read/write, `isProxyEnabled()`, `proxyJsonExists()` (evidence discriminator, D-STRIP-1), `resolveProxyBin()`, `buildRoutingConfigJson()` (0.3.0 5-key shape, `ROUTING_CONFIG_REJECTED_SUBKEYS`, connectTimeoutMs inject) - `src/core/external-models.ts` — `CLAUDE_MODEL_ALIASES` (as const), `ClaudeModelAlias` literal union, `isClaudeModelName()`, `isDormantExternalModel()` (leaf module, no project imports) - `src/core/agent-state.ts` — `AgentState` type, `AGENT_STATE_LABELS` record, `classifyAgentState()` — single vocabulary for the STATE column shared by `--list` and the TUI (applies ADR-013; leaf module, imports only external-models.ts) - `src/core/agent-frontmatter.ts` — pure frontmatter rewriter, `readFrontmatterModel()`, `rewriteAgentFrontmatter()`, `isValidModelName()` (MODEL_NAME_RE — imported by validateSetArgs for zero-spawn charset validation) @@ -370,24 +431,27 @@ A user who hardened `settings.json` to `0600` (to protect `ANTHROPIC_API_KEY`) n - `src/core/cache.ts` — `modelCacheDir(devflowDir)` and `hudCacheDir(devflowDir)` path accessors (single source of truth for cache layout — avoids PF-013); `parseRawEnvelope()` (single canonical envelope parser, rejects non-finite ttl); `readCache()`, `writeCache()` with 0700/0600 permissions; `pruneOldEntries()` (keeps 3 entries by timestamp, reaps `.json.tmp.*` orphans) - `src/core/proxy-log.ts` — `scrubChildEnv()` (allowlist-based env for relay spawn; paired with hook's `env -i` allowlist), `openProxyLog()`, `rotateProxyLogIfLarge()` - `src/core/fs-atomic.ts` — `writeFileAtomicExclusive()` — mode-preserving atomic write -- `src/cli/commands/proxy.ts` — `proxyCommand`; exported seams: `buildRealPreflightDeps`, `spawnRelayAndWaitForPort`, `runPostSpawnVerification`, `resolvePort`, `isOurRelayBody`, `runProxyPreflight`, `applyProxyEnv`, `stripProxyEnv`, `applyDisableToSettings`, `addProxyHooks`, `removeProxyHooks`, `hasProxyHooks`, `readProxyEnvState`, `formatCodexAuthLine` (returns `{level,msg}`), `realHttpGet`, `PostSpawnDoctorDeps` +- `src/cli/commands/proxy.ts` — `proxyCommand`; exported seams: `buildRealPreflightDeps`, `spawnRelayAndWaitForPort`, `runPostSpawnVerification`, `resolvePort`, `isOurRelayBody`, `runProxyPreflight`, `applyProxyEnv`, `stripProxyEnv`, `applyDisableToSettings`, `applyProxyTeardownToSettings` (unified teardown, D-STRIP-1), `addProxyHooks`, `removeProxyHooks`, `hasProxyHooks`, `readProxyEnvState`, `formatCodexAuthLine`, `realHttpGet`, `PostSpawnDoctorDeps` - `src/cli/commands/agents.ts` — `agentsCommand`, `validateSetArgs()` (calls `isValidModelName`, zero-spawn), `applySetMapping()`, `buildListRows()`, `mergeTuiRowsIntoMapping()` (consumes `persistedModelFor`/`persistedEffortFor`) - `src/cli/agents-view/state.ts` — pure reducer, `buildRow()`, `isDirtyModel()`, `isDirtyEffort()`, `persistedModelFor()`, `persistedEffortFor()`, `rowState()` (delegates to `classifyAgentState`), `unsavedCount()` - `src/cli/agents-view/render.ts` — pure frame renderer; `COL_STATE = 14`; exports `FIXED_ROWS`, `computeViewportHeight` - `src/cli/agents-view/terminal.ts` — thin adapter over the shared `runTui` driver (`src/cli/tui/terminal.ts`); exports `runAgentsTui()`, re-exports `TuiIO` and `MAX_KEYPRESSES` from tui/ - `src/cli/tui/terminal.ts` — generic `runTui` driver (`RunTuiSpec`: `signalAction: Exclude`, `continueIntent: C`, `screen?: 'alt'|'inline'`), `normalizeKey`, `TuiIO`, `MAX_KEYPRESSES`, `RenderDims`, `INLINE_MARGIN`; agents-view uses `signalAction='cancel'` + default `'alt'` screen; flags-view uses `signalAction='abort'` + `'inline'` screen - `src/cli/tui/cells.ts` — cell helper utilities (shared across TUI modules) -- `src/assets/scripts/hooks/ensure-proxy` — SessionStart + UserPromptSubmit hook; writes `proxy.pid` after spawn; UserPromptSubmit exits before proxy-state reads; relay spawned via `env -i` 6-var allowlist -- `src/cli/commands/init.ts` — proxy preflight block (4-check, no doctor, no spawn); `reapplyAgentMapping` guard after preflight; convergence writes `proxy.json enabled:false` on preflight failure +- `src/assets/scripts/hooks/ensure-proxy` — SessionStart + UserPromptSubmit hook; binPath re-resolution (bounded walk max 6 + `command -v subswitch`); writes `proxy.pid` after spawn; UserPromptSubmit exits before proxy-state reads; relay spawned via `env -i` 6-var allowlist +- `src/targets/claude-code/post-install.ts` — `mergeDevflowSettingsTemplate()` (D-SETTINGS-1: merge not overwrite; idempotent by exact command string; shape-guarded; exported for testing); `installSettings()` — no override confirm prompt; parse failure writes nothing +- `src/cli/commands/init.ts` — proxy preflight block (4-check, no doctor, no spawn); `proxyJsonExists()` gates env strip (D-STRIP-1); `reapplyAgentMapping` guard after preflight; convergence writes `proxy.json enabled:false` on preflight failure +- `src/cli/commands/uninstall.ts` — imports `applyProxyTeardownToSettings`; cleanup phase calls `proxyJsonExists()` to determine `managedPort` (present → pass port; absent → pass `undefined`) ## Related - **ADR-013**: src/core vs src/cli boundary — all state I/O and pure logic in `src/core/`; CLI orchestration and user-facing action handlers in `src/cli/`. The proxy feature is the canonical multi-module example of this split. `src/core/agent-state.ts` is a new application: moving the agent-state classifier out of `external-models.ts` into its own core leaf module gives it a single responsibility and a clear home (applies ADR-013). - **ADR-014**: state-aware re-init — `proxy` is seeded from `manifest?.features.proxy ?? FEATURE_DEFAULTS.proxy` in `resolveSeedFeatures`. On `--reset`, seeds as `false`. Never read from `config.json`. -- **PF-009**: all proxy artifact removals in uninstall/disable are non-fatal; preflight failure warns but never aborts `devflow init` — `proxyEnabled` is simply forced to `false`. Also: `readInstalledAgentNames` degrades to empty set on any error; `writeFileAtomicExclusive` chmod step is non-fatal. +- **PF-009**: all proxy artifact removals in uninstall/disable are non-fatal; preflight failure warns but never aborts `devflow init` — `proxyEnabled` is simply forced to `false`. Also: `readInstalledAgentNames` degrades to empty set on any error; `writeFileAtomicExclusive` chmod step is non-fatal; ensure-proxy binPath re-resolution is best-effort (always exits 0). - **PF-013**: cache path accessors (`modelCacheDir`, `hudCacheDir`) in `cache.ts` prevent write-site/removal-site drift — the canonical PF-013 shape applied to the model-discovery cache. - **PF-014**: no `process.exit()` inside finally-guarded scopes — TUI cleanup wired via Promise `resolve()`; hard failures in CLI commands set `process.exitCode = 1` and return. -- **PF-015**: every path that forces proxy off must converge `proxy.json enabled:false` alongside manifest/hooks/env — `isProxyEnabled()` reads only `proxy.json`, so this file is the load-bearing convergence point. +- **PF-015**: every path that forces proxy off must converge `proxy.json enabled:false` alongside manifest/hooks/env — `isProxyEnabled()` reads only `proxy.json`, so this file is the load-bearing convergence point. Also: both-operations invariant in `applyDisableToSettings` — neither `removeProxyHooks` nor `_stripProxyEnvFromObject` may be short-circuited. - **PF-017**: relay spawn uses `env -i` 6-var allowlist (hook) + `scrubChildEnv()` (CLI spawn) — both allowlists must be kept in sync for corporate-TLS users. +- **PF-023**: validation at the sink that mutates — `mergeDevflowSettingsTemplate` shape-guards every `hooks` value before touching it; foreign/unexpected hook shapes left untouched rather than overwritten. - **PF-001**: port digit-validated before /dev/tcp interpolation in `ensure-proxy`. - Feature knowledge: `installer-shadowing` — covers `resolveSeedFeatures`, manifest-group feature seeding, and uninstall artifact cleanup patterns that proxy extends. diff --git a/.devflow/features/index.md b/.devflow/features/index.md index 5e9bc21d..14902fb7 100644 --- a/.devflow/features/index.md +++ b/.devflow/features/index.md @@ -4,5 +4,5 @@ - **resolve-pipeline** — src/assets/commands/resolve.mds, src/assets/agents/triage.md, src/assets/agents/code.md, src/core/plugins.ts, src/assets/commands/code-review.mds — Use when modifying /resolve or /code-review convergence logic, adding or changing Triage disposition rules (including DUPLICATE collapsing), adjusting Code-agent operating modes (issue-fix/validation-fix), touching the resolution-summary.md parser contract, changing the Verification Gate retry loop, understanding how DIFF_FILES flows from git validate-branch into blast-radius triage, or working on traceability operations (fetch-review-threads, resolve-review-threads, post-resolution-summary, check-merge-readiness, THREAD_MAP). Keywords: resolve, triage, disposition matrix, blast-radius, FIX_NOW, FIX_SEPARATE, TECH_DEBT, FALSE_POSITIVE, BY_DESIGN, ESCALATED, DUPLICATE, duplicate-grouping, duplicates-collapse, duplicate_of, resolution-summary, convergence parser, DIFF_FILES, issue-fix, validation-fix, Verification Gate, manage-debt, COMPLIANCE_SKILL_INSTALLED, TRACEABILITY DEGRADED, fetch-review-threads, THREAD_MAP, post-resolution-summary, Third-Party Threads, check-merge-readiness, ext-N, D7, D9, PF-024. - **installer-shadowing** — src/targets/claude-code/installer.ts, src/targets/claude-code/legacy.ts, src/cli/commands/init.ts, src/cli/commands/init-seed.ts, src/cli/commands/uninstall.ts, src/cli/commands/rules.ts, src/cli/commands/skills.ts, src/cli/commands/flags.ts, src/cli/flags-view, src/cli/tui, src/core/plugins.ts, src/core/assets.ts, src/core/paths.ts, src/core/manifest.ts, src/core/flags.ts, src/core/feature-config.ts, src/core/orphan-sweep.ts, src/core/migrations.ts, src/cli/commands/compliance-prompts.ts — Use when modifying the install pipeline (installViaFileCopy, installAllRules, composeScripts, InstallReport), adding or changing skill/rule shadow override logic, touching uninstall scope (enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, sweepDevflowNamespaces, resolveProjectDataCleanup) or install-artifact cleanup, extending the CLI skills/rules/flags management commands, working with asset directory accessors (rulesDir, skillsDir, commandsDir) and package-root resolution, modifying the init seeding layer (resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, --reset, FlagsRecord, knownPlugins, readConfigIfPresent, resolveExistingViewMode, getAllCommandNames, proxy), working on the flags TUI (FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, effectiveDisplay, blurb, inline mode, RunTuiSpec screen) or the flags CLI (createFlagsCommand, lookupFlag, persistFlagConfig, formatFlagValue), or working on the compliance wizard step (shouldRunComplianceStep, runComplianceStep, modePromptShown, CompliancePromptIO). Keywords: installViaFileCopy, installAllRules, composeScripts, InstallReport, RuleInstallOutcome, SkillShadowState, RuleShadowState, shadow, unshadow, validateSkillShadow, validateRuleShadow, seedRuleShadow, prefixSkillName, unprefixSkillName, devflow:, skills, rules, uninstall, EISDIR, enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, enumerateDryRunExtras, sweepDevflowNamespaces, resolveProjectDataCleanup, runDryRunPhase, runSelectivePhaseForScope, runFullPhaseForScope, runCleanupPhase, getPackageRoot, isContainedIn, rulesDir, skillsDir, agentsDir, commandsDir, scriptsDir, LEGACY_SKILL_NAMES, sweepOrphanedAssets, SweepResult, sweepOrphans, sweepFailures, SweepFailure, mdFileName, mdEntryName, orphan sweep, getAllSkillNames, getAllCommandNames, getAllAgentNames, DELETED_PLUGIN_NAMES, EXCLUDED, resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, resolveResetGatedInputs, applyCliToggles, FlagsRecord, FlagsRecordValue, getDefaultFlagsRecord, parseManifestFlags, migrateLegacyFlagsToRecord, sanitizeFlagsRecord, coerceFlagValue, parseFlagValueInput, neutralValueOf, isNeutral, countActiveFlags, readViewMode, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveFinalViewMode, reset, init-seed, proxy, reapplyAgentMapping, revertExternalAgents, agent-models.json, proxy.json, proxy-routing.json, proxy.pid, applyDisableToSettings, buildRealPreflightDeps, canonicalise-agent-keys-v1, AnyMigration, migrations.json, compliance-prompts, shouldRunComplianceStep, CompliancePromptIO, runComplianceStep, modePromptShown, createFlagsCommand, lookupFlag, persistFlagConfig, FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, buildStops, cycleForward, cycleBackward, sanitizeCell, padToVisible, truncateVisible, effectiveDisplay, EffectiveDisplay, formatFlagValue, blurb, FlagDefCommon, INLINE_MARGIN, cursorUp, RunTuiSpec, screen, inline. - **learning-capture-system** — src/assets/scripts/hooks, src/assets/agents/learning.md, src/cli/commands/learning.ts, src/core/feature-config.ts, src/core/learning-tuning-config.ts, src/hud/components/learning-counts.ts, src/assets/commands/_partials — Use when modifying capture hooks (capture-prompt/capture-turn/capture-question), the learning or memory pending-turns queues, the Learning agent (src/assets/agents/learning.md), the session-start-context learning directive, the feature-config toggles, the learning tuning config, the decisions content files (decisions.md/pitfalls.md/index.md) or their ledger ops, or the devflow learning CLI. Keywords: capture-prompt, capture-turn, capture-question, queue-append, pending-turns, memory-worker, Learning agent, learning directive, LEARNING MAINTENANCE, DEVFLOW_BG_UPDATER, learning-lock, queue_read_gates, decisions_load, DECISIONS_CONTEXT, feature-config, config.json, learning.json, decisions-ledger, assign-anchor, retire-anchor, refresh-anchor, render-decisions, staged-write CAS, WORKING-MEMORY.md.new, segmentDetails, amendments, is-hex-sha, verify_and_swap, compute_commits_since_note, divergence guard, isSafeRawBody. -- **external-model-routing** — src/core/proxy-state.ts, src/core/external-models.ts, src/core/agent-models.ts, src/core/agent-state.ts, src/core/agent-frontmatter.ts, src/core/codex-auth-inspect.ts, src/core/model-discovery.ts, src/core/cache.ts, src/core/proxy-log.ts, src/cli/commands/proxy.ts, src/cli/commands/agents.ts, src/cli/agents-view, src/cli/tui — Use when working on the proxy lifecycle (enable/disable/status/preflight), the ensure-proxy hook, per-agent model mapping, agent frontmatter rewriting, or the agents TUI. Keywords: proxy, external-model-routing, GPT, agent-models, ensure-proxy, frontmatter, devflow proxy, devflow agents, subswitch, ANTHROPIC_BASE_URL, dormancy, reapplyAgentMapping. +- **external-model-routing** — src/core/proxy-state.ts, src/core/external-models.ts, src/core/agent-models.ts, src/core/agent-state.ts, src/core/agent-frontmatter.ts, src/core/codex-auth-inspect.ts, src/core/model-discovery.ts, src/core/cache.ts, src/core/proxy-log.ts, src/cli/commands/proxy.ts, src/cli/commands/agents.ts, src/cli/agents-view, src/cli/tui — Use when working on the proxy lifecycle (enable/disable/status/preflight), the ensure-proxy hook, per-agent model mapping, agent frontmatter rewriting, or the agents TUI. Keywords: proxy, external-model-routing, GPT, agent-models, ensure-proxy, frontmatter, devflow proxy, devflow agents, subswitch, ANTHROPIC_BASE_URL, dormancy, reapplyAgentMapping, proxyJsonExists, applyProxyTeardownToSettings, D-STRIP-1, mergeDevflowSettingsTemplate, subswitch 0.3.0. - **compliance-feature** — src/core/compliance.ts, src/targets/claude-code/compliance-install.ts, src/cli/commands/compliance.ts, src/assets/skills/compliance, src/assets/rules/compliance.md, src/assets/agents/git.md, src/assets/commands/code-review.mds, src/assets/commands/plan.mds, src/assets/commands/implement.mds, src/assets/commands/resolve.mds, src/assets/commands/release.md — Use when adding or modifying the compliance feature (framework registry, converge contract, CLI, rule stamping), changing how host commands resolve COMPLIANCE_SKILL_INSTALLED, modifying traceability operations in the Git agent (learn-conventions, issue-first, thread resolution, shipped markers, release evidence), or extending the D4 DEGRADED contract. Keywords: compliance, COMPLIANCE_SKILL_INSTALLED, convergeComplianceArtifacts, convergeFromManifest, frameworks, FEATURE_OWNED_SKILLS, traceability, D4, D9, gather-release-evidence, conventions.md, resolve-review-threads, ensure-traceable-issue, stamper, manifest-group, ComplianceFeatureState. From 69b963eee967b1c9e63c6a59a4df868d6f201cc5 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 11:02:01 +0300 Subject: [PATCH 13/17] chore(deps): bump subswitch 0.3.0 -> 0.4.0 Streams over-window Anthropic-bound bodies instead of relay-synthesized 413, so long prompts no longer fail at the proxy; only translated (Codex) routes still return 413 request_too_large. limits.maxBodyBytes is renamed limits.maxBufferedBodyBytes, with the old spelling registered as a legacy key that makes the relay refuse to start. Includes the packaging version guard (SUBSWITCH_VERSION) so the pin and its assertion move together and the commit stays green under bisect. Refs: subswitch#42 / PR#43 --- package-lock.json | 8 ++++---- package.json | 2 +- tests/packaging.test.ts | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9a30b2f7..bb4ca712 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,7 @@ "@clack/prompts": "^0.9.1", "commander": "^12.0.0", "picocolors": "^1.1.1", - "subswitch": "0.3.0" + "subswitch": "0.4.0" }, "bin": { "devflow": "dist/cli.js" @@ -1522,9 +1522,9 @@ "license": "MIT" }, "node_modules/subswitch": { - "version": "0.3.0", - "resolved": "https://registry.npmjs.org/subswitch/-/subswitch-0.3.0.tgz", - "integrity": "sha512-tmXMR8K3kCXgNVGPohvUE4vNPX4wPrSD/RotO57Lb9lk44VsYhDbQXF/2Bd8iGwimvGks+ixsrVRUIiu6s/aew==", + "version": "0.4.0", + "resolved": "https://registry.npmjs.org/subswitch/-/subswitch-0.4.0.tgz", + "integrity": "sha512-z/lLW59foN60NdNIdCb7obz0jWvQoAQ2WbOI5edtiqyTjCfeI7hrgi6Rnbdytx3M4qQ8to7haPYYCVoLUBSeFA==", "license": "MIT", "dependencies": { "@clack/prompts": "^1.7.0", diff --git a/package.json b/package.json index b19b2d8f..9bcfacbf 100644 --- a/package.json +++ b/package.json @@ -63,7 +63,7 @@ "@clack/prompts": "^0.9.1", "commander": "^12.0.0", "picocolors": "^1.1.1", - "subswitch": "0.3.0" + "subswitch": "0.4.0" }, "devDependencies": { "@mdscript/mds": "0.2.0", diff --git a/tests/packaging.test.ts b/tests/packaging.test.ts index be444d18..fcc83b64 100644 --- a/tests/packaging.test.ts +++ b/tests/packaging.test.ts @@ -25,7 +25,7 @@ const ROOT = path.resolve(import.meta.dirname, '..'); * Expected exact-pinned version of the routing runtime. * Hoisted so the next bump is a one-line change. */ -const SUBSWITCH_VERSION = '0.3.0'; +const SUBSWITCH_VERSION = '0.4.0'; // --------------------------------------------------------------------------- // Guard 3: dependency pin integrity From ba09de93bcaf5fa447b11e0a2e0c61c94be03bb1 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 11:02:12 +0300 Subject: [PATCH 14/17] fix(proxy): align subswitch integration with 0.4.0 config surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildRoutingConfigJson now strips limits.maxBodyBytes, renamed to limits.maxBufferedBodyBytes in 0.4.0. The old spelling is a registered legacy key that makes the relay refuse to start, so a hand-edited proxy-routing.json carrying it would have killed the relay on every session start — spawned by the ensure-proxy hook, with no route back. This is the exact failure ROUTING_CONFIG_REJECTED_SUBKEYS exists to prevent; it joins streamIdleTimeoutMs, limits.connectTimeoutMs, and limits.maxConcurrentRequests. DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS drops 120000 -> 10000. The 120s override dates from 0.2.0, where the key was an upstream inactivity cap whose 10s default killed long requests. Since 0.3.0 it is armed on the socket and disarmed on connect, bounding only DNS+TCP, so the wide budget bounded nothing extra and only delayed failure against an unroutable host. 10000 matches the relay's own default; a user-supplied value still wins. Pinned by literal value, not just by symbol — every prior assertion compared against the imported constant and would have stayed green if the value drifted. Corrects the CLAUDE.md claim that proxy-routing.json is a bare {port} object (it has always also carried anthropic.connectTimeoutMs) and re-stamps 0.2.0-era catalog provenance comments, re-verified against the 0.4.0 binary: gpt-5.6-sol, gpt-5.6-terra, gpt-5.6-luna, gpt-5.5, and models --json still exits 0 with configFileFound:false. No devflow code depended on the removed 503 overloaded_error, the concurrency admission gate, or the local 400 unknown-provider path. --- .../external-model-routing/KNOWLEDGE.md | 17 ++++++---- CHANGELOG.md | 8 +++++ CLAUDE.md | 4 +-- src/core/proxy-state.ts | 34 +++++++++++-------- tests/external-models.test.ts | 2 +- tests/init-proxy.test.ts | 2 +- tests/model-discovery.test.ts | 2 +- tests/proxy-state.test.ts | 33 ++++++++++++++++-- 8 files changed, 74 insertions(+), 28 deletions(-) diff --git a/.devflow/features/external-model-routing/KNOWLEDGE.md b/.devflow/features/external-model-routing/KNOWLEDGE.md index f6b17607..2c9434ab 100644 --- a/.devflow/features/external-model-routing/KNOWLEDGE.md +++ b/.devflow/features/external-model-routing/KNOWLEDGE.md @@ -18,7 +18,7 @@ Two authority sources govern the proxy at different points in its lifecycle. `ma ## System Context -The routing runtime is an internal package (`subswitch@0.3.0`, exact-pinned in `package.json`). Its name is a **hard branding constraint** — it must never appear in user-visible strings, error messages, CLI output, or agent context injections. User-facing vocabulary is always "external model routing" / "Devflow proxy". The one exception is internal code: health-check body comparisons (`body['name'] === 'subswitch'`), `SUBSWITCH_CONFIG` env var, and hook log lines are fine. +The routing runtime is an internal package (`subswitch@0.4.0`, exact-pinned in `package.json`). Its name is a **hard branding constraint** — it must never appear in user-visible strings, error messages, CLI output, or agent context injections. User-facing vocabulary is always "external model routing" / "Devflow proxy". The one exception is internal code: health-check body comparisons (`body['name'] === 'subswitch'`), `SUBSWITCH_CONFIG` env var, and hook log lines are fine. ## Proxy Lifecycle @@ -27,7 +27,7 @@ The routing runtime is an internal package (`subswitch@0.3.0`, exact-pinned in ` | File | Role | |------|------| | `~/.devflow/proxy.json` | Runtime authority. Tolerant-parsed by `readProxyState()`. ENOENT → default disabled state (not an error). Fields: `enabled`, `port`, `binPath`, `configPath`, `resolvedAt`, `devflowVersion`. | -| `~/.devflow/proxy-routing.json` | Routing config written by `buildRoutingConfigJson(port, existingContent?)`. Strict 5-key shape accepted by subswitch 0.3.0: `port`, `logLevel`, `anthropic`, `providers`, `limits`. Always injects `anthropic.connectTimeoutMs: 120000` when absent (D-EFR-4). Strips 0.2.0-era sub-keys that are hard startup errors in 0.3.0: `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, `limits.maxConcurrentRequests` (see `ROUTING_CONFIG_REJECTED_SUBKEYS`). Written before preflight runs on enable. | +| `~/.devflow/proxy-routing.json` | Routing config written by `buildRoutingConfigJson(port, existingContent?)`. Strict 5-key shape accepted by subswitch 0.4.0: `port`, `logLevel`, `anthropic`, `providers`, `limits`. Always injects `anthropic.connectTimeoutMs: 10000` when absent (D-EFR-4). Strips legacy sub-keys that are hard startup errors in 0.4.0: `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, `limits.maxConcurrentRequests`, `limits.maxBodyBytes` (see `ROUTING_CONFIG_REJECTED_SUBKEYS`). Written before preflight runs on enable. | | `manifest.features.proxy` | Init/uninstall authority. Seeds from prior manifest on re-init (ADR-014). Never in `config.json` — manifest-group by design, same as `ambient`/`hud`/`rules`. | **`isProxyEnabled()` is the sole dormancy authority**: it reads only `proxy.json` (never manifest). This is load-bearing: on preflight failure, `init.ts` converges `proxy.json` to `enabled:false` alongside manifest, hooks, and env — all four artifacts must agree (avoids PF-015). Any path that forces `proxyEnabled=false` must write `proxy.json enabled:false` so that a subsequent `isProxyEnabled()` call in the same process returns false correctly. @@ -206,18 +206,21 @@ The old "override confirm" prompt is gone. The merge is purely additive (Devflow `mergeDevflowSettingsTemplate` is exported for unit testing. Every shape check is at the mutation sink (not upstream) per PF-023. -## subswitch 0.3.0 Routing Config Contract (D-EFR-4) +## subswitch 0.4.0 Routing Config Contract (D-EFR-4) -`buildRoutingConfigJson(port, existingContent?)` builds `proxy-routing.json` with a strict 5-key shape (`port`, `logLevel`, `anthropic`, `providers`, `limits`) accepted by subswitch 0.3.0's `z.strictObject` schema. Unknown top-level keys cause a hard relay startup error — never emit them. +`buildRoutingConfigJson(port, existingContent?)` builds `proxy-routing.json` with a strict 5-key shape (`port`, `logLevel`, `anthropic`, `providers`, `limits`) accepted by subswitch 0.4.0's `z.strictObject` schema. Unknown top-level keys cause a hard relay startup error — never emit them. -**`ROUTING_CONFIG_REJECTED_SUBKEYS`** strips sub-keys that were valid in prior pinned versions but are hard startup errors in 0.3.0: +**`ROUTING_CONFIG_REJECTED_SUBKEYS`** strips sub-keys that were valid in prior pinned versions but are hard startup errors in 0.4.0: - `anthropic.streamIdleTimeoutMs` — valid in 0.2.0, removed in 0.3.0 - `limits.connectTimeoutMs` — moved to `anthropic.connectTimeoutMs` in 0.3.0 - `limits.maxConcurrentRequests` — valid in 0.2.0, removed in 0.3.0 +- `limits.maxBodyBytes` — valid through 0.3.0, renamed `limits.maxBufferedBodyBytes` in 0.4.0 -**`anthropic.connectTimeoutMs`** (injected at `DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS = 120_000`ms when absent): in 0.3.0 this is a genuine DNS+TCP connect budget — armed on the socket, disarmed on the `'connect'` event. It no longer bounds the stream/headers phase. A user-specified value always wins. The 120s override dates from 0.2.0, where the same key was an inactivity cap whose 10s default killed long opus requests. +**`anthropic.connectTimeoutMs`** (injected at `DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS = 10_000`ms when absent): bounds only the DNS+TCP connect — armed on the socket, disarmed on the `'connect'` event. It never bounds the headers or stream phase, so it cannot cap a long-running request that has already connected. The value matches the relay's own default; a wider budget would only delay failure against an unroutable host. A user-specified value always wins. -`buildRoutingConfigJson` preserves user customisations from the existing file (logLevel, providers, anthropic sub-keys except rejected ones, limits sub-keys except rejected ones) so an upgrade from 0.2.0 cannot leave the relay unable to boot. +**Body handling in 0.4.0**: bodies over the buffering window are streamed to Anthropic rather than rejected (route labels `anthropic:streamed` / `anthropic:streamed:unsniffed`, log field `bodyMode`). Only translated (Codex) routes still return 413 `request_too_large`. The 0.2.0-era synthesized 503 `overloaded_error` no longer exists — devflow never depended on it. + +`buildRoutingConfigJson` preserves user customisations from the existing file (logLevel, providers, anthropic sub-keys except rejected ones, limits sub-keys except rejected ones) so an upgrade from an older pinned version cannot leave the relay unable to boot. ## Model Discovery (model-discovery.ts) diff --git a/CHANGELOG.md b/CHANGELOG.md index 697e9f74..11089e65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- **Routing runtime pinned to `subswitch@0.4.0`** (from `0.2.0`). Over-window Anthropic-bound request bodies are now streamed upstream instead of being rejected by the relay, so long prompts no longer fail at the proxy; only translated (Codex) routes still return `413 request_too_large`. `anthropic.connectTimeoutMs` now bounds only the DNS+TCP connect, so the injected default drops from `120000` to `10000` — the relay's own default. A wider budget bounded nothing extra and only delayed failure against an unroutable host. + +### Fixed +- **`proxy-routing.json` upgrade safety**: `buildRoutingConfigJson` now strips `limits.maxBodyBytes`, which `subswitch@0.4.0` renamed to `limits.maxBufferedBodyBytes`. The old spelling is a registered legacy key that makes the relay refuse to start, so a hand-edited config carrying it would have killed the relay on every session start — spawned by the `ensure-proxy` hook, with no route back. Joins the existing strips for `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, and `limits.maxConcurrentRequests`. + +**Upgrade**: no action required. If you hand-edited `~/.devflow/proxy-routing.json` to set `limits.maxBodyBytes`, re-set it as `limits.maxBufferedBodyBytes`; the next `devflow proxy --enable` drops the stale key for you. + --- ## [2.3.0] - 2026-08-31 diff --git a/CLAUDE.md b/CLAUDE.md index 3b89ddca..f1159cd9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -63,7 +63,7 @@ Debug logs stored at `~/.devflow/logs/{project-slug}/`. Knowledge write-back is in-command (not a background pipeline): gated by `devflow knowledge --enable/--disable` (flips `knowledge` in feature config); Knowledge agent writes directly at workflow end. -**External Model Routing (Devflow Proxy)**: Routes Devflow agents through GPT models via an OpenAI/Codex subscription using a local relay. Feature state is manifest-gated (like ambient/hud/rules, per ADR-001): `manifest.features.proxy` is the source of truth; `~/.devflow/proxy.json` holds runtime authority (enabled, port, binPath, configPath, resolvedAt, devflowVersion). `~/.devflow/proxy-routing.json` holds the routing config (port only — bare `{port}` object; the 0.2.0 routing runtime rejects unrecognised keys). The `ensure-proxy` hook (SessionStart + UserPromptSubmit, registered/removed by `addProxyHooks`/`removeProxyHooks`) auto-starts the relay when a session begins. `ANTHROPIC_BASE_URL` and `CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT` are injected into (and stripped from) `settings.json` at CLI enable/disable time via `applyProxyEnv`/`stripProxyEnv`, not by the hook (the window var is removed unconditionally on strip; the URL delete is port-scoped to avoid clobbering foreign gateways). Toggle via `devflow proxy --enable/--disable/--status` or via the Advanced init wizard. Enabling runs `runProxyPreflight` (4 checks: bin, codex auth, port, settings), spawns the relay, then gates on a post-spawn doctor verification against the live relay (doctor requires a running relay to pass); on doctor failure the enable rolls back, killing the relay only if it spawned it. Init runs the same preflight but never spawns or runs doctor — the first session's ensure-proxy hook starts the relay; on init preflight failure: warning + force-disabled, init never aborted (avoids PF-009). Disabling reverts agent frontmatter to Claude defaults but preserves the model mapping for re-enable. Default OFF; Advanced-only — never part of Recommended defaults. +**External Model Routing (Devflow Proxy)**: Routes Devflow agents through GPT models via an OpenAI/Codex subscription using a local relay. Feature state is manifest-gated (like ambient/hud/rules, per ADR-001): `manifest.features.proxy` is the source of truth; `~/.devflow/proxy.json` holds runtime authority (enabled, port, binPath, configPath, resolvedAt, devflowVersion). `~/.devflow/proxy-routing.json` holds the routing config (`port` plus an injected `anthropic.connectTimeoutMs` default, preserving any existing `logLevel`/`anthropic`/`providers`/`limits` blocks; the 0.4.0 routing runtime rejects unrecognised top-level keys and hard-fails on registered legacy keys — `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, `limits.maxConcurrentRequests`, `limits.maxBodyBytes` — which `buildRoutingConfigJson` strips). The `ensure-proxy` hook (SessionStart + UserPromptSubmit, registered/removed by `addProxyHooks`/`removeProxyHooks`) auto-starts the relay when a session begins. `ANTHROPIC_BASE_URL` and `CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT` are injected into (and stripped from) `settings.json` at CLI enable/disable time via `applyProxyEnv`/`stripProxyEnv`, not by the hook (the window var is removed unconditionally on strip; the URL delete is port-scoped to avoid clobbering foreign gateways). Toggle via `devflow proxy --enable/--disable/--status` or via the Advanced init wizard. Enabling runs `runProxyPreflight` (4 checks: bin, codex auth, port, settings), spawns the relay, then gates on a post-spawn doctor verification against the live relay (doctor requires a running relay to pass); on doctor failure the enable rolls back, killing the relay only if it spawned it. Init runs the same preflight but never spawns or runs doctor — the first session's ensure-proxy hook starts the relay; on init preflight failure: warning + force-disabled, init never aborted (avoids PF-009). Disabling reverts agent frontmatter to Claude defaults but preserves the model mapping for re-enable. Default OFF; Advanced-only — never part of Recommended defaults. **Per-Agent Model Configuration**: User overrides to agent model assignments persist in `~/.devflow/agent-models.json` (deviations only — absent entry = shipped default). `reapplyAgentMapping` runs after every `devflow init` post-install to re-apply user overrides to freshly copied agent files. `revertExternalAgents` reverts all agents to shipped defaults (called on proxy disable and before agent removal on uninstall). GPT model assignments are **dormant** when routing is off — they are stored in `agent-models.json` but not written to agent frontmatter until routing is enabled. Manage via `devflow agents` TUI or `devflow agents --list/--set/--reset`. Core source files: `src/core/agent-frontmatter.ts` (pure rewrite engine), `src/core/agent-models.ts` (schema + apply/revert), `src/core/external-models.ts` (CLAUDE_MODEL_ALIASES, isClaudeModelName, isDormantExternalModel — leaf module), `src/core/model-discovery.ts` (discoverExternalModels, getExternalModelsCached, cache-warming), `src/core/cache.ts` (cache read/write, 0700/0600 permissions, parseRawEnvelope), `src/core/proxy-log.ts` (scrubChildEnv, openProxyLog, relay env allowlisting), `src/core/proxy-state.ts` (state I/O), `src/cli/commands/proxy.ts` (CLI + hook wiring), `src/cli/commands/agents.ts` (CLI), `src/cli/agents-view/` (TUI — state, render, terminal; thin adapter over the shared `src/cli/tui/` driver). @@ -199,7 +199,7 @@ Per-project runtime files live under `.devflow/`: ~/.devflow/ ├── proxy.json # Proxy runtime state (enabled, port, binPath, configPath, resolvedAt, devflowVersion) — global, not per-project -├── proxy-routing.json # Routing config (port only — bare {port} object) read by the ensure-proxy hook +├── proxy-routing.json # Routing config (port + anthropic.connectTimeoutMs) read by the ensure-proxy hook ├── proxy.pid # Relay PID — written by CLI enable and by the ensure-proxy hook spawn (transient) ├── .proxy-spawn.lock/ # Hook spawn lock dir — prevents concurrent session double-spawn (transient) ├── agent-models.json # Per-agent model overrides (deviations only; absent = shipped default) diff --git a/src/core/proxy-state.ts b/src/core/proxy-state.ts index 6b76ed7d..259ecdb1 100644 --- a/src/core/proxy-state.ts +++ b/src/core/proxy-state.ts @@ -126,22 +126,20 @@ export async function writeProxyState( // --------------------------------------------------------------------------- /** - * Accepted top-level keys for the subswitch 0.3.0 FileConfigSchema (z.strictObject). + * Accepted top-level keys for the subswitch 0.4.0 FileConfigSchema (z.strictObject). * Unknown top-level keys cause a hard relay startup error — never emit them. * - * @D-EFR-4 Subswitch 0.3.0 routing config contract: + * @D-EFR-4 Subswitch 0.4.0 routing config contract: * FileConfigSchema is a z.strictObject with exactly 5 accepted top-level keys: * port, logLevel, anthropic, providers, limits. Unknown keys cause a hard startup * error — the relay refuses to start. anthropic and limits are themselves * strictObject + prefault({}), so they may be partially specified. * - * connectTimeoutMs: in 0.3.0 this is a genuine DNS+TCP connect budget, armed on - * the socket and disarmed on 'connect' — neither the headers phase nor the stream - * phase is bounded. The devflow override to 120 000ms dates from 0.2.0, where the - * same key was applied as an upstream socket *inactivity* timeout whose 10s default - * killed any Anthropic request taking >10s to emit its first byte (confirmed: 99 - * spurious 504s clustered at 10004-10098ms while successful requests ran 25s-99s). - * The override is now only a wider connect budget; a user-specified value always wins. + * connectTimeoutMs bounds only the DNS+TCP connect: it is armed on the socket and + * disarmed on 'connect', so neither the headers phase nor the stream phase is + * bounded by it. The relay's own 10s default is therefore the correct value, and a + * larger budget buys nothing — it only delays the failure when a host is + * unroutable. A user-specified value always wins. */ const ROUTING_CONFIG_ALLOWED_TOP_KEYS = new Set([ 'port', 'logLevel', 'anthropic', 'providers', 'limits', @@ -161,17 +159,24 @@ const ROUTING_CONFIG_ALLOWED_TOP_KEYS = new Set([ * builder sets itself, so dropping it loses nothing. * limits.maxConcurrentRequests — valid in 0.2.0, removed in 0.3.0 (admission gate * removed). + * limits.maxBodyBytes — valid through 0.3.0, renamed to + * limits.maxBufferedBodyBytes in 0.4.0. The old spelling is a registered legacy + * key, so leaving it in place would stop the relay from booting. * * Keys retired before 0.2.0 are deliberately absent: a config that worked against the * version devflow shipped cannot contain them. */ const ROUTING_CONFIG_REJECTED_SUBKEYS: Readonly> = { anthropic: ['streamIdleTimeoutMs'], - limits: ['connectTimeoutMs', 'maxConcurrentRequests'], + limits: ['connectTimeoutMs', 'maxConcurrentRequests', 'maxBodyBytes'], }; -/** Default anthropic.connectTimeoutMs injected when not specified by the user (ms). */ -export const DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS = 120_000; +/** + * Default anthropic.connectTimeoutMs injected when not specified by the user (ms). + * Matches the relay's own default — connectTimeoutMs bounds only DNS+TCP connect, + * so a wider budget would only delay failure against an unroutable host (D-EFR-4). + */ +export const DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS = 10_000; /** * Build the routing config JSON for ~/.devflow/proxy-routing.json. @@ -181,7 +186,7 @@ export const DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS = 120_000; * out unknown top-level keys and every sub-key in ROUTING_CONFIG_REJECTED_SUBKEYS * (each one a hard relay startup error). * - * Injects a default `anthropic.connectTimeoutMs` of 120 000ms when the user + * Injects a default `anthropic.connectTimeoutMs` of 10 000ms when the user * has not set one — see @D-EFR-4 for the rationale. * * If `existingContent` is missing or malformed, falls back to a clean config @@ -217,7 +222,8 @@ export function buildRoutingConfigJson(port: number, existingContent?: string): } // Anthropic block: preserve user settings; inject connectTimeoutMs default when absent. - // @D-EFR-4: the 10s upstream inactivity timeout kills long opus requests. + // @D-EFR-4: connectTimeoutMs is a DNS+TCP connect budget only — it never bounds a + // request that has already connected, however long that request runs. const existingAnthropic = typeof preserved.anthropic === 'object' && preserved.anthropic !== null && diff --git a/tests/external-models.test.ts b/tests/external-models.test.ts index c63bb13f..544d4dc5 100644 --- a/tests/external-models.test.ts +++ b/tests/external-models.test.ts @@ -20,7 +20,7 @@ import { } from '../src/core/external-models.js'; // Literal GPT model IDs — independent of the deleted hardcoded registry. -// These reflect the subswitch@0.2.0 catalog used throughout Phase D tests. +// These reflect the subswitch@0.4.0 catalog used throughout Phase D tests. // applies ADR-003: end-state only — no compatibility imports from deleted exports. const KNOWN_GPT_IDS = ['gpt-5.6-sol', 'gpt-5.6-terra', 'gpt-5.6-luna', 'gpt-5.5']; import { diff --git a/tests/init-proxy.test.ts b/tests/init-proxy.test.ts index c7cf88e1..b97c1ca2 100644 --- a/tests/init-proxy.test.ts +++ b/tests/init-proxy.test.ts @@ -42,7 +42,7 @@ function makeAgentFrontmatter(model: string): string { /** * Known GPT model IDs — literal list so this test file does not depend on the * hardcoded registry in external-models.ts (which is deleted in Commit 9). - * These match the subswitch@0.2.0 catalog used throughout the Phase D tests. + * These match the subswitch@0.4.0 catalog used throughout the Phase D tests. * applies ADR-003: end-state only — no externalModelIds() import. */ const GPT_IDS = ['gpt-5.6-sol', 'gpt-5.6-terra', 'gpt-5.6-luna', 'gpt-5.5']; diff --git a/tests/model-discovery.test.ts b/tests/model-discovery.test.ts index d0a63493..0175a95b 100644 --- a/tests/model-discovery.test.ts +++ b/tests/model-discovery.test.ts @@ -776,7 +776,7 @@ describe('T1: Real-binary — discoverExternalModels with live runtime', () => { const result = await discoverExternalModels(cacheDir, logPath); - // subswitch models --json (0.2.0) is a static registry dump: no auth, no relay, + // subswitch models --json (0.4.0) is a static registry dump: no auth, no relay, // no config file required (verified: exit 0, configFileFound:false). The per-test // cacheDir is freshly created (beforeEach), so no cache hit is possible — source // must be 'live'. Guarding these behind `if (result.known)` would let the test diff --git a/tests/proxy-state.test.ts b/tests/proxy-state.test.ts index f3371f08..39e1956c 100644 --- a/tests/proxy-state.test.ts +++ b/tests/proxy-state.test.ts @@ -251,6 +251,14 @@ describe('buildRoutingConfigJson — base behavior', () => { expect(anthropic.connectTimeoutMs).toBe(DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS); }); + // Pinned by value, not just by symbol: connectTimeoutMs bounds only DNS+TCP connect, + // so the relay's own 10s default is the correct budget. A wider value silently + // delays failure against an unroutable host, and every other assertion in this file + // compares against the imported symbol — which would stay green if the value drifted. + it('injects the relay-default 10s connect budget, not a wider one (D-EFR-4)', () => { + expect(DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS).toBe(10_000); + }); + it('only emits allowed top-level keys (strictObject constraint D-EFR-4)', () => { const allowed = new Set(['port', 'logLevel', 'anthropic', 'providers', 'limits']); const obj = JSON.parse(buildRoutingConfigJson(4141)) as Record; @@ -300,8 +308,8 @@ describe('buildRoutingConfigJson — existing config preservation', () => { expect(limits.maxConcurrent).toBe(10); }); - // Keys that were valid under the previously pinned 0.2.0 runtime and are registered - // legacy keys in 0.3.0 — a hard startup error, not a warning. Carrying a user's own + // Keys that were valid under a previously pinned runtime and are registered legacy + // keys in the current one — a hard startup error, not a warning. Carrying a user's own // proxy-routing.json forward across the upgrade must drop them, or the relay that the // ensure-proxy hook spawns dies on boot every session with no route back. it('strips anthropic.streamIdleTimeoutMs (removed in the pinned runtime)', () => { @@ -328,6 +336,27 @@ describe('buildRoutingConfigJson — existing config preservation', () => { expect(limits.maxConcurrent).toBe(10); }); + it('strips limits.maxBodyBytes (renamed maxBufferedBodyBytes in the pinned runtime)', () => { + const existing = JSON.stringify({ + port: 4141, + limits: { maxBodyBytes: 10_485_760, maxConcurrent: 10 }, + }); + const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; + const limits = obj.limits as Record; + expect(Object.prototype.hasOwnProperty.call(limits, 'maxBodyBytes')).toBe(false); + expect(limits.maxConcurrent).toBe(10); + }); + + it('preserves the current limits.maxBufferedBodyBytes spelling', () => { + const existing = JSON.stringify({ + port: 4141, + limits: { maxBufferedBodyBytes: 10_485_760 }, + }); + const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; + const limits = obj.limits as Record; + expect(limits.maxBufferedBodyBytes).toBe(10_485_760); + }); + it('preserves providers block from existing config', () => { const existing = JSON.stringify({ port: 4141, providers: { openai: { baseUrl: 'https://api.openai.com' } } }); const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; From c98bd060742ffb3ea9a2789891ec71f1806bc432 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 11:32:28 +0300 Subject: [PATCH 15/17] fix(proxy): stop injecting anthropic.connectTimeoutMs into generated configs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The relay's own default (10 s, DNS+TCP connect-only since subswitch 0.3.0) governs when the user has not set a value. Injection was a 0.2.0-era workaround artifact that outlived its purpose once the key's semantics were narrowed to connect-only — a wider budget bought nothing and only delayed failure against an unroutable host. User-set values are still preserved through the existing config-merge path. --- .../external-model-routing/KNOWLEDGE.md | 10 ++-- CHANGELOG.md | 2 +- CLAUDE.md | 4 +- src/core/proxy-state.ts | 50 +++++++------------ tests/proxy-state.test.ts | 43 ++++++---------- 5 files changed, 40 insertions(+), 69 deletions(-) diff --git a/.devflow/features/external-model-routing/KNOWLEDGE.md b/.devflow/features/external-model-routing/KNOWLEDGE.md index 2c9434ab..cc162e19 100644 --- a/.devflow/features/external-model-routing/KNOWLEDGE.md +++ b/.devflow/features/external-model-routing/KNOWLEDGE.md @@ -27,7 +27,7 @@ The routing runtime is an internal package (`subswitch@0.4.0`, exact-pinned in ` | File | Role | |------|------| | `~/.devflow/proxy.json` | Runtime authority. Tolerant-parsed by `readProxyState()`. ENOENT → default disabled state (not an error). Fields: `enabled`, `port`, `binPath`, `configPath`, `resolvedAt`, `devflowVersion`. | -| `~/.devflow/proxy-routing.json` | Routing config written by `buildRoutingConfigJson(port, existingContent?)`. Strict 5-key shape accepted by subswitch 0.4.0: `port`, `logLevel`, `anthropic`, `providers`, `limits`. Always injects `anthropic.connectTimeoutMs: 10000` when absent (D-EFR-4). Strips legacy sub-keys that are hard startup errors in 0.4.0: `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, `limits.maxConcurrentRequests`, `limits.maxBodyBytes` (see `ROUTING_CONFIG_REJECTED_SUBKEYS`). Written before preflight runs on enable. | +| `~/.devflow/proxy-routing.json` | Routing config written by `buildRoutingConfigJson(port, existingContent?)`. Strict 5-key shape accepted by subswitch 0.4.0: `port`, `logLevel`, `anthropic`, `providers`, `limits`. Port-only on a fresh write — no `anthropic` block injected; the relay's own default governs (D-EFR-4). User-set `anthropic` sub-keys are preserved. Strips legacy sub-keys that are hard startup errors in 0.4.0: `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, `limits.maxConcurrentRequests`, `limits.maxBodyBytes` (see `ROUTING_CONFIG_REJECTED_SUBKEYS`). Written before preflight runs on enable. | | `manifest.features.proxy` | Init/uninstall authority. Seeds from prior manifest on re-init (ADR-014). Never in `config.json` — manifest-group by design, same as `ambient`/`hud`/`rules`. | **`isProxyEnabled()` is the sole dormancy authority**: it reads only `proxy.json` (never manifest). This is load-bearing: on preflight failure, `init.ts` converges `proxy.json` to `enabled:false` alongside manifest, hooks, and env — all four artifacts must agree (avoids PF-015). Any path that forces `proxyEnabled=false` must write `proxy.json enabled:false` so that a subsequent `isProxyEnabled()` call in the same process returns false correctly. @@ -37,7 +37,7 @@ The routing runtime is an internal package (`subswitch@0.4.0`, exact-pinned in ` ### Enable path (crash-safe) 1. Read `proxy.json` for the remembered port; `resolvePort(portOption, priorPort)` picks the effective port. `--port` has **no commander default** — omission leaves `portOption` as `undefined` and the remembered port from `proxy.json` wins (TS-1 fix). -2. Write `proxy-routing.json` with the effective port via `buildRoutingConfigJson(port, existingContent)`, which reads the existing file and merges user customisations (logLevel, providers, anthropic sub-keys) while stripping rejected 0.3.0 sub-keys and injecting the connectTimeoutMs default. +2. Write `proxy-routing.json` with the effective port via `buildRoutingConfigJson(port, existingContent)`, which reads the existing file and merges user customisations (logLevel, providers, anthropic sub-keys) while stripping rejected 0.3.0 sub-keys. No `anthropic` block is injected — the relay's own default governs. 3. Run `runProxyPreflight()` (4 ordered checks — ①–④: bin, codex auth, port probe/adoption, settings — see Preflight section). Doctor excluded: a pre-spawn gate is always unsatisfiable on a cold path (D-EFR-2; see Anti-Patterns). 4. On success: write `proxy.json` `enabled:true`. 5. Spawn relay via `spawnRelayAndWaitForPort()` (exported): bounded ≤50×100ms probe loop (5s max). `SpawnRelayResult.spawnedPid` is set when this process spawned the relay; absent on the adopted path. If relay never accepts, `rollbackProxyState` closure writes `proxy.json enabled:false` and returns error. @@ -216,7 +216,7 @@ The old "override confirm" prompt is gone. The merge is purely additive (Devflow - `limits.maxConcurrentRequests` — valid in 0.2.0, removed in 0.3.0 - `limits.maxBodyBytes` — valid through 0.3.0, renamed `limits.maxBufferedBodyBytes` in 0.4.0 -**`anthropic.connectTimeoutMs`** (injected at `DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS = 10_000`ms when absent): bounds only the DNS+TCP connect — armed on the socket, disarmed on the `'connect'` event. It never bounds the headers or stream phase, so it cannot cap a long-running request that has already connected. The value matches the relay's own default; a wider budget would only delay failure against an unroutable host. A user-specified value always wins. +**`anthropic.connectTimeoutMs`**: bounds only the DNS+TCP connect — armed on the socket, disarmed on the `'connect'` event. It never bounds the headers or stream phase, so it cannot cap a long-running request that has already connected. The relay's own 10 s default applies when the user has not set a value; `buildRoutingConfigJson` no longer injects one. A user-specified value is preserved as-is. **Body handling in 0.4.0**: bodies over the buffering window are streamed to Anthropic rather than rejected (route labels `anthropic:streamed` / `anthropic:streamed:unsniffed`, log field `bodyMode`). Only translated (Codex) routes still return 413 `request_too_large`. The 0.2.0-era synthesized 503 `overloaded_error` no longer exists — devflow never depended on it. @@ -420,11 +420,11 @@ A user who hardened `settings.json` to `0600` (to protect `ANTHROPIC_API_KEY`) n - **`classifyCodexAuthReadError` is directly testable**: the ENOENT→`{kind:'absent'}` vs other-error→`{kind:'unreadable'}` classification that used to live inside `runStatus` is now exported from `codex-auth-inspect.ts`. Tests can import and call it with a synthetic error object without needing to mock the filesystem. - **`isProxyEnabled()` is the sole dormancy authority**: it reads only `proxy.json`. On preflight failure in `init.ts`, `proxy.json` is explicitly written to `enabled:false` so `isProxyEnabled()` returns the correct value for the `reapplyAgentMapping` call that follows. Any new code path that forces the proxy off must write this file — relying on manifest alone is insufficient. - **`readInstalledAgentNames` degrades on any error, not just ENOENT**: the catch block is bare (`catch {}`) — EPERM, ENOTDIR, and any other OS error all return an empty set rather than throwing (avoids PF-009). A misconfigured install path must not crash the TUI or `--list`. -- **subswitch 0.3.0 `connectTimeoutMs` semantics changed**: in 0.2.0 it was an inactivity/socket timeout that killed long requests; in 0.3.0 it is strictly a DNS+TCP connect budget (armed on socket, disarmed on `'connect'`). The 120s devflow default is now only a generous connect window, not a keep-alive guard. +- **subswitch 0.3.0 `connectTimeoutMs` semantics changed**: in 0.2.0 it was an inactivity/socket timeout that killed long requests; in 0.3.0 it is strictly a DNS+TCP connect budget (armed on socket, disarmed on `'connect'`). It cannot cap a long-running request that has already connected. `buildRoutingConfigJson` no longer injects a default — the relay's own 10 s budget governs, and a user-set value is preserved. ## Key Files -- `src/core/proxy-state.ts` — ProxyState schema, read/write, `isProxyEnabled()`, `proxyJsonExists()` (evidence discriminator, D-STRIP-1), `resolveProxyBin()`, `buildRoutingConfigJson()` (0.3.0 5-key shape, `ROUTING_CONFIG_REJECTED_SUBKEYS`, connectTimeoutMs inject) +- `src/core/proxy-state.ts` — ProxyState schema, read/write, `isProxyEnabled()`, `proxyJsonExists()` (evidence discriminator, D-STRIP-1), `resolveProxyBin()`, `buildRoutingConfigJson()` (0.4.0 5-key shape, `ROUTING_CONFIG_REJECTED_SUBKEYS`, user-set anthropic keys preserved; no injection) - `src/core/external-models.ts` — `CLAUDE_MODEL_ALIASES` (as const), `ClaudeModelAlias` literal union, `isClaudeModelName()`, `isDormantExternalModel()` (leaf module, no project imports) - `src/core/agent-state.ts` — `AgentState` type, `AGENT_STATE_LABELS` record, `classifyAgentState()` — single vocabulary for the STATE column shared by `--list` and the TUI (applies ADR-013; leaf module, imports only external-models.ts) - `src/core/agent-frontmatter.ts` — pure frontmatter rewriter, `readFrontmatterModel()`, `rewriteAgentFrontmatter()`, `isValidModelName()` (MODEL_NAME_RE — imported by validateSetArgs for zero-spawn charset validation) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11089e65..2e6eb71c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Changed -- **Routing runtime pinned to `subswitch@0.4.0`** (from `0.2.0`). Over-window Anthropic-bound request bodies are now streamed upstream instead of being rejected by the relay, so long prompts no longer fail at the proxy; only translated (Codex) routes still return `413 request_too_large`. `anthropic.connectTimeoutMs` now bounds only the DNS+TCP connect, so the injected default drops from `120000` to `10000` — the relay's own default. A wider budget bounded nothing extra and only delayed failure against an unroutable host. +- **Routing runtime pinned to `subswitch@0.4.0`** (from `0.2.0`). Over-window Anthropic-bound request bodies are now streamed upstream instead of being rejected by the relay, so long prompts no longer fail at the proxy; only translated (Codex) routes still return `413 request_too_large`. `buildRoutingConfigJson` no longer injects `anthropic.connectTimeoutMs` — the relay's own default (10 s, connect-only) governs; user-set values are preserved as before. The injection was a 0.2.0-era workaround artifact that outlived its purpose once the key's semantics were narrowed to DNS+TCP connect only. ### Fixed - **`proxy-routing.json` upgrade safety**: `buildRoutingConfigJson` now strips `limits.maxBodyBytes`, which `subswitch@0.4.0` renamed to `limits.maxBufferedBodyBytes`. The old spelling is a registered legacy key that makes the relay refuse to start, so a hand-edited config carrying it would have killed the relay on every session start — spawned by the `ensure-proxy` hook, with no route back. Joins the existing strips for `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, and `limits.maxConcurrentRequests`. diff --git a/CLAUDE.md b/CLAUDE.md index f1159cd9..1cd33ca2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -63,7 +63,7 @@ Debug logs stored at `~/.devflow/logs/{project-slug}/`. Knowledge write-back is in-command (not a background pipeline): gated by `devflow knowledge --enable/--disable` (flips `knowledge` in feature config); Knowledge agent writes directly at workflow end. -**External Model Routing (Devflow Proxy)**: Routes Devflow agents through GPT models via an OpenAI/Codex subscription using a local relay. Feature state is manifest-gated (like ambient/hud/rules, per ADR-001): `manifest.features.proxy` is the source of truth; `~/.devflow/proxy.json` holds runtime authority (enabled, port, binPath, configPath, resolvedAt, devflowVersion). `~/.devflow/proxy-routing.json` holds the routing config (`port` plus an injected `anthropic.connectTimeoutMs` default, preserving any existing `logLevel`/`anthropic`/`providers`/`limits` blocks; the 0.4.0 routing runtime rejects unrecognised top-level keys and hard-fails on registered legacy keys — `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, `limits.maxConcurrentRequests`, `limits.maxBodyBytes` — which `buildRoutingConfigJson` strips). The `ensure-proxy` hook (SessionStart + UserPromptSubmit, registered/removed by `addProxyHooks`/`removeProxyHooks`) auto-starts the relay when a session begins. `ANTHROPIC_BASE_URL` and `CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT` are injected into (and stripped from) `settings.json` at CLI enable/disable time via `applyProxyEnv`/`stripProxyEnv`, not by the hook (the window var is removed unconditionally on strip; the URL delete is port-scoped to avoid clobbering foreign gateways). Toggle via `devflow proxy --enable/--disable/--status` or via the Advanced init wizard. Enabling runs `runProxyPreflight` (4 checks: bin, codex auth, port, settings), spawns the relay, then gates on a post-spawn doctor verification against the live relay (doctor requires a running relay to pass); on doctor failure the enable rolls back, killing the relay only if it spawned it. Init runs the same preflight but never spawns or runs doctor — the first session's ensure-proxy hook starts the relay; on init preflight failure: warning + force-disabled, init never aborted (avoids PF-009). Disabling reverts agent frontmatter to Claude defaults but preserves the model mapping for re-enable. Default OFF; Advanced-only — never part of Recommended defaults. +**External Model Routing (Devflow Proxy)**: Routes Devflow agents through GPT models via an OpenAI/Codex subscription using a local relay. Feature state is manifest-gated (like ambient/hud/rules, per ADR-001): `manifest.features.proxy` is the source of truth; `~/.devflow/proxy.json` holds runtime authority (enabled, port, binPath, configPath, resolvedAt, devflowVersion). `~/.devflow/proxy-routing.json` holds the routing config (port only on a fresh write; user-set keys in `logLevel`/`anthropic`/`providers`/`limits` blocks are preserved; the 0.4.0 routing runtime rejects unrecognised top-level keys and hard-fails on registered legacy keys — `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, `limits.maxConcurrentRequests`, `limits.maxBodyBytes` — which `buildRoutingConfigJson` strips; no `anthropic` block is injected — the relay's own default governs). The `ensure-proxy` hook (SessionStart + UserPromptSubmit, registered/removed by `addProxyHooks`/`removeProxyHooks`) auto-starts the relay when a session begins. `ANTHROPIC_BASE_URL` and `CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT` are injected into (and stripped from) `settings.json` at CLI enable/disable time via `applyProxyEnv`/`stripProxyEnv`, not by the hook (the window var is removed unconditionally on strip; the URL delete is port-scoped to avoid clobbering foreign gateways). Toggle via `devflow proxy --enable/--disable/--status` or via the Advanced init wizard. Enabling runs `runProxyPreflight` (4 checks: bin, codex auth, port, settings), spawns the relay, then gates on a post-spawn doctor verification against the live relay (doctor requires a running relay to pass); on doctor failure the enable rolls back, killing the relay only if it spawned it. Init runs the same preflight but never spawns or runs doctor — the first session's ensure-proxy hook starts the relay; on init preflight failure: warning + force-disabled, init never aborted (avoids PF-009). Disabling reverts agent frontmatter to Claude defaults but preserves the model mapping for re-enable. Default OFF; Advanced-only — never part of Recommended defaults. **Per-Agent Model Configuration**: User overrides to agent model assignments persist in `~/.devflow/agent-models.json` (deviations only — absent entry = shipped default). `reapplyAgentMapping` runs after every `devflow init` post-install to re-apply user overrides to freshly copied agent files. `revertExternalAgents` reverts all agents to shipped defaults (called on proxy disable and before agent removal on uninstall). GPT model assignments are **dormant** when routing is off — they are stored in `agent-models.json` but not written to agent frontmatter until routing is enabled. Manage via `devflow agents` TUI or `devflow agents --list/--set/--reset`. Core source files: `src/core/agent-frontmatter.ts` (pure rewrite engine), `src/core/agent-models.ts` (schema + apply/revert), `src/core/external-models.ts` (CLAUDE_MODEL_ALIASES, isClaudeModelName, isDormantExternalModel — leaf module), `src/core/model-discovery.ts` (discoverExternalModels, getExternalModelsCached, cache-warming), `src/core/cache.ts` (cache read/write, 0700/0600 permissions, parseRawEnvelope), `src/core/proxy-log.ts` (scrubChildEnv, openProxyLog, relay env allowlisting), `src/core/proxy-state.ts` (state I/O), `src/cli/commands/proxy.ts` (CLI + hook wiring), `src/cli/commands/agents.ts` (CLI), `src/cli/agents-view/` (TUI — state, render, terminal; thin adapter over the shared `src/cli/tui/` driver). @@ -199,7 +199,7 @@ Per-project runtime files live under `.devflow/`: ~/.devflow/ ├── proxy.json # Proxy runtime state (enabled, port, binPath, configPath, resolvedAt, devflowVersion) — global, not per-project -├── proxy-routing.json # Routing config (port + anthropic.connectTimeoutMs) read by the ensure-proxy hook +├── proxy-routing.json # Routing config (port-only by default; user-set anthropic/logLevel/providers/limits preserved) read by the ensure-proxy hook ├── proxy.pid # Relay PID — written by CLI enable and by the ensure-proxy hook spawn (transient) ├── .proxy-spawn.lock/ # Hook spawn lock dir — prevents concurrent session double-spawn (transient) ├── agent-models.json # Per-agent model overrides (deviations only; absent = shipped default) diff --git a/src/core/proxy-state.ts b/src/core/proxy-state.ts index 259ecdb1..79f3ccf4 100644 --- a/src/core/proxy-state.ts +++ b/src/core/proxy-state.ts @@ -135,11 +135,6 @@ export async function writeProxyState( * error — the relay refuses to start. anthropic and limits are themselves * strictObject + prefault({}), so they may be partially specified. * - * connectTimeoutMs bounds only the DNS+TCP connect: it is armed on the socket and - * disarmed on 'connect', so neither the headers phase nor the stream phase is - * bounded by it. The relay's own 10s default is therefore the correct value, and a - * larger budget buys nothing — it only delays the failure when a host is - * unroutable. A user-specified value always wins. */ const ROUTING_CONFIG_ALLOWED_TOP_KEYS = new Set([ 'port', 'logLevel', 'anthropic', 'providers', 'limits', @@ -155,8 +150,8 @@ const ROUTING_CONFIG_ALLOWED_TOP_KEYS = new Set([ * it reachable in a real user's file: * anthropic.streamIdleTimeoutMs — valid in 0.2.0, removed in 0.3.0 (relay no longer * bounds the stream-idle phase on a connected client). - * limits.connectTimeoutMs — moved to anthropic.connectTimeoutMs, which this - * builder sets itself, so dropping it loses nothing. + * limits.connectTimeoutMs — moved to anthropic.connectTimeoutMs in 0.3.0; + * stripping it loses nothing (relay default governs). * limits.maxConcurrentRequests — valid in 0.2.0, removed in 0.3.0 (admission gate * removed). * limits.maxBodyBytes — valid through 0.3.0, renamed to @@ -171,29 +166,19 @@ const ROUTING_CONFIG_REJECTED_SUBKEYS: Readonly) } - : {}; - for (const key of ROUTING_CONFIG_REJECTED_SUBKEYS.anthropic) { - delete existingAnthropic[key]; - } - if (!Object.prototype.hasOwnProperty.call(existingAnthropic, 'connectTimeoutMs')) { - existingAnthropic.connectTimeoutMs = DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS; + ) { + const existingAnthropic = { ...(preserved.anthropic as Record) }; + for (const key of ROUTING_CONFIG_REJECTED_SUBKEYS.anthropic) { + delete existingAnthropic[key]; + } + if (Object.keys(existingAnthropic).length > 0) { + config.anthropic = existingAnthropic; + } } - config.anthropic = existingAnthropic; // Preserve providers block if present. if (preserved.providers !== undefined) { diff --git a/tests/proxy-state.test.ts b/tests/proxy-state.test.ts index 39e1956c..0ad97b42 100644 --- a/tests/proxy-state.test.ts +++ b/tests/proxy-state.test.ts @@ -10,7 +10,8 @@ * - readProxyState: malformed JSON → tolerant default, no throw (TEST-2) * - writeProxyState → readProxyState round-trip (TEST-2) * - Tolerant field parsing: wrong-typed fields self-heal to defaults (TEST-2) - * - buildRoutingConfigJson: exact {port} shape only — no codex key (AC-C4) + * - buildRoutingConfigJson: port-only shape — no injected anthropic block (AC-C4) + * - buildRoutingConfigJson: user-set anthropic.connectTimeoutMs is preserved * - Pre-existing proxy.json with models loads cleanly; key absent after write (AC-C5) * - RUNTIME_VERSION_RE: path-traversal and length-limit rejection (AC-S4) */ @@ -27,7 +28,6 @@ import { proxyJsonExists, DEFAULT_PROXY_PORT, RUNTIME_VERSION_RE, - DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS, } from '../src/core/proxy-state.js'; // --------------------------------------------------------------------------- @@ -216,7 +216,7 @@ describe('readProxyState — wrong-typed fields self-heal to defaults', () => { }); // --------------------------------------------------------------------------- -// buildRoutingConfigJson — port + anthropic.connectTimeoutMs default (AC-C4 / D-EFR-4) +// buildRoutingConfigJson — port-only shape; user-set anthropic keys preserved (AC-C4) // --------------------------------------------------------------------------- describe('buildRoutingConfigJson — base behavior', () => { @@ -243,20 +243,9 @@ describe('buildRoutingConfigJson — base behavior', () => { expect(() => JSON.parse(json)).not.toThrow(); }); - it('injects default anthropic.connectTimeoutMs when no existing config supplied (D-EFR-4)', () => { - const json = buildRoutingConfigJson(4141); - const obj = JSON.parse(json) as Record; - const anthropic = obj.anthropic as Record; - expect(anthropic).toBeDefined(); - expect(anthropic.connectTimeoutMs).toBe(DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS); - }); - - // Pinned by value, not just by symbol: connectTimeoutMs bounds only DNS+TCP connect, - // so the relay's own 10s default is the correct budget. A wider value silently - // delays failure against an unroutable host, and every other assertion in this file - // compares against the imported symbol — which would stay green if the value drifted. - it('injects the relay-default 10s connect budget, not a wider one (D-EFR-4)', () => { - expect(DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS).toBe(10_000); + it('emits no anthropic block when no existing config is supplied (relay default governs)', () => { + const obj = JSON.parse(buildRoutingConfigJson(4141)) as Record; + expect(Object.prototype.hasOwnProperty.call(obj, 'anthropic')).toBe(false); }); it('only emits allowed top-level keys (strictObject constraint D-EFR-4)', () => { @@ -269,14 +258,14 @@ describe('buildRoutingConfigJson — base behavior', () => { }); describe('buildRoutingConfigJson — existing config preservation', () => { - it('user-specified anthropic.connectTimeoutMs wins over default', () => { + it('user-specified anthropic.connectTimeoutMs is preserved (relay default not injected)', () => { const existing = JSON.stringify({ port: 4141, anthropic: { connectTimeoutMs: 30_000 } }); const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; const anthropic = obj.anthropic as Record; expect(anthropic.connectTimeoutMs).toBe(30_000); }); - it('preserves other anthropic fields alongside injected default', () => { + it('preserves other anthropic fields from existing config', () => { // maxUpstreamSockets is a live key in the pinned runtime's AnthropicSchema — // a preservation fixture has to use a shape the relay actually accepts, or it // pins behaviour that would break the relay at startup (avoids PF-043). @@ -284,7 +273,7 @@ describe('buildRoutingConfigJson — existing config preservation', () => { const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; const anthropic = obj.anthropic as Record; expect(anthropic.maxUpstreamSockets).toBe(64); - expect(anthropic.connectTimeoutMs).toBe(DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS); + expect(Object.prototype.hasOwnProperty.call(anthropic, 'connectTimeoutMs')).toBe(false); }); it('preserves logLevel from existing config', () => { @@ -320,9 +309,9 @@ describe('buildRoutingConfigJson — existing config preservation', () => { const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; const anthropic = obj.anthropic as Record; expect(Object.prototype.hasOwnProperty.call(anthropic, 'streamIdleTimeoutMs')).toBe(false); - // Neighbouring valid keys survive the strip. + // Neighbouring valid keys survive the strip; no default is injected. expect(anthropic.maxUpstreamSockets).toBe(64); - expect(anthropic.connectTimeoutMs).toBe(DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS); + expect(Object.prototype.hasOwnProperty.call(anthropic, 'connectTimeoutMs')).toBe(false); }); it('strips limits.maxConcurrentRequests (removed in the pinned runtime)', () => { @@ -376,19 +365,17 @@ describe('buildRoutingConfigJson — existing config preservation', () => { expect(obj.port).toBe(4141); }); - it('malformed existing content falls back cleanly — no throw, port + anthropic default emitted', () => { + it('malformed existing content falls back cleanly — no throw, port-only config emitted', () => { expect(() => buildRoutingConfigJson(4141, '{ bad json !!!')).not.toThrow(); const obj = JSON.parse(buildRoutingConfigJson(4141, '{ bad json !!!')) as Record; expect(obj.port).toBe(4141); - const anthropic = obj.anthropic as Record; - expect(anthropic.connectTimeoutMs).toBe(DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS); + expect(Object.prototype.hasOwnProperty.call(obj, 'anthropic')).toBe(false); }); - it('undefined existingContent falls back to clean defaults', () => { + it('undefined existingContent falls back to port-only config', () => { const obj = JSON.parse(buildRoutingConfigJson(4141, undefined)) as Record; expect(obj.port).toBe(4141); - const anthropic = obj.anthropic as Record; - expect(anthropic.connectTimeoutMs).toBe(DEFAULT_ANTHROPIC_CONNECT_TIMEOUT_MS); + expect(Object.prototype.hasOwnProperty.call(obj, 'anthropic')).toBe(false); }); }); From 34aee33a43b3765ac7bd7e3d69912fc4a8a4ba37 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 11:58:12 +0300 Subject: [PATCH 16/17] fix(proxy): complete subswitch 0.4.0 legacy-key strip list (#313) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend ROUTING_CONFIG_REJECTED_SUBKEYS.limits with the four 0.3.0-era limits.* keys that subswitch 0.4.0 promotes to hard startup errors: maxUpstreamSockets (→ anthropic.maxUpstreamSockets), streamIdleTimeoutMs (→ providers.codex.streamIdleTimeoutMs), requestTimeoutMs (→ providers.codex.requestTimeoutMs), and maxSseEventBytes (→ providers.codex.maxSseEventBytes). Also makes the limits block omit-when-empty (mirrors the existing anthropic branch), deletes the dangling bare ' *' JSDoc residue, and updates the proxy-log.ts provenance comment (three → four env vars, 0.2.0 → 0.4.0). Adds five new tests: one per new key (each asserting a neighbouring valid key survives) and one empty-after-strip test. Co-Authored-By: Claude --- src/core/proxy-log.ts | 6 +++-- src/core/proxy-state.ts | 27 ++++++++++++++++---- tests/proxy-state.test.ts | 53 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 7 deletions(-) diff --git a/src/core/proxy-log.ts b/src/core/proxy-log.ts index 7823c493..dbf6d2bf 100644 --- a/src/core/proxy-log.ts +++ b/src/core/proxy-log.ts @@ -33,8 +33,10 @@ export const PROXY_LOG_TAIL_BYTES = 1_048_576; * (e.g. SUBSWITCH_CONFIG for the relay spawn and doctor spawn). * * applies ADR-003: the prior denylist rationale is gone — the routing runtime - * reads exactly three env vars (ANTHROPIC_API_KEY, FORCE_COLOR, SUBSWITCH_CONFIG). - * Verified by whole-dist grep of the 0.2.0 package. An allowlist is the correct + * reads exactly four env vars (ANTHROPIC_API_KEY, FORCE_COLOR, NO_COLOR, + * SUBSWITCH_CONFIG — NO_COLOR read in dist/tty.js with presence semantics; CI is + * read only by the `init` subcommand which devflow never invokes). + * Verified by whole-dist grep of the 0.4.0 package. An allowlist is the correct * shape: 61 inherited vars → 6. * * HOME is retained: the runtime's loadConfig resolves ~ paths via homedir(). diff --git a/src/core/proxy-state.ts b/src/core/proxy-state.ts index 79f3ccf4..5a68aa9a 100644 --- a/src/core/proxy-state.ts +++ b/src/core/proxy-state.ts @@ -134,7 +134,6 @@ export async function writeProxyState( * port, logLevel, anthropic, providers, limits. Unknown keys cause a hard startup * error — the relay refuses to start. anthropic and limits are themselves * strictObject + prefault({}), so they may be partially specified. - * */ const ROUTING_CONFIG_ALLOWED_TOP_KEYS = new Set([ 'port', 'logLevel', 'anthropic', 'providers', 'limits', @@ -157,13 +156,29 @@ const ROUTING_CONFIG_ALLOWED_TOP_KEYS = new Set([ * limits.maxBodyBytes — valid through 0.3.0, renamed to * limits.maxBufferedBodyBytes in 0.4.0. The old spelling is a registered legacy * key, so leaving it in place would stop the relay from booting. + * limits.maxUpstreamSockets — valid in 0.3.0, moved to anthropic.maxUpstreamSockets + * in 0.4.0's providers.* restructure. + * limits.streamIdleTimeoutMs — valid in 0.3.0, moved to + * providers.codex.streamIdleTimeoutMs in 0.4.0. + * limits.requestTimeoutMs — valid in 0.3.0, moved to + * providers.codex.requestTimeoutMs in 0.4.0. + * limits.maxSseEventBytes — valid in 0.3.0, moved to + * providers.codex.maxSseEventBytes in 0.4.0. * - * Keys retired before 0.2.0 are deliberately absent: a config that worked against the - * version devflow shipped cannot contain them. + * Keys retired before the 0.2.0 baseline are deliberately absent: only keys reachable + * in a config written by a previously pinned devflow version belong on this list. */ const ROUTING_CONFIG_REJECTED_SUBKEYS: Readonly> = { anthropic: ['streamIdleTimeoutMs'], - limits: ['connectTimeoutMs', 'maxConcurrentRequests', 'maxBodyBytes'], + limits: [ + 'connectTimeoutMs', + 'maxConcurrentRequests', + 'maxBodyBytes', + 'maxUpstreamSockets', + 'streamIdleTimeoutMs', + 'requestTimeoutMs', + 'maxSseEventBytes', + ], }; /** @@ -239,7 +254,9 @@ export function buildRoutingConfigJson(port: number, existingContent?: string): for (const key of ROUTING_CONFIG_REJECTED_SUBKEYS.limits) { delete limitsObj[key]; } - config.limits = limitsObj; + if (Object.keys(limitsObj).length > 0) { + config.limits = limitsObj; + } } else { config.limits = preserved.limits; } diff --git a/tests/proxy-state.test.ts b/tests/proxy-state.test.ts index 0ad97b42..27ecefb4 100644 --- a/tests/proxy-state.test.ts +++ b/tests/proxy-state.test.ts @@ -346,6 +346,59 @@ describe('buildRoutingConfigJson — existing config preservation', () => { expect(limits.maxBufferedBodyBytes).toBe(10_485_760); }); + it('strips limits.maxUpstreamSockets (moved to anthropic.maxUpstreamSockets in 0.4.0)', () => { + const existing = JSON.stringify({ + port: 4141, + limits: { maxUpstreamSockets: 128, maxBufferedBodyBytes: 10_485_760 }, + }); + const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; + const limits = obj.limits as Record; + expect(Object.prototype.hasOwnProperty.call(limits, 'maxUpstreamSockets')).toBe(false); + expect(limits.maxBufferedBodyBytes).toBe(10_485_760); + }); + + it('strips limits.streamIdleTimeoutMs (moved to providers.codex.streamIdleTimeoutMs in 0.4.0)', () => { + const existing = JSON.stringify({ + port: 4141, + limits: { streamIdleTimeoutMs: 30_000, maxBufferedBodyBytes: 10_485_760 }, + }); + const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; + const limits = obj.limits as Record; + expect(Object.prototype.hasOwnProperty.call(limits, 'streamIdleTimeoutMs')).toBe(false); + expect(limits.maxBufferedBodyBytes).toBe(10_485_760); + }); + + it('strips limits.requestTimeoutMs (moved to providers.codex.requestTimeoutMs in 0.4.0)', () => { + const existing = JSON.stringify({ + port: 4141, + limits: { requestTimeoutMs: 60_000, maxBufferedBodyBytes: 10_485_760 }, + }); + const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; + const limits = obj.limits as Record; + expect(Object.prototype.hasOwnProperty.call(limits, 'requestTimeoutMs')).toBe(false); + expect(limits.maxBufferedBodyBytes).toBe(10_485_760); + }); + + it('strips limits.maxSseEventBytes (moved to providers.codex.maxSseEventBytes in 0.4.0)', () => { + const existing = JSON.stringify({ + port: 4141, + limits: { maxSseEventBytes: 65_536, maxBufferedBodyBytes: 10_485_760 }, + }); + const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; + const limits = obj.limits as Record; + expect(Object.prototype.hasOwnProperty.call(limits, 'maxSseEventBytes')).toBe(false); + expect(limits.maxBufferedBodyBytes).toBe(10_485_760); + }); + + it('omits limits key from output when all sub-keys are stripped (mirrors anthropic omit-when-empty)', () => { + const existing = JSON.stringify({ + port: 4141, + limits: { maxUpstreamSockets: 128, streamIdleTimeoutMs: 30_000 }, + }); + const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; + expect(Object.prototype.hasOwnProperty.call(obj, 'limits')).toBe(false); + }); + it('preserves providers block from existing config', () => { const existing = JSON.stringify({ port: 4141, providers: { openai: { baseUrl: 'https://api.openai.com' } } }); const obj = JSON.parse(buildRoutingConfigJson(4141, existing)) as Record; From 93f14ef02bf902acf6fecaac3de6b6e78c2cd131 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 11:58:20 +0300 Subject: [PATCH 17/17] docs: sync 0.4.0 alignment across CLAUDE.md, KNOWLEDGE.md, index.md, CHANGELOG.md (#313) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CLAUDE.md: expand legacy-key strip list to all 7 limits.* sub-keys; replace stale "window var removed unconditionally" with the accurate evidence-gated strip description; add buildRoutingConfigJson port-only fresh-write note; document applyProxyTeardownToSettings, proxyJsonExists gating, foreign-gateway preflight, and binPath re-resolution. - KNOWLEDGE.md anti-pattern bullet: re-stamp from subswitch 0.3.0 to 0.4.0 and list all 7 stripped limits sub-keys. - index.md: keyword tail subswitch 0.3.0 → 0.4.0. - CHANGELOG.md: fold 4 additional strips into the existing Fixed entry; extend Upgrade note with the 7-key complete list. Co-Authored-By: Claude --- .devflow/features/external-model-routing/KNOWLEDGE.md | 2 +- .devflow/features/index.md | 2 +- CHANGELOG.md | 4 ++-- CLAUDE.md | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.devflow/features/external-model-routing/KNOWLEDGE.md b/.devflow/features/external-model-routing/KNOWLEDGE.md index cc162e19..687aa303 100644 --- a/.devflow/features/external-model-routing/KNOWLEDGE.md +++ b/.devflow/features/external-model-routing/KNOWLEDGE.md @@ -401,7 +401,7 @@ A user who hardened `settings.json` to `0600` (to protect `ANTHROPIC_API_KEY`) n - **D-EFR-3: Never mock the routing-runtime subprocess without a paired real-binary test**: any test that mocks the routing-runtime subprocess must be paired with at least one CI-executed test that does not. The specific trap (PF-016 reproduced exactly): `tests/integration/**` is excluded from `npm test` by `vitest.config.ts` while CI runs only `npm run build && npm test` — a real-binary test placed in `tests/integration/` would never execute in CI. Place real-binary tests in `tests/` (not `tests/integration/`). - **Calling model discovery from `validateSetArgs` or `--list`/`--reset`**: `validateSetArgs` uses `isValidModelName` (from agent-frontmatter.ts, pure regex) — not model discovery. The zero-spawn constraint for `--list`, `--set`, and `--reset` is a firm requirement pinned by module-boundary spy tests in `tests/agents-command.test.ts`. Importing or calling `discoverExternalModels`/`getExternalModelsCached` from the validation path breaks these tests and violates the configure-first-then-enable flow. - **Putting the agent-state classifier in external-models.ts**: `external-models.ts` is a leaf module with a single responsibility (dormancy predicate + Claude alias set). Agent state classification belongs in `src/core/agent-state.ts` (applies ADR-013). Adding classifier logic to external-models.ts would give it two reasons to change and break its leaf-module contract. -- **Emitting 0.2.0-era sub-keys into proxy-routing.json**: `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, and `limits.maxConcurrentRequests` are hard startup errors in subswitch 0.3.0. Always build the config via `buildRoutingConfigJson` — never construct the JSON manually. +- **Emitting legacy sub-keys into proxy-routing.json**: `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, `limits.maxConcurrentRequests`, `limits.maxBodyBytes`, `limits.maxUpstreamSockets`, `limits.streamIdleTimeoutMs`, `limits.requestTimeoutMs`, and `limits.maxSseEventBytes` are hard startup errors in subswitch 0.4.0. Always build the config via `buildRoutingConfigJson` — never construct the JSON manually. - **Gating env strip on `isProxyEnabled()` instead of `proxyJsonExists()`**: `readProxyState()` returns `Ok(defaultState)` on ENOENT — it cannot distinguish absence from a present file at the default port. Env strip must be gated on `proxyJsonExists()` (D-STRIP-1) to avoid clobbering a foreign gateway on machines that never had the proxy. - **Skipping check ④ on the adopted-relay path**: a healthy relay on the port does not exempt the caller from the foreign-gateway refusal. `checkSettingsEnv(deps, port)` must run before any early-return on adoption (D-EFR-5). diff --git a/.devflow/features/index.md b/.devflow/features/index.md index 14902fb7..cf1948ce 100644 --- a/.devflow/features/index.md +++ b/.devflow/features/index.md @@ -4,5 +4,5 @@ - **resolve-pipeline** — src/assets/commands/resolve.mds, src/assets/agents/triage.md, src/assets/agents/code.md, src/core/plugins.ts, src/assets/commands/code-review.mds — Use when modifying /resolve or /code-review convergence logic, adding or changing Triage disposition rules (including DUPLICATE collapsing), adjusting Code-agent operating modes (issue-fix/validation-fix), touching the resolution-summary.md parser contract, changing the Verification Gate retry loop, understanding how DIFF_FILES flows from git validate-branch into blast-radius triage, or working on traceability operations (fetch-review-threads, resolve-review-threads, post-resolution-summary, check-merge-readiness, THREAD_MAP). Keywords: resolve, triage, disposition matrix, blast-radius, FIX_NOW, FIX_SEPARATE, TECH_DEBT, FALSE_POSITIVE, BY_DESIGN, ESCALATED, DUPLICATE, duplicate-grouping, duplicates-collapse, duplicate_of, resolution-summary, convergence parser, DIFF_FILES, issue-fix, validation-fix, Verification Gate, manage-debt, COMPLIANCE_SKILL_INSTALLED, TRACEABILITY DEGRADED, fetch-review-threads, THREAD_MAP, post-resolution-summary, Third-Party Threads, check-merge-readiness, ext-N, D7, D9, PF-024. - **installer-shadowing** — src/targets/claude-code/installer.ts, src/targets/claude-code/legacy.ts, src/cli/commands/init.ts, src/cli/commands/init-seed.ts, src/cli/commands/uninstall.ts, src/cli/commands/rules.ts, src/cli/commands/skills.ts, src/cli/commands/flags.ts, src/cli/flags-view, src/cli/tui, src/core/plugins.ts, src/core/assets.ts, src/core/paths.ts, src/core/manifest.ts, src/core/flags.ts, src/core/feature-config.ts, src/core/orphan-sweep.ts, src/core/migrations.ts, src/cli/commands/compliance-prompts.ts — Use when modifying the install pipeline (installViaFileCopy, installAllRules, composeScripts, InstallReport), adding or changing skill/rule shadow override logic, touching uninstall scope (enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, sweepDevflowNamespaces, resolveProjectDataCleanup) or install-artifact cleanup, extending the CLI skills/rules/flags management commands, working with asset directory accessors (rulesDir, skillsDir, commandsDir) and package-root resolution, modifying the init seeding layer (resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, --reset, FlagsRecord, knownPlugins, readConfigIfPresent, resolveExistingViewMode, getAllCommandNames, proxy), working on the flags TUI (FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, effectiveDisplay, blurb, inline mode, RunTuiSpec screen) or the flags CLI (createFlagsCommand, lookupFlag, persistFlagConfig, formatFlagValue), or working on the compliance wizard step (shouldRunComplianceStep, runComplianceStep, modePromptShown, CompliancePromptIO). Keywords: installViaFileCopy, installAllRules, composeScripts, InstallReport, RuleInstallOutcome, SkillShadowState, RuleShadowState, shadow, unshadow, validateSkillShadow, validateRuleShadow, seedRuleShadow, prefixSkillName, unprefixSkillName, devflow:, skills, rules, uninstall, EISDIR, enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, enumerateDryRunExtras, sweepDevflowNamespaces, resolveProjectDataCleanup, runDryRunPhase, runSelectivePhaseForScope, runFullPhaseForScope, runCleanupPhase, getPackageRoot, isContainedIn, rulesDir, skillsDir, agentsDir, commandsDir, scriptsDir, LEGACY_SKILL_NAMES, sweepOrphanedAssets, SweepResult, sweepOrphans, sweepFailures, SweepFailure, mdFileName, mdEntryName, orphan sweep, getAllSkillNames, getAllCommandNames, getAllAgentNames, DELETED_PLUGIN_NAMES, EXCLUDED, resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, resolveResetGatedInputs, applyCliToggles, FlagsRecord, FlagsRecordValue, getDefaultFlagsRecord, parseManifestFlags, migrateLegacyFlagsToRecord, sanitizeFlagsRecord, coerceFlagValue, parseFlagValueInput, neutralValueOf, isNeutral, countActiveFlags, readViewMode, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveFinalViewMode, reset, init-seed, proxy, reapplyAgentMapping, revertExternalAgents, agent-models.json, proxy.json, proxy-routing.json, proxy.pid, applyDisableToSettings, buildRealPreflightDeps, canonicalise-agent-keys-v1, AnyMigration, migrations.json, compliance-prompts, shouldRunComplianceStep, CompliancePromptIO, runComplianceStep, modePromptShown, createFlagsCommand, lookupFlag, persistFlagConfig, FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, buildStops, cycleForward, cycleBackward, sanitizeCell, padToVisible, truncateVisible, effectiveDisplay, EffectiveDisplay, formatFlagValue, blurb, FlagDefCommon, INLINE_MARGIN, cursorUp, RunTuiSpec, screen, inline. - **learning-capture-system** — src/assets/scripts/hooks, src/assets/agents/learning.md, src/cli/commands/learning.ts, src/core/feature-config.ts, src/core/learning-tuning-config.ts, src/hud/components/learning-counts.ts, src/assets/commands/_partials — Use when modifying capture hooks (capture-prompt/capture-turn/capture-question), the learning or memory pending-turns queues, the Learning agent (src/assets/agents/learning.md), the session-start-context learning directive, the feature-config toggles, the learning tuning config, the decisions content files (decisions.md/pitfalls.md/index.md) or their ledger ops, or the devflow learning CLI. Keywords: capture-prompt, capture-turn, capture-question, queue-append, pending-turns, memory-worker, Learning agent, learning directive, LEARNING MAINTENANCE, DEVFLOW_BG_UPDATER, learning-lock, queue_read_gates, decisions_load, DECISIONS_CONTEXT, feature-config, config.json, learning.json, decisions-ledger, assign-anchor, retire-anchor, refresh-anchor, render-decisions, staged-write CAS, WORKING-MEMORY.md.new, segmentDetails, amendments, is-hex-sha, verify_and_swap, compute_commits_since_note, divergence guard, isSafeRawBody. -- **external-model-routing** — src/core/proxy-state.ts, src/core/external-models.ts, src/core/agent-models.ts, src/core/agent-state.ts, src/core/agent-frontmatter.ts, src/core/codex-auth-inspect.ts, src/core/model-discovery.ts, src/core/cache.ts, src/core/proxy-log.ts, src/cli/commands/proxy.ts, src/cli/commands/agents.ts, src/cli/agents-view, src/cli/tui — Use when working on the proxy lifecycle (enable/disable/status/preflight), the ensure-proxy hook, per-agent model mapping, agent frontmatter rewriting, or the agents TUI. Keywords: proxy, external-model-routing, GPT, agent-models, ensure-proxy, frontmatter, devflow proxy, devflow agents, subswitch, ANTHROPIC_BASE_URL, dormancy, reapplyAgentMapping, proxyJsonExists, applyProxyTeardownToSettings, D-STRIP-1, mergeDevflowSettingsTemplate, subswitch 0.3.0. +- **external-model-routing** — src/core/proxy-state.ts, src/core/external-models.ts, src/core/agent-models.ts, src/core/agent-state.ts, src/core/agent-frontmatter.ts, src/core/codex-auth-inspect.ts, src/core/model-discovery.ts, src/core/cache.ts, src/core/proxy-log.ts, src/cli/commands/proxy.ts, src/cli/commands/agents.ts, src/cli/agents-view, src/cli/tui — Use when working on the proxy lifecycle (enable/disable/status/preflight), the ensure-proxy hook, per-agent model mapping, agent frontmatter rewriting, or the agents TUI. Keywords: proxy, external-model-routing, GPT, agent-models, ensure-proxy, frontmatter, devflow proxy, devflow agents, subswitch, ANTHROPIC_BASE_URL, dormancy, reapplyAgentMapping, proxyJsonExists, applyProxyTeardownToSettings, D-STRIP-1, mergeDevflowSettingsTemplate, subswitch 0.4.0. - **compliance-feature** — src/core/compliance.ts, src/targets/claude-code/compliance-install.ts, src/cli/commands/compliance.ts, src/assets/skills/compliance, src/assets/rules/compliance.md, src/assets/agents/git.md, src/assets/commands/code-review.mds, src/assets/commands/plan.mds, src/assets/commands/implement.mds, src/assets/commands/resolve.mds, src/assets/commands/release.md — Use when adding or modifying the compliance feature (framework registry, converge contract, CLI, rule stamping), changing how host commands resolve COMPLIANCE_SKILL_INSTALLED, modifying traceability operations in the Git agent (learn-conventions, issue-first, thread resolution, shipped markers, release evidence), or extending the D4 DEGRADED contract. Keywords: compliance, COMPLIANCE_SKILL_INSTALLED, convergeComplianceArtifacts, convergeFromManifest, frameworks, FEATURE_OWNED_SKILLS, traceability, D4, D9, gather-release-evidence, conventions.md, resolve-review-threads, ensure-traceable-issue, stamper, manifest-group, ComplianceFeatureState. diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e6eb71c..4fe9ebb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,9 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Routing runtime pinned to `subswitch@0.4.0`** (from `0.2.0`). Over-window Anthropic-bound request bodies are now streamed upstream instead of being rejected by the relay, so long prompts no longer fail at the proxy; only translated (Codex) routes still return `413 request_too_large`. `buildRoutingConfigJson` no longer injects `anthropic.connectTimeoutMs` — the relay's own default (10 s, connect-only) governs; user-set values are preserved as before. The injection was a 0.2.0-era workaround artifact that outlived its purpose once the key's semantics were narrowed to DNS+TCP connect only. ### Fixed -- **`proxy-routing.json` upgrade safety**: `buildRoutingConfigJson` now strips `limits.maxBodyBytes`, which `subswitch@0.4.0` renamed to `limits.maxBufferedBodyBytes`. The old spelling is a registered legacy key that makes the relay refuse to start, so a hand-edited config carrying it would have killed the relay on every session start — spawned by the `ensure-proxy` hook, with no route back. Joins the existing strips for `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, and `limits.maxConcurrentRequests`. +- **`proxy-routing.json` upgrade safety**: `buildRoutingConfigJson` now strips all seven `limits.*` sub-keys that `subswitch@0.4.0` promotes to hard startup errors: `limits.maxBodyBytes` (renamed `limits.maxBufferedBodyBytes`), `limits.maxUpstreamSockets` (moved to `anthropic.maxUpstreamSockets`), `limits.streamIdleTimeoutMs` (moved to `providers.codex.streamIdleTimeoutMs`), `limits.requestTimeoutMs` (moved to `providers.codex.requestTimeoutMs`), and `limits.maxSseEventBytes` (moved to `providers.codex.maxSseEventBytes`), joining the existing strips for `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, and `limits.maxConcurrentRequests`. A hand-edited config carrying any of these keys would have killed the relay on every session start — spawned by the `ensure-proxy` hook, with no route back. The `limits` block is now omitted entirely when all its sub-keys are stripped (matching the `anthropic` block's existing omit-when-empty behaviour). -**Upgrade**: no action required. If you hand-edited `~/.devflow/proxy-routing.json` to set `limits.maxBodyBytes`, re-set it as `limits.maxBufferedBodyBytes`; the next `devflow proxy --enable` drops the stale key for you. +**Upgrade**: no action required. If you hand-edited `~/.devflow/proxy-routing.json` to set any of the seven stripped `limits.*` keys, move their values to the 0.4.0 target paths (`limits.maxBufferedBodyBytes`, `anthropic.maxUpstreamSockets`, or the appropriate `providers.codex.*` key); the next `devflow proxy --enable` drops the stale keys for you. --- diff --git a/CLAUDE.md b/CLAUDE.md index 1cd33ca2..6a2fa177 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -63,7 +63,7 @@ Debug logs stored at `~/.devflow/logs/{project-slug}/`. Knowledge write-back is in-command (not a background pipeline): gated by `devflow knowledge --enable/--disable` (flips `knowledge` in feature config); Knowledge agent writes directly at workflow end. -**External Model Routing (Devflow Proxy)**: Routes Devflow agents through GPT models via an OpenAI/Codex subscription using a local relay. Feature state is manifest-gated (like ambient/hud/rules, per ADR-001): `manifest.features.proxy` is the source of truth; `~/.devflow/proxy.json` holds runtime authority (enabled, port, binPath, configPath, resolvedAt, devflowVersion). `~/.devflow/proxy-routing.json` holds the routing config (port only on a fresh write; user-set keys in `logLevel`/`anthropic`/`providers`/`limits` blocks are preserved; the 0.4.0 routing runtime rejects unrecognised top-level keys and hard-fails on registered legacy keys — `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, `limits.maxConcurrentRequests`, `limits.maxBodyBytes` — which `buildRoutingConfigJson` strips; no `anthropic` block is injected — the relay's own default governs). The `ensure-proxy` hook (SessionStart + UserPromptSubmit, registered/removed by `addProxyHooks`/`removeProxyHooks`) auto-starts the relay when a session begins. `ANTHROPIC_BASE_URL` and `CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT` are injected into (and stripped from) `settings.json` at CLI enable/disable time via `applyProxyEnv`/`stripProxyEnv`, not by the hook (the window var is removed unconditionally on strip; the URL delete is port-scoped to avoid clobbering foreign gateways). Toggle via `devflow proxy --enable/--disable/--status` or via the Advanced init wizard. Enabling runs `runProxyPreflight` (4 checks: bin, codex auth, port, settings), spawns the relay, then gates on a post-spawn doctor verification against the live relay (doctor requires a running relay to pass); on doctor failure the enable rolls back, killing the relay only if it spawned it. Init runs the same preflight but never spawns or runs doctor — the first session's ensure-proxy hook starts the relay; on init preflight failure: warning + force-disabled, init never aborted (avoids PF-009). Disabling reverts agent frontmatter to Claude defaults but preserves the model mapping for re-enable. Default OFF; Advanced-only — never part of Recommended defaults. +**External Model Routing (Devflow Proxy)**: Routes Devflow agents through GPT models via an OpenAI/Codex subscription using a local relay. Feature state is manifest-gated (like ambient/hud/rules, per ADR-001): `manifest.features.proxy` is the source of truth; `~/.devflow/proxy.json` holds runtime authority (enabled, port, binPath, configPath, resolvedAt, devflowVersion). `~/.devflow/proxy-routing.json` holds the routing config; `buildRoutingConfigJson` writes port-only on a fresh write (no `anthropic` block injected — the relay's own default governs); user-set keys in `logLevel`/`anthropic`/`providers`/`limits` blocks are preserved from existing content; the 0.4.0 routing runtime rejects unrecognised top-level keys and hard-fails on registered legacy keys — `anthropic.streamIdleTimeoutMs`, `limits.connectTimeoutMs`, `limits.maxConcurrentRequests`, `limits.maxBodyBytes`, `limits.maxUpstreamSockets`, `limits.streamIdleTimeoutMs`, `limits.requestTimeoutMs`, `limits.maxSseEventBytes` — which `buildRoutingConfigJson` strips. The `ensure-proxy` hook (SessionStart + UserPromptSubmit, registered/removed by `addProxyHooks`/`removeProxyHooks`) auto-starts the relay when a session begins; hook removal is unconditional on disable. `ANTHROPIC_BASE_URL` and `CLAUDE_CODE_DISABLE_UNKNOWN_MODEL_WINDOW_ENFORCEMENT` are injected into (and stripped from) `settings.json` at CLI enable/disable time via `applyProxyEnv`/`applyProxyTeardownToSettings`, not by the hook; env stripping is evidence-gated on `~/.devflow/proxy.json` existing (`proxyJsonExists()`); the URL delete is port-scoped to avoid clobbering foreign gateways; preflight refuses a foreign `ANTHROPIC_BASE_URL` before adopting a healthy relay; the `ensure-proxy` hook re-resolves a missing `binPath` before warning. Toggle via `devflow proxy --enable/--disable/--status` or via the Advanced init wizard. Enabling runs `runProxyPreflight` (4 checks: bin, codex auth, port, settings), spawns the relay, then gates on a post-spawn doctor verification against the live relay (doctor requires a running relay to pass); on doctor failure the enable rolls back, killing the relay only if it spawned it. Init runs the same preflight but never spawns or runs doctor — the first session's ensure-proxy hook starts the relay; on init preflight failure: warning + force-disabled, init never aborted (avoids PF-009). Disabling reverts agent frontmatter to Claude defaults but preserves the model mapping for re-enable. Default OFF; Advanced-only — never part of Recommended defaults. **Per-Agent Model Configuration**: User overrides to agent model assignments persist in `~/.devflow/agent-models.json` (deviations only — absent entry = shipped default). `reapplyAgentMapping` runs after every `devflow init` post-install to re-apply user overrides to freshly copied agent files. `revertExternalAgents` reverts all agents to shipped defaults (called on proxy disable and before agent removal on uninstall). GPT model assignments are **dormant** when routing is off — they are stored in `agent-models.json` but not written to agent frontmatter until routing is enabled. Manage via `devflow agents` TUI or `devflow agents --list/--set/--reset`. Core source files: `src/core/agent-frontmatter.ts` (pure rewrite engine), `src/core/agent-models.ts` (schema + apply/revert), `src/core/external-models.ts` (CLAUDE_MODEL_ALIASES, isClaudeModelName, isDormantExternalModel — leaf module), `src/core/model-discovery.ts` (discoverExternalModels, getExternalModelsCached, cache-warming), `src/core/cache.ts` (cache read/write, 0700/0600 permissions, parseRawEnvelope), `src/core/proxy-log.ts` (scrubChildEnv, openProxyLog, relay env allowlisting), `src/core/proxy-state.ts` (state I/O), `src/cli/commands/proxy.ts` (CLI + hook wiring), `src/cli/commands/agents.ts` (CLI), `src/cli/agents-view/` (TUI — state, render, terminal; thin adapter over the shared `src/cli/tui/` driver).