-
Notifications
You must be signed in to change notification settings - Fork 223
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -122,26 +132,142 @@ 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 as any).defaultModel && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typeof (agent as any).defaultModel === 'string' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resolvedModelId = (agent as any).defaultModel as string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
144
to
159
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The IPC handler's config: {
userMessage: string;
agentType: string;
cwd: string;
sessionSshRemoteConfig?: { ... };
}
Suggested change
Consider adding |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+206
to
+214
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fallback logic violates stated model precedence order by overriding session models. If
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Canonicalize model flags: strip all existing --model/-m tokens before the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // prompt separator, then re-inject the single canonical --model=<value>. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+245
to
+251
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Model arg builders by agent ==="
rg -n "modelArgs:\s*\(modelId: string\)\s*=>\s*\[" src/main/agents/definitions.ts -C 1
echo
echo "=== Canonical injection in tab naming handler ==="
rg -n "Injected canonical --model|--model=" src/main/ipc/handlers/tabNaming.ts -C 2Repository: RunMaestro/Maestro Length of output: 1557 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Context around lines 245-251 in tabNaming.ts ==="
sed -n '240,260p' src/main/ipc/handlers/tabNaming.ts | cat -A
echo
echo "=== Check if agent object is accessible at line 245-251 ==="
sed -n '200,260p' src/main/ipc/handlers/tabNaming.ts | head -70
echo
echo "=== Check indentation of line 245-251 (checking for tabs) ==="
sed -n '245,251p' src/main/ipc/handlers/tabNaming.ts | od -c | head -30Repository: RunMaestro/Maestro Length of output: 3366 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Check if agent has modelArgs property and its type ==="
rg -n "interface.*Agent|type.*Agent" src/main/agents/ -A 20 | head -60
echo
echo "=== Look for the agent variable declaration in tabNaming.ts context ==="
sed -n '150,210p' src/main/ipc/handlers/tabNaming.ts
echo
echo "=== Check if agent is typed and has modelArgs ==="
rg -n "agent:" src/main/ipc/handlers/tabNaming.ts -B 5 -A 2 | head -30Repository: RunMaestro/Maestro Length of output: 6357 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Function signature around tabNaming handler ==="
sed -n '1,100p' src/main/ipc/handlers/tabNaming.ts | grep -A 30 "export\|function\|const.*="
echo
echo "=== Search for buildAgentArgs and applyAgentConfigOverrides imports ==="
head -50 src/main/ipc/handlers/tabNaming.ts | grep -E "import|from"
echo
echo "=== Full agent definitions for model arg styles ==="
sed -n '155,225p' src/main/agents/definitions.tsRepository: RunMaestro/Maestro Length of output: 6129 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Check agent definition interface/type to see if modelArgs is defined ==="
rg -n "interface.*Agent|type.*Agent|^export.*Agent" src/main/agents/definitions.ts -A 30 | head -80
echo
echo "=== Search for Agent type definition ==="
rg -n "^interface Agent|^type Agent" src/ -A 25Repository: RunMaestro/Maestro Length of output: 45362 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Verify agent variable is available at line 245-251 scope ==="
sed -n '100,260p' src/main/ipc/handlers/tabNaming.ts | grep -n "const.*agent\|let.*agent" | head -20
echo
echo "=== Check if agent is obtained from AgentDetector ==="
grep -n "getAgentDetector\|agent = " src/main/ipc/handlers/tabNaming.ts | head -20
echo
echo "=== Verify AgentConfig type includes optional modelArgs ==="
sed -n '71,107p' src/main/agents/definitions.ts | grep -A 2 "modelArgs"Repository: RunMaestro/Maestro Length of output: 1458 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Final confirmation: buildAgentArgs does NOT use modelArgs ==="
grep -A 30 "export function buildAgentArgs" src/main/utils/agent-args.ts | head -40
echo
echo "=== Show that applyAgentConfigOverrides also does NOT use modelArgs directly ==="
grep -A 20 "export function applyAgentConfigOverrides" src/main/utils/agent-args.ts | head -30Repository: RunMaestro/Maestro Length of output: 1739 Canonical model injection bypasses agent-specific model flag styles and breaks compatibility. The code at lines 245-251 hardcodes 🔧 Proposed fix- if (cliModelId && typeof cliModelId === 'string') {
+ if (cliModelId && typeof cliModelId === 'string') {
// Validate: skip empty or trailing-slash-only values
const sanitized = cliModelId.replace(/\/+$/, '').trim();
if (sanitized) {
- filteredPrefix.push('--model=' + sanitized);
+ const modelArgs = agent.modelArgs
+ ? agent.modelArgs(sanitized)
+ : [`--model=${sanitized}`];
+ filteredPrefix.push(...modelArgs);
safeDebug('[TabNaming] Injected canonical --model for spawn', {
sessionId,
model: sanitized,
});
}
}🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sessionId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| model: sanitized, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| finalArgs = [...filteredPrefix, ...suffix]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // swallow | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+259
to
+261
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Empty catch violates CLAUDE.md error handling guidelines CLAUDE.md states: "DO let exceptions bubble up" and "silently swallowing errors hides bugs from Sentry". If model canonicalization fails, |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -165,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']; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -196,6 +323,20 @@ 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) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // swallow safety log errors | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Create a promise that resolves when we get the tab name | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new Promise<string | null>((resolve) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let output = ''; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -231,6 +372,36 @@ 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))) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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 +418,20 @@ 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) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| safeDebug('[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 +487,18 @@ 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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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, ''); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+492
to
+495
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logic contradiction - quoted lines filtered before Line 492 returns |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+490
to
496
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Quoted single-line tab names are filtered out before unquoting. The early quote check drops valid outputs like 🔧 Proposed fix- // 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.
+ // Keep wrapped quoted names (e.g. "Refactor auth flow"), but still
+ // filter quote-prefixed example lines.
+ const isWrappedQuoted = /^["'].+["']$/.test(trimmed);
+ if ((trimmed.startsWith('"') || trimmed.startsWith("'")) && !isWrappedQuoted) return false;
const unquoted = trimmed.replace(/^['"]+|['"]+$/g, '');🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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:') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary type casting -
defaultModelis already part ofAgentConfigdefaultModel?: stringwas added to theAgentConfiginterface in this PR (line 101 of definitions.ts). Theas anycast bypasses type safety.