From eb33ee6bb3b1a43faf4cf48c0893205d2c89141e Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Sun, 6 Sep 2026 01:46:01 -0700 Subject: [PATCH] refactor(native-chat): name the structured launch option seed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The create-intent resolver narrowed saved options to model/effort with an inline key literal, inside a file carrying @ts-nocheck — so neither the key list nor the string narrowing had a typechecked or testable home, and the repo already expresses this concept as a named shared shape. Move it to resolveStructuredLaunchSeedOptions beside the persisted settings it reads, where it is typechecked and unit-tested, and document why the seed is exactly model and effort: they are the only ids the picker persists that both providers also accept as strings. No behavior change. Adds coverage for a non-string persisted effort, which settings.json can hold and the durable record must not carry. --- ...lve-recovered-structured-tui-transcript.ts | 13 ++--- ...ative-chat-session-option-defaults.test.ts | 54 +++++++++++++++++++ .../native-chat-session-option-defaults.ts | 27 ++++++++++ 3 files changed, 84 insertions(+), 10 deletions(-) diff --git a/src/main/runtime/orca-runtime-resolve-recovered-structured-tui-transcript.ts b/src/main/runtime/orca-runtime-resolve-recovered-structured-tui-transcript.ts index b66f3ee04ac..497d5b381e7 100644 --- a/src/main/runtime/orca-runtime-resolve-recovered-structured-tui-transcript.ts +++ b/src/main/runtime/orca-runtime-resolve-recovered-structured-tui-transcript.ts @@ -13,7 +13,7 @@ import { getLocalProjectWorktreeGitOptions } from '../project-runtime-git-option import type { AgentSessionAttachParams } from '../native-chat/agent-session-wire/structured-agent-session-attach' import { getSystemCodexHomePath } from '../codex/codex-home-paths' import { resolveTuiAgentLaunchEnv } from '../../shared/tui-agent-launch-defaults' -import { resolveNativeChatSessionOptionDefaults } from '../../shared/native-chat-session-option-defaults' +import { resolveStructuredLaunchSeedOptions } from '../../shared/native-chat-session-option-defaults' import { hasPersistedStructuredAgentSessionStore as hasPersistedStructuredAgentSessionStoreOnDisk } from './structured-agent-session-runtime' import { getProfileUserDataPath } from '../orca-profiles/profile-storage-paths' import { homedir } from 'node:os' @@ -162,17 +162,10 @@ export class OrcaRuntimeWithResolveRecoveredStructuredTuiTranscript extends Orca } const settings = this.requireStore().getSettings() const launchEnv = resolveTuiAgentLaunchEnv(input.agent, settings.agentDefaultEnv) - const defaults = resolveNativeChatSessionOptionDefaults( + const options = resolveStructuredLaunchSeedOptions( settings.nativeChatSessionOptions, input.agent ) - const options = defaults - ? Object.fromEntries( - ['model', 'effort'].flatMap((key) => - typeof defaults[key] === 'string' ? [[key, defaults[key]]] : [] - ) - ) - : undefined const location = await this.resolveStructuredAgentSessionLocation(input.worktree) const workspacePath = (await this.resolveRuntimeFileTarget(input.worktree)).worktree.path return { @@ -189,7 +182,7 @@ export class OrcaRuntimeWithResolveRecoveredStructuredTuiTranscript extends Orca variable: input.agent === 'claude' ? 'CLAUDE_CONFIG_DIR' : 'CODEX_HOME', path: await resolveAccountHomePath({ workspacePath, launchEnv, location }) }, - ...(options && Object.keys(options).length > 0 ? { options } : {}), + ...(options ? { options } : {}), runtimeKind: 'native' } } diff --git a/src/shared/native-chat-session-option-defaults.test.ts b/src/shared/native-chat-session-option-defaults.test.ts index aed767d67ac..eb197683bdf 100644 --- a/src/shared/native-chat-session-option-defaults.test.ts +++ b/src/shared/native-chat-session-option-defaults.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import { clearNativeChatSessionOptionModel, resolveNativeChatSessionOptionDefaults, + resolveStructuredLaunchSeedOptions, updateNativeChatSessionOptionDefaults } from './native-chat-session-option-defaults' import type { PersistedNativeChatSessionOptions } from './native-chat-session-options' @@ -74,3 +75,56 @@ describe('resolveNativeChatSessionOptionDefaults', () => { }) }) }) + +describe('resolveStructuredLaunchSeedOptions', () => { + const persistedCodex = ( + valuesByModel: Record> + ): PersistedNativeChatSessionOptions => + ({ codex: { model: 'gpt-5.6-sol', valuesByModel } }) as PersistedNativeChatSessionOptions + + it('seeds the saved model and effort a structured create must apply', () => { + expect( + resolveStructuredLaunchSeedOptions( + persistedCodex({ 'gpt-5.6-sol': { effort: 'medium' } }), + 'codex' + ) + ).toEqual({ model: 'gpt-5.6-sol', effort: 'medium' }) + }) + + it('drops ids the providers only accept mid-session', () => { + // `fastMode` is a boolean and `personality` is settable only mid-session; + // neither belongs in the reservation's Record. + expect( + resolveStructuredLaunchSeedOptions( + persistedCodex({ + 'gpt-5.6-sol': { effort: 'high', fastMode: true, personality: 'concise' } + }), + 'codex' + ) + ).toEqual({ model: 'gpt-5.6-sol', effort: 'high' }) + }) + + it('drops a seeded id whose persisted value is not a usable string', () => { + // settings.json is user-writable, so a non-string `effort` must not reach a + // record typed Record and be emitted as a turn option. + expect( + resolveStructuredLaunchSeedOptions( + persistedCodex({ 'gpt-5.6-sol': { effort: true } }), + 'codex' + ) + ).toEqual({ model: 'gpt-5.6-sol' }) + expect( + resolveStructuredLaunchSeedOptions( + persistedCodex({ 'gpt-5.6-sol': { effort: ' ' } }), + 'codex' + ) + ).toEqual({ model: 'gpt-5.6-sol' }) + }) + + it('seeds nothing until a model is picked, so the CLI default survives', () => { + expect(resolveStructuredLaunchSeedOptions(undefined, 'codex')).toBeUndefined() + expect( + resolveStructuredLaunchSeedOptions({ codex: { valuesByModel: {} } }, 'codex') + ).toBeUndefined() + }) +}) diff --git a/src/shared/native-chat-session-option-defaults.ts b/src/shared/native-chat-session-option-defaults.ts index 238005ccd62..41f607ca12a 100644 --- a/src/shared/native-chat-session-option-defaults.ts +++ b/src/shared/native-chat-session-option-defaults.ts @@ -28,6 +28,33 @@ export function resolveNativeChatSessionOptionDefaults( return values } +/** Why only these two: they are the only ids the picker persists into + * `nativeChatSessionOptions` that both structured providers also accept as + * strings. Claude's `fastMode` is a boolean the durable `Record` + * record cannot carry, and the providers' remaining keys are settable only + * mid-session, never seeded at launch. */ +const STRUCTURED_LAUNCH_SEED_OPTION_IDS = ['model', 'effort'] as const + +/** The saved selection a structured create seeds into its reservation, narrowed + * to the wire-safe string subset the durable record and both providers accept. */ +export function resolveStructuredLaunchSeedOptions( + persisted: PersistedNativeChatSessionOptions | null | undefined, + agent: AgentType +): Record | undefined { + const defaults = resolveNativeChatSessionOptionDefaults(persisted, agent) + if (!defaults) { + return undefined + } + const seeded: Record = {} + for (const id of STRUCTURED_LAUNCH_SEED_OPTION_IDS) { + const value = defaults[id] + if (typeof value === 'string' && value.trim()) { + seeded[id] = value + } + } + return Object.keys(seeded).length > 0 ? seeded : undefined +} + /** Why: an authoritative probe proved this id gone, and a stale `model` is emitted * verbatim as a launch flag — grok exits fatally on an unknown one. Dropping only * `model` keeps the per-model option values for a later reselect. */