Skip to content

tab: enforce deterministic model arg and improve tab-name extraction (opencode) #487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
chr1syy wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

tab: enforce deterministic model arg and improve tab-name extraction (opencode) #487

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f958e5b
tab: enforce deterministic model arg and improve tab-name extraction
chr1syy Mar 1, 2026
c48e74a
fix: address PR #487 review — SSH ordering, model injection, and type…
chr1syy Mar 1, 2026
b71fedb
fix: address PR #487 review round 2 — model flag style, precedence, a…
chr1syy Mar 1, 2026
f756526
fix: move arg filtering outside try-catch, use logger.warn, remove re…
chr1syy Mar 2, 2026
33b2586
fix: use stdin passthrough for SSH tab naming to fix exit code 2
chr1syy Mar 2, 2026
96291f0
fix: redact sensitive data from safeDebug logs and fix misleading mod…
chr1syy Mar 2, 2026
1a3f3cb
fix: remove swallowed try-catch in non-string filter and redact remai…
chr1syy Mar 2, 2026
ff9fda8
fix: guard model flag value consumption against adjacent flags, remov…
chr1syy Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/main/agents/definitions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>; // Default environment variables for this agent (merged with user customEnvVars)
readOnlyEnvOverrides?: Record<string, string>; // 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;
}

/**
Expand Down
194 changes: 182 additions & 12 deletions src/main/ipc/handlers/tabNaming.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -84,6 +93,7 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v
userMessage: string;
agentType: string;
cwd: string;
sessionCustomModel?: string;
sessionSshRemoteConfig?: {
enabled: boolean;
remoteId: string | null;
Expand Down Expand Up @@ -122,26 +132,125 @@ 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.sessionCustomModel === 'string' && config.sessionCustomModel.trim()) {
resolvedModelId = config.sessionCustomModel.trim();
} else if (
agentConfigValues &&
typeof agentConfigValues.model === 'string' &&
agentConfigValues.model.trim() &&
agentConfigValues.model.includes('/')
) {
resolvedModelId = agentConfigValues.model.trim();
} else if (agent.defaultModel && typeof agent.defaultModel === 'string') {
resolvedModelId = agent.defaultModel;
}
Comment on lines +144 to +156
Copy link

@greptile-apps greptile-apps bot Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sessionCustomModel branch is dead code — field is not in the handler config type

The IPC handler's config parameter (lines 83–92) is typed as:

config: {
  userMessage: string;
  agentType: string;
  cwd: string;
  sessionSshRemoteConfig?: { ... };
}

sessionCustomModel is not part of this type. Accessing it via (config as any).sessionCustomModel will always evaluate to undefined, so the first branch of the model-resolution chain (lines 135–139) is never entered. The PR description lists "session override" as the highest-priority model source, but it will never be applied until sessionCustomModel is added to both the config type and the IPC caller.

Suggested change
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;
}
let resolvedModelId: string | undefined;
if (
typeof (config as any).sessionCustomModel === 'string' &&
(config as any).sessionCustomModel.trim()
) {
resolvedModelId = (config as any).sessionCustomModel.trim();
} else if (

Consider adding sessionCustomModel?: string to the config interface and threading the value through from the caller, or removing this branch if it is not yet implemented.


// Sanitize resolved model id (remove trailing slashes)
if (resolvedModelId) {
resolvedModelId = resolvedModelId.replace(/\/+$/, '').trim();
if (resolvedModelId === '') resolvedModelId = undefined;
}

// Debug: log resolved model for tab naming
safeDebug('[TabNaming] Resolved model', {
sessionId,
agentType: config.agentType,
agentConfigModel: agentConfigValues.model,
resolvedModelId,
});

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.
safeDebug('[TabNaming] Config resolution', {
sessionId,
agentType: config.agentType,
modelSource: configResolution.modelSource,
agentConfigModel: agentConfigValues?.model,
finalArgsPreview: finalArgs.slice(0, 40),
});

// Canonicalize model flags: strip all existing --model/-m tokens before the
// 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.
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);
}
Comment on lines +209 to +231
Copy link

@coderabbitai coderabbitai bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Model flag stripping may incorrectly consume a following argument if --model appears without a value.

If finalArgs contains a malformed sequence like ['--model', '--verbose', ...], the i++ at line 216 will skip --verbose, treating it as the model value.

Consider adding a guard to ensure the next element looks like a model value (not another flag):

🛡️ Proposed defensive check 
 						if (a === '--model') {
+							// Only consume next arg if it doesn't look like another flag
+							if (i + 1 < prefix.length && !String(prefix[i + 1]).startsWith('-')) {
+								i++; // drop flag + value
+							}
-							i++; // drop flag + value
 							continue;
 						}
 						if (a === '-m' && i + 1 < prefix.length) {
+							// Only consume next arg if it doesn't look like another flag
+							if (!String(prefix[i + 1]).startsWith('-')) {
+								i++; // drop short form + value
+							}
-							i++; // drop short form + value
 							continue;
 						}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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);
}
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') {
// Only consume next arg if it doesn't look like another flag
if (i + 1 < prefix.length && !String(prefix[i + 1]).startsWith('-')) {
i++; // drop flag + value
}
continue;
}
if (a === '-m' && i + 1 < prefix.length) {
// Only consume next arg if it doesn't look like another flag
if (!String(prefix[i + 1]).startsWith('-')) {
i++; // drop short form + value
}
continue;
}
}
filteredPrefix.push(a);
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/main/ipc/handlers/tabNaming.ts` around lines 209 - 225, The loop that
strips model flags in tabNaming.ts can wrongly consume the next token when
encountering '--model' or '-m'; update the logic inside the for loop that
inspects prefix[i] (and handles '--model' and '-m') to only advance i and drop
the next element if that next element exists, is a string, and does not start
with '-' (i.e., looks like a value); otherwise treat the flag as not having a
value and only drop the flag itself (push no increment), ensuring filteredPrefix
and prefix handling remains unchanged.


// 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;
// Start with resolved env vars from config resolution, allow mutation below
const customEnvVars: Record<string, string> | undefined =
configResolution.effectiveCustomEnvVars;
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.
Expand All @@ -165,8 +274,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'];
}
Expand Down Expand Up @@ -196,6 +306,20 @@ export function registerTabNamingHandlers(deps: TabNamingHandlerDependencies): v
}
}

// Final safety sanitization: ensure args are all plain strings
const nonStringItems = finalArgs.filter((a) => typeof a !== 'string');
if (nonStringItems.length > 0) {
finalArgs = finalArgs.filter((a) => typeof a === 'string');
try {
safeDebug('[TabNaming] Removing non-string args before spawn', {
sessionId,
removed: nonStringItems.map((i) => ({ typeof: typeof i, preview: String(i) })),
});
} catch {
// swallow logging errors
}
}
Comment on lines +292 to +300
Copy link

@greptile-apps greptile-apps bot Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Model injection runs after SSH command wrapping

The entire new sanitization and model-injection block (lines 292–424) executes after the SSH command is constructed (lines 244–290). When SSH is enabled, finalArgs at this point already contains the SSH-specific argument list (i.e., the remote agent command has been embedded as a single shell string or as SSH flags). The model-injection logic then scans those SSH args for -- separators and injects --model=<value> into the wrong position, corrupting the SSH command structure.

To fix this, the model injection must happen before buildSshCommand is called, so the model flag ends up inside the remote agent invocation, not in the SSH wrapper:

// ✅ Inject model flag here, before SSH wrapping
//    ... (sanitize finalArgs)

if (config.sessionSshRemoteConfig?.enabled && ...) {
    const sshCommand = await buildSshCommand(...)
    command = sshCommand.command;
    finalArgs = sshCommand.args;
}

// ❌ Do NOT modify finalArgs here for SSH sessions


// Create a promise that resolves when we get the tab name
return new Promise<string | null>((resolve) => {
let output = '';
Expand Down Expand Up @@ -231,6 +355,37 @@ 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 {
safeDebug('[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))) {
logger.warn(
'[TabNaming] Agent returned a generic tab name candidate; consider adjusting prompt or model',
LOG_CONTEXT,
{
sessionId,
detected: String(output).trim().slice(0, 80),
}
);
}
} catch {
// swallow logging errors
}

const tabName = extractTabName(output);
logger.info('Tab naming completed', LOG_CONTEXT, {
sessionId,
Expand All @@ -247,6 +402,16 @@ 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
// Debug: log full finalArgs array and types just before spawn
// (kept in console.debug for diagnosis only)
safeDebug('[TabNaming] About to spawn with final args', {
sessionId,
command,
cwd,
sendPromptViaStdin: shouldSendPromptViaStdin,
finalArgsDetail: finalArgs.map((a) => ({ value: a, type: typeof a })),
});

processManager.spawn({
sessionId,
toolType: config.agentType,
Expand Down Expand Up @@ -301,14 +466,19 @@ 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
// 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 (
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:')
);
});

Expand Down
2 changes: 2 additions & 0 deletions src/main/preload/tabNaming.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/renderer/global.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2598,6 +2598,7 @@ interface MaestroAPI {
userMessage: string;
agentType: string;
cwd: string;
sessionCustomModel?: string;
sessionSshRemoteConfig?: {
enabled: boolean;
remoteId: string | null;
Expand Down
1 change: 1 addition & 0 deletions src/renderer/hooks/input/useInputProcessing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
Loading