From f958e5b09e0d00dd701b1354d622db612ae58bf0 Mon Sep 17 00:00:00 2001 From: chr1syy Date: Sun, 1 Mar 2026 10:32:48 +0100 Subject: [PATCH 1/3] tab: enforce deterministic model arg and improve tab-name extraction --- src/main/ipc/handlers/tabNaming.ts | 297 +++++++++++++++++++++++++++-- 1 file changed, 286 insertions(+), 11 deletions(-) diff --git a/src/main/ipc/handlers/tabNaming.ts b/src/main/ipc/handlers/tabNaming.ts index 6eeb2b248..c2b0c952c 100644 --- a/src/main/ipc/handlers/tabNaming.ts +++ b/src/main/ipc/handlers/tabNaming.ts @@ -122,26 +122,119 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v const baseArgs = (agent.args ?? []).filter( (arg) => arg !== '--dangerously-skip-permissions' ); + + // Fetch stored agent config values (user overrides) early so we can + // prefer the configured model when building args for the tab naming call. + const allConfigs = agentConfigsStore.get('configs', {}); + const agentConfigValues = allConfigs[config.agentType] || {}; + + // Resolve model id with stricter rules: + // Preference: session override -> agent-config model (only if it looks complete) -> agent.defaultModel + // Only accept agent-config model when it contains a provider/model (contains a '/') + let resolvedModelId: string | undefined; + if ( + typeof (config as any).sessionCustomModel === 'string' && + (config as any).sessionCustomModel.trim() + ) { + resolvedModelId = (config as any).sessionCustomModel.trim(); + } else if ( + agentConfigValues && + typeof agentConfigValues.model === 'string' && + agentConfigValues.model.trim() && + agentConfigValues.model.includes('/') + ) { + resolvedModelId = agentConfigValues.model.trim(); + } else if ( + (agent as any).defaultModel && + typeof (agent as any).defaultModel === 'string' + ) { + resolvedModelId = (agent as any).defaultModel as string; + } + + // Sanitize resolved model id (remove trailing slashes) + if (resolvedModelId) { + resolvedModelId = resolvedModelId.replace(/\/+$/, '').trim(); + if (resolvedModelId === '') resolvedModelId = undefined; + } + + // Debug: log resolved model for tab naming + try { + // eslint-disable-next-line no-console + console.debug('[TabNaming] Resolved model', { + sessionId, + agentType: config.agentType, + agentConfigModel: agentConfigValues.model, + resolvedModelId, + }); + } catch (err) { + // swallow + } + let finalArgs = buildAgentArgs(agent, { baseArgs, prompt: fullPrompt, cwd: config.cwd, readOnlyMode: true, // Always read-only since we're not modifying anything + modelId: resolvedModelId, }); - // Apply config overrides from store - const allConfigs = agentConfigsStore.get('configs', {}); - const agentConfigValues = allConfigs[config.agentType] || {}; + // Apply config overrides from store (other overrides such as customArgs/env) const configResolution = applyAgentConfigOverrides(agent, finalArgs, { agentConfigValues, + sessionCustomModel: resolvedModelId, }); finalArgs = configResolution.args; + // Debug: log how model was resolved for tab naming requests so we can + // verify whether session/agent overrides are applied as expected. + try { + // eslint-disable-next-line no-console + console.debug('[TabNaming] Config resolution', { + sessionId, + agentType: config.agentType, + modelSource: configResolution.modelSource, + agentConfigModel: agentConfigValues?.model, + finalArgsPreview: finalArgs.slice(0, 40), + }); + } catch (err) { + // swallow logging errors + } + + // Sanitize model flags: avoid passing a --model value that is empty + // or looks like a namespace with a trailing slash (e.g. "github-copilot/") + // which some agent CLIs treat as invalid and error out. + try { + const sanitizedArgs: string[] = []; + for (let i = 0; i < finalArgs.length; i++) { + const a = finalArgs[i]; + if (a === '--model') { + const next = finalArgs[i + 1]; + if (!next || typeof next !== 'string' || next.trim() === '' || /\/$/.test(next)) { + // skip both the flag and the invalid value + i++; // advance past the invalid value + // eslint-disable-next-line no-console + console.debug('[TabNaming] Removed invalid --model flag for tab naming', { + sessionId, + removedValue: next, + }); + continue; + } + } + sanitizedArgs.push(a); + } + finalArgs = sanitizedArgs; + } catch (err) { + // ignore sanitization failures + } + // Determine command and working directory let command = agent.path || agent.command; let cwd = config.cwd; - const customEnvVars: Record | undefined = - configResolution.effectiveCustomEnvVars; + // Start with resolved env vars from config resolution, allow mutation below + let customEnvVars: Record | undefined = + configResolution.effectiveCustomEnvVars + ? { ...configResolution.effectiveCustomEnvVars } + : undefined; // Handle SSH remote execution if configured // IMPORTANT: For SSH, we must send the prompt via stdin to avoid shell escaping issues. @@ -196,6 +289,140 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v } } + // Final safety sanitization: ensure args are all plain strings + try { + const nonStringItems = finalArgs.filter((a) => typeof a !== 'string'); + if (nonStringItems.length > 0) { + // eslint-disable-next-line no-console + console.debug('[TabNaming] Removing non-string args before spawn', { + sessionId, + removed: nonStringItems.map((i) => ({ typeof: typeof i, preview: String(i) })), + }); + finalArgs = finalArgs.filter((a) => typeof a === 'string'); + } + + // Extract model arg value for debugging (if present) + const modelIndex = finalArgs.indexOf('--model'); + if (modelIndex !== -1 && finalArgs.length > modelIndex + 1) { + const modelVal = finalArgs[modelIndex + 1]; + // eslint-disable-next-line no-console + console.debug('[TabNaming] Final --model value', { + sessionId, + value: modelVal, + type: typeof modelVal, + }); + } + } catch (err) { + // swallow safety log errors + } + + // Quote model values that contain slashes so they survive shell-based + // spawns (PowerShell can interpret unquoted tokens containing slashes). + try { + // Deduplicate --model flags and ensure exactly one is present before the prompt separator + try { + const sepIndex = + finalArgs.indexOf('--') >= 0 ? finalArgs.indexOf('--') : finalArgs.length; + let lastModelVal: string | undefined; + for (let i = 0; i < sepIndex; i++) { + if (finalArgs[i] === '--model' && finalArgs.length > i + 1) { + const cand = finalArgs[i + 1]; + if (typeof cand === 'string' && cand.trim()) { + lastModelVal = cand; + } + } + } + + if (lastModelVal !== undefined) { + const newArgs: string[] = []; + for (let i = 0; i < sepIndex; i++) { + if (finalArgs[i] === '--model') { + i++; // skip value + continue; + } + newArgs.push(finalArgs[i]); + } + // Insert the single canonical model flag + newArgs.push('--model', lastModelVal); + // Append remaining args (including '--' and prompt) + finalArgs = [...newArgs, ...finalArgs.slice(sepIndex)]; + // eslint-disable-next-line no-console + console.debug('[TabNaming] Deduplicated --model flags', { + sessionId, + canonical: lastModelVal, + }); + } + } catch (err) { + // ignore dedupe failures + } + // Convert separate --model pairs into a single --model= + // token so shells don't split values. Then enforce a single canonical + // CLI model token derived from our resolvedModelId (if available). + const rebuilt: string[] = []; + for (let i = 0; i < finalArgs.length; i++) { + const a = finalArgs[i]; + if (a === '--model' && i + 1 < finalArgs.length) { + const raw = finalArgs[i + 1]; + const val = + typeof raw === 'string' ? raw.replace(/^['\"]|['\"]$/g, '') : String(raw); + rebuilt.push(`--model=${val}`); + i++; // skip the value + } else { + rebuilt.push(a); + } + } + finalArgs = rebuilt; + + // Remove any existing model tokens (either --model=... or -m/value) + const withoutModel: string[] = []; + for (let i = 0; i < finalArgs.length; i++) { + const a = finalArgs[i]; + if (typeof a === 'string' && a.startsWith('--model')) { + // skip + continue; + } + if (a === '-m' && i + 1 < finalArgs.length) { + i++; // skip short form value + continue; + } + withoutModel.push(a); + } + + // If we have a resolvedModelId (from session/agent/default), prefer inserting + // it explicitly as a CLI flag to avoid relying on OpenCode config/env. + if (resolvedModelId && typeof resolvedModelId === 'string') { + // If resolvedModelId doesn't look like provider/model, prefer agent.defaultModel + if ( + !resolvedModelId.includes('/') && + (agent as any).defaultModel && + typeof (agent as any).defaultModel === 'string' && + (agent as any).defaultModel.includes('/') + ) { + resolvedModelId = (agent as any).defaultModel as string; + } + + if (resolvedModelId && resolvedModelId.includes('/')) { + const modelToken = `--model=${resolvedModelId}`; + // Insert before the argument separator `--` if present + const sep = withoutModel.indexOf('--'); + if (sep === -1) { + withoutModel.push(modelToken); + } else { + withoutModel.splice(sep, 0, modelToken); + } + // eslint-disable-next-line no-console + console.debug('[TabNaming] Injected canonical --model for spawn', { + sessionId, + model: resolvedModelId, + }); + } + } + + finalArgs = withoutModel; + } catch (err) { + // swallow + } + // Create a promise that resolves when we get the tab name return new Promise((resolve) => { let output = ''; @@ -231,6 +458,38 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v // Extract the tab name from the output // The agent should return just the tab name, but we clean up any extra whitespace/formatting + // Log raw output and context to help diagnose generic/low-quality tab names + try { + // eslint-disable-next-line no-console + console.debug('[TabNaming] Raw output before extraction', { + sessionId, + agentType: config.agentType, + agentConfigModel: agentConfigValues?.model, + resolvedModelId, + finalArgsPreview: finalArgs.slice(0, 40), + promptPreview: fullPrompt + ? `${String(fullPrompt).slice(0, 200)}${String(fullPrompt).length > 200 ? '...' : ''}` + : undefined, + rawOutputPreview: `${String(output).slice(0, 200)}${String(output).length > 200 ? '...' : ''}`, + rawOutputLength: String(output).length, + }); + // Detect obviously generic outputs to surface in logs + const genericRegex = + /^("|')?\s*(coding task|task tab name|task tab|coding task tab|task name)\b/i; + if (genericRegex.test(String(output))) { + // eslint-disable-next-line no-console + console.warn( + '[TabNaming] Agent returned a generic tab name candidate; consider adjusting prompt or model', + { + sessionId, + detected: String(output).trim().slice(0, 80), + } + ); + } + } catch (err) { + // swallow logging errors + } + const tabName = extractTabName(output); logger.info('Tab naming completed', LOG_CONTEXT, { sessionId, @@ -247,6 +506,21 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v // Spawn the process // When using SSH with stdin, pass the flag so ChildProcessSpawner // sends the prompt via stdin instead of command line args + try { + // Debug: log full finalArgs array and types just before spawn + // (kept in console.debug for diagnosis only) + // eslint-disable-next-line no-console + console.debug('[TabNaming] About to spawn with final args', { + sessionId, + command, + cwd, + sendPromptViaStdin: shouldSendPromptViaStdin, + finalArgsDetail: finalArgs.map((a) => ({ value: a, type: typeof a })), + }); + } catch (err) { + // ignore logging failures + } + processManager.spawn({ sessionId, toolType: config.agentType, @@ -302,13 +576,14 @@ function extractTabName(output: string): string | null { const lines = cleaned.split(/[.\n→]/).filter((line) => { const trimmed = line.trim(); // Filter out empty lines and lines that look like instructions/examples + // Allow quoted single-line outputs (agents often return the name in quotes) + const unquoted = trimmed.replace(/^['"]+|['"]+$/g, ''); return ( - trimmed.length > 0 && - trimmed.length <= 40 && // Tab names should be short - !trimmed.toLowerCase().includes('example') && - !trimmed.toLowerCase().includes('message:') && - !trimmed.toLowerCase().includes('rules:') && - !trimmed.startsWith('"') // Skip example inputs in quotes + unquoted.length > 0 && + unquoted.length <= 40 && // Tab names should be short + !unquoted.toLowerCase().includes('example') && + !unquoted.toLowerCase().includes('message:') && + !unquoted.toLowerCase().includes('rules:') ); }); From c48e74a9921155334efad4fc3f569aba580e20e4 Mon Sep 17 00:00:00 2001 From: chr1syy Date: Sun, 1 Mar 2026 19:19:13 +0100 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20address=20PR=20#487=20review=20?= =?UTF-8?q?=E2=80=94=20SSH=20ordering,=20model=20injection,=20and=20type?= =?UTF-8?q?=20safety?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move model token stripping/re-injection before SSH wrapping (fixes corrupted SSH args) - Inject canonical model unconditionally via cliModelId (fixes silent drop for bare model names) - Add sessionCustomModel to config type and thread it from renderer through preload to handler (fixes dead-code branch) - Use separate cliModelId variable for late fallback, leaving resolvedModelId stable for logging - Fix stream-json adjacency check (indexOf + [idx+1] instead of two includes) - Add safeDebug helper to replace repetitive try/catch blocks around console.debug - Add defaultModel?: string to AgentConfig interface Co-Authored-By: Claude Sonnet 4.6 --- src/main/agents/definitions.ts | 2 + src/main/ipc/handlers/tabNaming.ts | 265 ++++++------------ src/main/preload/tabNaming.ts | 2 + src/renderer/global.d.ts | 1 + .../hooks/input/useInputProcessing.ts | 1 + 5 files changed, 96 insertions(+), 175 deletions(-) diff --git a/src/main/agents/definitions.ts b/src/main/agents/definitions.ts index de256fc91..99a7249bd 100644 --- a/src/main/agents/definitions.ts +++ b/src/main/agents/definitions.ts @@ -97,6 +97,8 @@ export interface AgentConfig { noPromptSeparator?: boolean; // If true, don't add '--' before the prompt in batch mode (OpenCode doesn't support it) defaultEnvVars?: Record; // Default environment variables for this agent (merged with user customEnvVars) readOnlyEnvOverrides?: Record; // Env var overrides applied in read-only mode (replaces keys from defaultEnvVars) + // Optional default model id discovered from the agent's local config or binary + defaultModel?: string; } /** diff --git a/src/main/ipc/handlers/tabNaming.ts b/src/main/ipc/handlers/tabNaming.ts index c2b0c952c..d29f0d23d 100644 --- a/src/main/ipc/handlers/tabNaming.ts +++ b/src/main/ipc/handlers/tabNaming.ts @@ -28,6 +28,15 @@ import type { MaestroSettings } from './persistence'; const LOG_CONTEXT = '[TabNaming]'; +// Safe debug wrapper to centralize console.debug error isolation +const safeDebug = (message: string, data?: any) => { + try { + console.debug(message, data); + } catch { + // swallow + } +}; + /** * Helper to create handler options with consistent context */ @@ -84,6 +93,7 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v userMessage: string; agentType: string; cwd: string; + sessionCustomModel?: string; sessionSshRemoteConfig?: { enabled: boolean; remoteId: string | null; @@ -132,11 +142,8 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v // Preference: session override -> agent-config model (only if it looks complete) -> agent.defaultModel // Only accept agent-config model when it contains a provider/model (contains a '/') let resolvedModelId: string | undefined; - if ( - typeof (config as any).sessionCustomModel === 'string' && - (config as any).sessionCustomModel.trim() - ) { - resolvedModelId = (config as any).sessionCustomModel.trim(); + if (typeof config.sessionCustomModel === 'string' && config.sessionCustomModel.trim()) { + resolvedModelId = config.sessionCustomModel.trim(); } else if ( agentConfigValues && typeof agentConfigValues.model === 'string' && @@ -158,17 +165,12 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v } // Debug: log resolved model for tab naming - try { - // eslint-disable-next-line no-console - console.debug('[TabNaming] Resolved model', { - sessionId, - agentType: config.agentType, - agentConfigModel: agentConfigValues.model, - resolvedModelId, - }); - } catch (err) { - // swallow - } + safeDebug('[TabNaming] Resolved model', { + sessionId, + agentType: config.agentType, + agentConfigModel: agentConfigValues.model, + resolvedModelId, + }); let finalArgs = buildAgentArgs(agent, { baseArgs, @@ -187,51 +189,82 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v // Debug: log how model was resolved for tab naming requests so we can // verify whether session/agent overrides are applied as expected. - try { - // eslint-disable-next-line no-console - console.debug('[TabNaming] Config resolution', { - sessionId, - agentType: config.agentType, - modelSource: configResolution.modelSource, - agentConfigModel: agentConfigValues?.model, - finalArgsPreview: finalArgs.slice(0, 40), - }); - } catch (err) { - // swallow logging errors + safeDebug('[TabNaming] Config resolution', { + sessionId, + agentType: config.agentType, + modelSource: configResolution.modelSource, + agentConfigModel: agentConfigValues?.model, + finalArgsPreview: finalArgs.slice(0, 40), + }); + + // Determine the canonical CLI model to inject. + // resolvedModelId stays stable for logging; cliModelId is what gets injected. + // If resolvedModelId has no provider prefix, fall back to agent.defaultModel. + // If neither has a provider prefix, still inject the value so the model is + // not silently dropped when existing model tokens are stripped. + let cliModelId: string | undefined = resolvedModelId; + if ( + cliModelId && + !cliModelId.includes('/') && + (agent as any).defaultModel && + typeof (agent as any).defaultModel === 'string' && + (agent as any).defaultModel.includes('/') + ) { + cliModelId = (agent as any).defaultModel as string; } - // Sanitize model flags: avoid passing a --model value that is empty - // or looks like a namespace with a trailing slash (e.g. "github-copilot/") - // which some agent CLIs treat as invalid and error out. + // Canonicalize model flags: strip all existing --model/-m tokens before the + // prompt separator, then re-inject the single canonical --model=. + // This must run BEFORE SSH wrapping so the flag ends up inside the remote + // agent invocation, not in the SSH wrapper arguments. try { - const sanitizedArgs: string[] = []; - for (let i = 0; i < finalArgs.length; i++) { - const a = finalArgs[i]; - if (a === '--model') { - const next = finalArgs[i + 1]; - if (!next || typeof next !== 'string' || next.trim() === '' || /\/$/.test(next)) { - // skip both the flag and the invalid value - i++; // advance past the invalid value - // eslint-disable-next-line no-console - console.debug('[TabNaming] Removed invalid --model flag for tab naming', { - sessionId, - removedValue: next, - }); + const sepIndex = + finalArgs.indexOf('--') >= 0 ? finalArgs.indexOf('--') : finalArgs.length; + const prefix = finalArgs.slice(0, sepIndex); + const suffix = finalArgs.slice(sepIndex); + + const filteredPrefix: string[] = []; + for (let i = 0; i < prefix.length; i++) { + const a = prefix[i]; + if (typeof a === 'string') { + if (a.startsWith('--model=')) { + continue; // drop explicit --model=value + } + if (a === '--model') { + i++; // drop flag + value continue; } + if (a === '-m' && i + 1 < prefix.length) { + i++; // drop short form + value + continue; + } + } + filteredPrefix.push(a); + } + + // Re-inject the canonical model if we have one. + if (cliModelId && typeof cliModelId === 'string') { + // Validate: skip empty or trailing-slash-only values + const sanitized = cliModelId.replace(/\/+$/, '').trim(); + if (sanitized) { + filteredPrefix.push('--model=' + sanitized); + safeDebug('[TabNaming] Injected canonical --model for spawn', { + sessionId, + model: sanitized, + }); } - sanitizedArgs.push(a); } - finalArgs = sanitizedArgs; + + finalArgs = [...filteredPrefix, ...suffix]; } catch (err) { - // ignore sanitization failures + // swallow } // Determine command and working directory let command = agent.path || agent.command; let cwd = config.cwd; // Start with resolved env vars from config resolution, allow mutation below - let customEnvVars: Record | undefined = + const customEnvVars: Record | undefined = configResolution.effectiveCustomEnvVars ? { ...configResolution.effectiveCustomEnvVars } : undefined; @@ -258,8 +291,9 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v const agentSupportsStreamJson = agent.capabilities?.supportsStreamJsonInput ?? false; if (agentSupportsStreamJson) { // Add --input-format stream-json to args so agent reads from stdin + const inputFormatIdx = finalArgs.indexOf('--input-format'); const hasStreamJsonInput = - finalArgs.includes('--input-format') && finalArgs.includes('stream-json'); + inputFormatIdx !== -1 && finalArgs[inputFormatIdx + 1] === 'stream-json'; if (!hasStreamJsonInput) { finalArgs = [...finalArgs, '--input-format', 'stream-json']; } @@ -293,136 +327,16 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v try { const nonStringItems = finalArgs.filter((a) => typeof a !== 'string'); if (nonStringItems.length > 0) { - // eslint-disable-next-line no-console - console.debug('[TabNaming] Removing non-string args before spawn', { + safeDebug('[TabNaming] Removing non-string args before spawn', { sessionId, removed: nonStringItems.map((i) => ({ typeof: typeof i, preview: String(i) })), }); finalArgs = finalArgs.filter((a) => typeof a === 'string'); } - - // Extract model arg value for debugging (if present) - const modelIndex = finalArgs.indexOf('--model'); - if (modelIndex !== -1 && finalArgs.length > modelIndex + 1) { - const modelVal = finalArgs[modelIndex + 1]; - // eslint-disable-next-line no-console - console.debug('[TabNaming] Final --model value', { - sessionId, - value: modelVal, - type: typeof modelVal, - }); - } } catch (err) { // swallow safety log errors } - // Quote model values that contain slashes so they survive shell-based - // spawns (PowerShell can interpret unquoted tokens containing slashes). - try { - // Deduplicate --model flags and ensure exactly one is present before the prompt separator - try { - const sepIndex = - finalArgs.indexOf('--') >= 0 ? finalArgs.indexOf('--') : finalArgs.length; - let lastModelVal: string | undefined; - for (let i = 0; i < sepIndex; i++) { - if (finalArgs[i] === '--model' && finalArgs.length > i + 1) { - const cand = finalArgs[i + 1]; - if (typeof cand === 'string' && cand.trim()) { - lastModelVal = cand; - } - } - } - - if (lastModelVal !== undefined) { - const newArgs: string[] = []; - for (let i = 0; i < sepIndex; i++) { - if (finalArgs[i] === '--model') { - i++; // skip value - continue; - } - newArgs.push(finalArgs[i]); - } - // Insert the single canonical model flag - newArgs.push('--model', lastModelVal); - // Append remaining args (including '--' and prompt) - finalArgs = [...newArgs, ...finalArgs.slice(sepIndex)]; - // eslint-disable-next-line no-console - console.debug('[TabNaming] Deduplicated --model flags', { - sessionId, - canonical: lastModelVal, - }); - } - } catch (err) { - // ignore dedupe failures - } - // Convert separate --model pairs into a single --model= - // token so shells don't split values. Then enforce a single canonical - // CLI model token derived from our resolvedModelId (if available). - const rebuilt: string[] = []; - for (let i = 0; i < finalArgs.length; i++) { - const a = finalArgs[i]; - if (a === '--model' && i + 1 < finalArgs.length) { - const raw = finalArgs[i + 1]; - const val = - typeof raw === 'string' ? raw.replace(/^['\"]|['\"]$/g, '') : String(raw); - rebuilt.push(`--model=${val}`); - i++; // skip the value - } else { - rebuilt.push(a); - } - } - finalArgs = rebuilt; - - // Remove any existing model tokens (either --model=... or -m/value) - const withoutModel: string[] = []; - for (let i = 0; i < finalArgs.length; i++) { - const a = finalArgs[i]; - if (typeof a === 'string' && a.startsWith('--model')) { - // skip - continue; - } - if (a === '-m' && i + 1 < finalArgs.length) { - i++; // skip short form value - continue; - } - withoutModel.push(a); - } - - // If we have a resolvedModelId (from session/agent/default), prefer inserting - // it explicitly as a CLI flag to avoid relying on OpenCode config/env. - if (resolvedModelId && typeof resolvedModelId === 'string') { - // If resolvedModelId doesn't look like provider/model, prefer agent.defaultModel - if ( - !resolvedModelId.includes('/') && - (agent as any).defaultModel && - typeof (agent as any).defaultModel === 'string' && - (agent as any).defaultModel.includes('/') - ) { - resolvedModelId = (agent as any).defaultModel as string; - } - - if (resolvedModelId && resolvedModelId.includes('/')) { - const modelToken = `--model=${resolvedModelId}`; - // Insert before the argument separator `--` if present - const sep = withoutModel.indexOf('--'); - if (sep === -1) { - withoutModel.push(modelToken); - } else { - withoutModel.splice(sep, 0, modelToken); - } - // eslint-disable-next-line no-console - console.debug('[TabNaming] Injected canonical --model for spawn', { - sessionId, - model: resolvedModelId, - }); - } - } - - finalArgs = withoutModel; - } catch (err) { - // swallow - } - // Create a promise that resolves when we get the tab name return new Promise((resolve) => { let output = ''; @@ -460,8 +374,7 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v // The agent should return just the tab name, but we clean up any extra whitespace/formatting // Log raw output and context to help diagnose generic/low-quality tab names try { - // eslint-disable-next-line no-console - console.debug('[TabNaming] Raw output before extraction', { + safeDebug('[TabNaming] Raw output before extraction', { sessionId, agentType: config.agentType, agentConfigModel: agentConfigValues?.model, @@ -477,7 +390,6 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v const genericRegex = /^("|')?\s*(coding task|task tab name|task tab|coding task tab|task name)\b/i; if (genericRegex.test(String(output))) { - // eslint-disable-next-line no-console console.warn( '[TabNaming] Agent returned a generic tab name candidate; consider adjusting prompt or model', { @@ -509,8 +421,7 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v try { // Debug: log full finalArgs array and types just before spawn // (kept in console.debug for diagnosis only) - // eslint-disable-next-line no-console - console.debug('[TabNaming] About to spawn with final args', { + safeDebug('[TabNaming] About to spawn with final args', { sessionId, command, cwd, @@ -576,7 +487,11 @@ function extractTabName(output: string): string | null { const lines = cleaned.split(/[.\n→]/).filter((line) => { const trimmed = line.trim(); // Filter out empty lines and lines that look like instructions/examples - // Allow quoted single-line outputs (agents often return the name in quotes) + // Treat lines that start with a quote as example inputs and filter them out. + // (Agents sometimes include example quoted names in the prompt.) + if (trimmed.startsWith('"') || trimmed.startsWith("'")) return false; + // Allow quoted single-line outputs to be cleaned later, but lines that begin + // with quotes are typically examples and should be ignored. const unquoted = trimmed.replace(/^['"]+|['"]+$/g, ''); return ( unquoted.length > 0 && diff --git a/src/main/preload/tabNaming.ts b/src/main/preload/tabNaming.ts index 8a6dd13dd..57a639a32 100644 --- a/src/main/preload/tabNaming.ts +++ b/src/main/preload/tabNaming.ts @@ -17,6 +17,8 @@ export interface TabNamingConfig { agentType: string; /** Working directory for the session */ cwd: string; + /** Optional session-level model override */ + sessionCustomModel?: string; /** Optional SSH remote configuration */ sessionSshRemoteConfig?: { enabled: boolean; diff --git a/src/renderer/global.d.ts b/src/renderer/global.d.ts index c76aa5e15..6bf165709 100644 --- a/src/renderer/global.d.ts +++ b/src/renderer/global.d.ts @@ -2598,6 +2598,7 @@ interface MaestroAPI { userMessage: string; agentType: string; cwd: string; + sessionCustomModel?: string; sessionSshRemoteConfig?: { enabled: boolean; remoteId: string | null; diff --git a/src/renderer/hooks/input/useInputProcessing.ts b/src/renderer/hooks/input/useInputProcessing.ts index 095db9375..ca9278383 100644 --- a/src/renderer/hooks/input/useInputProcessing.ts +++ b/src/renderer/hooks/input/useInputProcessing.ts @@ -709,6 +709,7 @@ export function useInputProcessing(deps: UseInputProcessingDeps): UseInputProces userMessage: effectiveInputValue, agentType: activeSession.toolType, cwd: activeSession.cwd, + sessionCustomModel: activeSession.customModel, sessionSshRemoteConfig: activeSession.sessionSshRemoteConfig, }) .then((generatedName) => { From b71fedb8009e8994b5f579bbe4a287e1d6b3be34 Mon Sep 17 00:00:00 2001 From: chr1syy Date: Sun, 1 Mar 2026 21:28:45 +0100 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20address=20PR=20#487=20review=20round?= =?UTF-8?q?=202=20=E2=80=94=20model=20flag=20style,=20precedence,=20and=20?= =?UTF-8?q?quote=20filter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use agent.modelArgs() for model re-injection instead of hardcoded --model= (Codex uses -m, other agents may have different styles) - Remove cliModelId fallback that incorrectly overrode session model with agent.defaultModel when resolvedModelId lacked a provider prefix — resolvedModelId already encodes the correct session > agent-config > agent-default precedence - Remove (agent as any).defaultModel casts — defaultModel is typed on AgentConfig - Remove try/catch swallowing model canonicalization errors (bubbles per CLAUDE.md) - Fix extractTabName quote filter: fully-wrapped quoted strings like "Fix CI tests" are now kept for the unquoting step instead of being discarded early Co-Authored-By: Claude Sonnet 4.6 --- src/main/ipc/handlers/tabNaming.ts | 113 ++++++++++++----------------- 1 file changed, 48 insertions(+), 65 deletions(-) diff --git a/src/main/ipc/handlers/tabNaming.ts b/src/main/ipc/handlers/tabNaming.ts index d29f0d23d..54cc999a2 100644 --- a/src/main/ipc/handlers/tabNaming.ts +++ b/src/main/ipc/handlers/tabNaming.ts @@ -151,11 +151,8 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v agentConfigValues.model.includes('/') ) { resolvedModelId = agentConfigValues.model.trim(); - } else if ( - (agent as any).defaultModel && - typeof (agent as any).defaultModel === 'string' - ) { - resolvedModelId = (agent as any).defaultModel as string; + } else if (agent.defaultModel && typeof agent.defaultModel === 'string') { + resolvedModelId = agent.defaultModel; } // Sanitize resolved model id (remove trailing slashes) @@ -197,69 +194,55 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v finalArgsPreview: finalArgs.slice(0, 40), }); - // Determine the canonical CLI model to inject. - // resolvedModelId stays stable for logging; cliModelId is what gets injected. - // If resolvedModelId has no provider prefix, fall back to agent.defaultModel. - // If neither has a provider prefix, still inject the value so the model is - // not silently dropped when existing model tokens are stripped. - let cliModelId: string | undefined = resolvedModelId; - if ( - cliModelId && - !cliModelId.includes('/') && - (agent as any).defaultModel && - typeof (agent as any).defaultModel === 'string' && - (agent as any).defaultModel.includes('/') - ) { - cliModelId = (agent as any).defaultModel as string; - } - // Canonicalize model flags: strip all existing --model/-m tokens before the - // prompt separator, then re-inject the single canonical --model=. + // prompt separator, then re-inject the single canonical model flag using the + // agent-specific flag style (e.g. Codex uses -m, Claude Code uses --model=). // This must run BEFORE SSH wrapping so the flag ends up inside the remote // agent invocation, not in the SSH wrapper arguments. - try { - const sepIndex = - finalArgs.indexOf('--') >= 0 ? finalArgs.indexOf('--') : finalArgs.length; - const prefix = finalArgs.slice(0, sepIndex); - const suffix = finalArgs.slice(sepIndex); - - const filteredPrefix: string[] = []; - for (let i = 0; i < prefix.length; i++) { - const a = prefix[i]; - if (typeof a === 'string') { - if (a.startsWith('--model=')) { - continue; // drop explicit --model=value - } - if (a === '--model') { - i++; // drop flag + value - continue; - } - if (a === '-m' && i + 1 < prefix.length) { - i++; // drop short form + value - continue; - } + const sepIndex = + finalArgs.indexOf('--') >= 0 ? finalArgs.indexOf('--') : finalArgs.length; + const prefix = finalArgs.slice(0, sepIndex); + const suffix = finalArgs.slice(sepIndex); + + const filteredPrefix: string[] = []; + for (let i = 0; i < prefix.length; i++) { + const a = prefix[i]; + if (typeof a === 'string') { + if (a.startsWith('--model=')) { + continue; // drop explicit --model=value } - filteredPrefix.push(a); - } - - // Re-inject the canonical model if we have one. - if (cliModelId && typeof cliModelId === 'string') { - // Validate: skip empty or trailing-slash-only values - const sanitized = cliModelId.replace(/\/+$/, '').trim(); - if (sanitized) { - filteredPrefix.push('--model=' + sanitized); - safeDebug('[TabNaming] Injected canonical --model for spawn', { - sessionId, - model: sanitized, - }); + if (a === '--model') { + i++; // drop flag + value + continue; + } + if (a === '-m' && i + 1 < prefix.length) { + i++; // drop short form + value + continue; } } + filteredPrefix.push(a); + } - finalArgs = [...filteredPrefix, ...suffix]; - } catch (err) { - // swallow + // Re-inject using resolvedModelId directly — it already reflects session > + // agent-config > agent-default precedence. Use agent.modelArgs() when available + // so each agent gets its own flag style. + if (resolvedModelId) { + const sanitized = resolvedModelId.replace(/\/+$/, '').trim(); + if (sanitized) { + const modelArgTokens = agent.modelArgs + ? agent.modelArgs(sanitized) + : [`--model=${sanitized}`]; + filteredPrefix.push(...modelArgTokens); + safeDebug('[TabNaming] Injected canonical model flag for spawn', { + sessionId, + model: sanitized, + tokens: modelArgTokens, + }); + } } + finalArgs = [...filteredPrefix, ...suffix]; + // Determine command and working directory let command = agent.path || agent.command; let cwd = config.cwd; @@ -486,12 +469,12 @@ function extractTabName(output: string): string | null { // Split by newlines, periods, or arrow symbols and take meaningful lines const lines = cleaned.split(/[.\n→]/).filter((line) => { const trimmed = line.trim(); - // Filter out empty lines and lines that look like instructions/examples - // Treat lines that start with a quote as example inputs and filter them out. - // (Agents sometimes include example quoted names in the prompt.) - if (trimmed.startsWith('"') || trimmed.startsWith("'")) return false; - // Allow quoted single-line outputs to be cleaned later, but lines that begin - // with quotes are typically examples and should be ignored. + // Filter out empty lines and lines that look like instructions/examples. + // Lines that are fully wrapped in quotes (e.g. "Fix CI flaky tests") are valid + // tab name candidates — keep them so the unquoting step below can clean them. + // Only discard lines that START with a quote but are not fully wrapped (example inputs). + const isWrappedQuoted = /^["'].+["']$/.test(trimmed); + if ((trimmed.startsWith('"') || trimmed.startsWith("'")) && !isWrappedQuoted) return false; const unquoted = trimmed.replace(/^['"]+|['"]+$/g, ''); return ( unquoted.length > 0 &&