diff --git a/src/browser/contexts/ThemeContext.tsx b/src/browser/contexts/ThemeContext.tsx index 5e83061b65..fc20eaa609 100644 --- a/src/browser/contexts/ThemeContext.tsx +++ b/src/browser/contexts/ThemeContext.tsx @@ -10,6 +10,7 @@ import React, { } from "react"; import { readPersistedString, usePersistedState } from "@/browser/hooks/usePersistedState"; import { UI_THEME_KEY } from "@/common/constants/storage"; +import { isLightThemeMode } from "@/browser/utils/highlighting/shiki-shared"; export type ThemeMode = "light" | "dark" | "flexoki-light" | "flexoki-dark"; export type ThemePreference = ThemeMode | "auto"; @@ -74,7 +75,8 @@ const FAVICON_BY_SCHEME: Record<"light" | "dark", string> = { /** Map theme mode to CSS color-scheme value */ function getColorScheme(theme: ThemeMode): "light" | "dark" { - return theme === "light" || theme === "flexoki-light" ? "light" : "dark"; + // Reuse the shared `-light` suffix convention so we have one source of truth for the light/dark mapping. + return isLightThemeMode(theme) ? "light" : "dark"; } function applyThemeFavicon(theme: ThemeMode) { diff --git a/src/browser/features/Settings/Sections/ProvidersSection.tsx b/src/browser/features/Settings/Sections/ProvidersSection.tsx index b7c140a04c..7d82c7b08e 100644 --- a/src/browser/features/Settings/Sections/ProvidersSection.tsx +++ b/src/browser/features/Settings/Sections/ProvidersSection.tsx @@ -78,6 +78,7 @@ import type { AddCustomOpenAICompatibleProviderInput, ProviderConfigInfo, } from "@/common/orpc/types"; +import type { ServiceTier } from "@/common/config/schemas/providersConfig"; type MuxGatewayLoginStatus = "idle" | "starting" | "waiting" | "success" | "error"; type CodexOauthFlowStatus = "idle" | "starting" | "waiting" | "error"; @@ -85,7 +86,7 @@ type CopilotLoginStatus = "idle" | "starting" | "waiting" | "success" | "error"; const OPENAI_SERVICE_TIER_UNSET = "unset"; -type OpenAIServiceTier = "auto" | "default" | "flex" | "priority"; +type OpenAIServiceTier = ServiceTier; type OpenAIServiceTierSelectValue = typeof OPENAI_SERVICE_TIER_UNSET | OpenAIServiceTier; function isOpenAIServiceTier(value: string): value is OpenAIServiceTier { diff --git a/src/browser/hooks/useAutoScroll.ts b/src/browser/hooks/useAutoScroll.ts index dfe455c3c7..ac990558ca 100644 --- a/src/browser/hooks/useAutoScroll.ts +++ b/src/browser/hooks/useAutoScroll.ts @@ -83,6 +83,19 @@ export function useAutoScroll() { setAutoScroll(enabled); }, []); + // Seed the baseline read by handleScroll's released-branch direction check + // (`currentScrollTop > previousScrollTop`). Call this from any code path that + // flips autoScrollRef / programmaticDisableRef without a guaranteed follow-up + // scroll event — e.g. jumpToBottom skips the write when scrollTop is already + // max, and disableAutoScroll never fires a scroll event itself. Without a + // fresh baseline, the next user-driven scroll event could compare against a + // stale value (carried across workspace switches or the prior session) and + // misread a small wheel-up notch as "moving toward bottom", spuriously + // relocking the lock that was just released. + const seedScrollDirectionBaseline = useCallback(() => { + lastScrollTopRef.current = contentRef.current?.scrollTop ?? 0; + }, []); + const stickToBottom = useCallback(() => { const scrollContainer = contentRef.current; if (!scrollContainer) return; @@ -142,29 +155,22 @@ export function useAutoScroll() { programmaticDisableRef.current = false; setAutoScrollEnabled(true); stickToBottom(); - // Seed the direction baseline used by handleScroll's released-branch - // user-intent path. stickToBottom doesn't always emit a scroll event - // (it skips the write when scrollTop is already max), so without this - // seed the next user-driven scroll event could compare against a stale - // value carried across workspace switches or earlier sessions. - lastScrollTopRef.current = contentRef.current?.scrollTop ?? 0; + // stickToBottom skips the write when scrollTop is already max, so we may + // not get a follow-up scroll event to refresh lastScrollTopRef. + seedScrollDirectionBaseline(); startBottomLockFrameLoop(); - }, [setAutoScrollEnabled, startBottomLockFrameLoop, stickToBottom]); + }, [seedScrollDirectionBaseline, setAutoScrollEnabled, startBottomLockFrameLoop, stickToBottom]); const disableAutoScroll = useCallback(() => { userScrollIntentUntilRef.current = 0; programmaticDisableRef.current = true; setAutoScrollEnabled(false); - // Seed the direction baseline. The released-branch user-intent path in - // handleScroll compares the next scroll event's scrollTop against - // lastScrollTopRef. disableAutoScroll never fires a scroll event itself, - // so without this seed a small wheel-up notch following a programmatic - // disable would be misread as "moving toward bottom" (because - // previousScrollTop was 0 or some unrelated earlier value), spuriously - // relocking the lock that was just disabled. - lastScrollTopRef.current = contentRef.current?.scrollTop ?? 0; + // disableAutoScroll never fires a scroll event itself, so seed the + // baseline now to keep the next user-driven scroll event's direction + // check honest. + seedScrollDirectionBaseline(); stopBottomLockFrameLoop(); - }, [setAutoScrollEnabled, stopBottomLockFrameLoop]); + }, [seedScrollDirectionBaseline, setAutoScrollEnabled, stopBottomLockFrameLoop]); const markUserScrollIntent = useCallback(() => { programmaticDisableRef.current = false; diff --git a/src/browser/utils/chatCommands.test.ts b/src/browser/utils/chatCommands.test.ts index 5af6130512..7a467f3b8c 100644 --- a/src/browser/utils/chatCommands.test.ts +++ b/src/browser/utils/chatCommands.test.ts @@ -53,27 +53,25 @@ beforeEach(() => { }); describe("parseRuntimeString", () => { - const workspaceName = "test-workspace"; - test("returns undefined for undefined runtime (default to worktree)", () => { - expect(parseRuntimeString(undefined, workspaceName)).toBeUndefined(); + expect(parseRuntimeString(undefined)).toBeUndefined(); }); test("returns undefined for explicit 'worktree' runtime", () => { - expect(parseRuntimeString("worktree", workspaceName)).toBeUndefined(); - expect(parseRuntimeString("WORKTREE", workspaceName)).toBeUndefined(); - expect(parseRuntimeString(" worktree ", workspaceName)).toBeUndefined(); + expect(parseRuntimeString("worktree")).toBeUndefined(); + expect(parseRuntimeString("WORKTREE")).toBeUndefined(); + expect(parseRuntimeString(" worktree ")).toBeUndefined(); }); test("returns local config for explicit 'local' runtime", () => { // "local" now returns project-dir runtime config (no srcBaseDir) - expect(parseRuntimeString("local", workspaceName)).toEqual({ type: "local" }); - expect(parseRuntimeString("LOCAL", workspaceName)).toEqual({ type: "local" }); - expect(parseRuntimeString(" local ", workspaceName)).toEqual({ type: "local" }); + expect(parseRuntimeString("local")).toEqual({ type: "local" }); + expect(parseRuntimeString("LOCAL")).toEqual({ type: "local" }); + expect(parseRuntimeString(" local ")).toEqual({ type: "local" }); }); test("parses valid SSH runtime", () => { - const result = parseRuntimeString("ssh user@host", workspaceName); + const result = parseRuntimeString("ssh user@host"); expect(result).toEqual({ type: "ssh", host: "user@host", @@ -82,7 +80,7 @@ describe("parseRuntimeString", () => { }); test("preserves case in SSH host", () => { - const result = parseRuntimeString("ssh User@Host.Example.Com", workspaceName); + const result = parseRuntimeString("ssh User@Host.Example.Com"); expect(result).toEqual({ type: "ssh", host: "User@Host.Example.Com", @@ -91,7 +89,7 @@ describe("parseRuntimeString", () => { }); test("handles extra whitespace", () => { - const result = parseRuntimeString(" ssh user@host ", workspaceName); + const result = parseRuntimeString(" ssh user@host "); expect(result).toEqual({ type: "ssh", host: "user@host", @@ -100,12 +98,12 @@ describe("parseRuntimeString", () => { }); test("throws error for SSH without host", () => { - expect(() => parseRuntimeString("ssh", workspaceName)).toThrow("SSH runtime requires host"); - expect(() => parseRuntimeString("ssh ", workspaceName)).toThrow("SSH runtime requires host"); + expect(() => parseRuntimeString("ssh")).toThrow("SSH runtime requires host"); + expect(() => parseRuntimeString("ssh ")).toThrow("SSH runtime requires host"); }); test("accepts SSH with hostname only (user will be inferred)", () => { - const result = parseRuntimeString("ssh hostname", workspaceName); + const result = parseRuntimeString("ssh hostname"); // Uses tilde path - backend will resolve it via runtime.resolvePath() expect(result).toEqual({ type: "ssh", @@ -115,7 +113,7 @@ describe("parseRuntimeString", () => { }); test("accepts SSH with hostname.domain only", () => { - const result = parseRuntimeString("ssh dev.example.com", workspaceName); + const result = parseRuntimeString("ssh dev.example.com"); // Uses tilde path - backend will resolve it via runtime.resolvePath() expect(result).toEqual({ type: "ssh", @@ -125,7 +123,7 @@ describe("parseRuntimeString", () => { }); test("uses tilde path for root user too", () => { - const result = parseRuntimeString("ssh root@hostname", workspaceName); + const result = parseRuntimeString("ssh root@hostname"); // Backend will resolve ~ to /root for root user expect(result).toEqual({ type: "ssh", @@ -135,7 +133,7 @@ describe("parseRuntimeString", () => { }); test("parses docker runtime with image", () => { - const result = parseRuntimeString("docker ubuntu:22.04", workspaceName); + const result = parseRuntimeString("docker ubuntu:22.04"); expect(result).toEqual({ type: "docker", image: "ubuntu:22.04", @@ -143,10 +141,7 @@ describe("parseRuntimeString", () => { }); test("parses devcontainer runtime with config path", () => { - const result = parseRuntimeString( - "devcontainer .devcontainer/devcontainer.json", - workspaceName - ); + const result = parseRuntimeString("devcontainer .devcontainer/devcontainer.json"); expect(result).toEqual({ type: "devcontainer", configPath: ".devcontainer/devcontainer.json", @@ -154,13 +149,13 @@ describe("parseRuntimeString", () => { }); test("throws error for devcontainer without config path", () => { - expect(() => parseRuntimeString("devcontainer", workspaceName)).toThrow( + expect(() => parseRuntimeString("devcontainer")).toThrow( "Dev container runtime requires a config path" ); }); test("parses docker with registry image", () => { - const result = parseRuntimeString("docker ghcr.io/myorg/dev:latest", workspaceName); + const result = parseRuntimeString("docker ghcr.io/myorg/dev:latest"); expect(result).toEqual({ type: "docker", image: "ghcr.io/myorg/dev:latest", @@ -168,19 +163,15 @@ describe("parseRuntimeString", () => { }); test("throws error for docker without image", () => { - expect(() => parseRuntimeString("docker", workspaceName)).toThrow( - "Docker runtime requires image" - ); - expect(() => parseRuntimeString("docker ", workspaceName)).toThrow( - "Docker runtime requires image" - ); + expect(() => parseRuntimeString("docker")).toThrow("Docker runtime requires image"); + expect(() => parseRuntimeString("docker ")).toThrow("Docker runtime requires image"); }); test("throws error for unknown runtime type", () => { - expect(() => parseRuntimeString("remote", workspaceName)).toThrow( + expect(() => parseRuntimeString("remote")).toThrow( "Unknown runtime type: 'remote'. Use 'ssh ', 'docker ', 'devcontainer ', 'worktree', or 'local'" ); - expect(() => parseRuntimeString("kubernetes", workspaceName)).toThrow( + expect(() => parseRuntimeString("kubernetes")).toThrow( "Unknown runtime type: 'kubernetes'. Use 'ssh ', 'docker ', 'devcontainer ', 'worktree', or 'local'" ); }); diff --git a/src/browser/utils/chatCommands.ts b/src/browser/utils/chatCommands.ts index bb51128e7d..a035b95a32 100644 --- a/src/browser/utils/chatCommands.ts +++ b/src/browser/utils/chatCommands.ts @@ -685,10 +685,7 @@ async function handleForkCommand( * - "devcontainer " -> Dev container runtime * - undefined -> Worktree runtime (default) */ -export function parseRuntimeString( - runtime: string | undefined, - _workspaceName: string -): RuntimeConfig | undefined { +export function parseRuntimeString(runtime: string | undefined): RuntimeConfig | undefined { // Use shared parser from common/types/runtime const parsed = parseRuntimeModeAndHost(runtime); @@ -801,13 +798,8 @@ export async function createNewWorkspace( } } - // Parse runtime config if provided. Use a placeholder when no caller-provided - // workspace name is available (auto-name path); parseRuntimeString only uses - // the name for error reporting context. - const runtimeConfig = parseRuntimeString( - effectiveRuntime, - options.workspaceName ?? "(auto-generated)" - ); + // Parse runtime config if provided. + const runtimeConfig = parseRuntimeString(effectiveRuntime); const result = await options.client.workspace.create({ projectPath: options.projectPath, diff --git a/src/browser/utils/messages/StreamingMessageAggregator.ts b/src/browser/utils/messages/StreamingMessageAggregator.ts index 76987e5bb6..655e924e4f 100644 --- a/src/browser/utils/messages/StreamingMessageAggregator.ts +++ b/src/browser/utils/messages/StreamingMessageAggregator.ts @@ -27,6 +27,7 @@ import { type StreamLifecycleSnapshot, } from "@/common/types/stream"; import type { LanguageModelV2Usage } from "@ai-sdk/provider"; +import type { StreamErrorType } from "@/common/types/errors"; import type { TodoItem, StatusSetToolResult, NotifyToolResult } from "@/common/types/tools"; import { completeInProgressTodoItems } from "@/common/utils/todoList"; import { getToolOutputUiOnly } from "@/common/utils/tools/toolOutputUiOnly"; @@ -3064,20 +3065,37 @@ export class StreamingMessageAggregator { } }); - // Create stream-error DisplayedMessage if message has error metadata - // This happens after all parts are displayed, so error appears at the end - if (message.metadata?.error) { + // Both stream-error rows (real error metadata + synthesized + // max_tokens truncation) share the same parent-message-derived + // fields. Capture them in one place so adding a new branch later + // can't accidentally drift on `model` / `routedThroughGateway` / + // `historySequence` / `timestamp`. + const pushStreamErrorRow = ( + idSuffix: string, + error: string, + errorType: StreamErrorType + ): void => { displayedMessages.push({ type: "stream-error", - id: `${message.id}-error`, + id: `${message.id}-${idSuffix}`, historyId: message.id, - error: message.metadata.error, - errorType: message.metadata.errorType ?? "unknown", + error, + errorType, historySequence, - model: message.metadata.model, + model: message.metadata?.model, routedThroughGateway: message.metadata?.routedThroughGateway, timestamp: baseTimestamp, }); + }; + + // Create stream-error DisplayedMessage if message has error metadata + // This happens after all parts are displayed, so error appears at the end + if (message.metadata?.error) { + pushStreamErrorRow( + "error", + message.metadata.error, + message.metadata.errorType ?? "unknown" + ); } else if ( // Stream ended cleanly *but* the provider truncated us at max_tokens. // The backend's stream-end path treats this as a successful completion @@ -3090,19 +3108,12 @@ export class StreamingMessageAggregator { !hasActiveStream && message.metadata?.finishReason === "length" ) { - displayedMessages.push({ - type: "stream-error", - id: `${message.id}-length`, - historyId: message.id, - error: - "The model hit its max output token limit before finishing this response. " + + pushStreamErrorRow( + "length", + "The model hit its max output token limit before finishing this response. " + "Lower the thinking level (or split the turn into smaller steps) to give it more headroom.", - errorType: "max_output_tokens", - historySequence, - model: message.metadata.model, - routedThroughGateway: message.metadata?.routedThroughGateway, - timestamp: baseTimestamp, - }); + "max_output_tokens" + ); } } diff --git a/src/browser/utils/slashCommands/suggestions.ts b/src/browser/utils/slashCommands/suggestions.ts index 35d6751886..d43b896c43 100644 --- a/src/browser/utils/slashCommands/suggestions.ts +++ b/src/browser/utils/slashCommands/suggestions.ts @@ -82,12 +82,14 @@ function buildTopLevelSuggestions( return scope; }; + // The skill build callback below hardcodes the trailing space, so we omit + // `appendSpace` here — leaving it set would be a no-op and falsely suggest + // the build path consults it. const skillDefinitions: SuggestionDefinition[] = (context.agentSkills ?? []) .filter((skill) => !SLASH_COMMAND_DEFINITION_MAP.has(skill.name)) .map((skill) => ({ key: skill.name, description: `${skill.description} (${formatScopeLabel(skill.scope)})`, - appendSpace: true, })); const skillSuggestions = filterAndMapSuggestions(skillDefinitions, partial, (definition) => { @@ -100,12 +102,13 @@ function buildTopLevelSuggestions( }; }); - // Model alias one-shot suggestions (e.g., /haiku, /sonnet, /opus+high) + // Model alias one-shot suggestions (e.g., /haiku, /sonnet, /opus+high). + // The build callback below hardcodes the trailing space, so `appendSpace` + // is intentionally omitted here. const modelAliasDefinitions: SuggestionDefinition[] = Object.entries(MODEL_ABBREVIATIONS).map( ([alias, modelId]) => ({ key: alias, description: `Send with ${formatModelDisplayName(modelId.split(":")[1] ?? modelId)} (one message, +level for thinking)`, - appendSpace: true, }) ); diff --git a/src/cli/run.ts b/src/cli/run.ts index 8037588351..233c9f0283 100644 --- a/src/cli/run.ts +++ b/src/cli/run.ts @@ -35,6 +35,7 @@ import { type SendMessageOptions, type WorkspaceChatMessage, } from "../common/orpc/types"; +import type { ServiceTier } from "../common/config/schemas/providersConfig"; import { createDisplayUsage } from "../common/utils/tokens/displayUsage"; import { getTotalCost, @@ -298,7 +299,7 @@ interface CLIOptions { mcpConfig: boolean; experiment: string[]; budget?: number; - serviceTier?: "auto" | "default" | "flex" | "priority"; + serviceTier?: ServiceTier; use1m?: boolean; keepBackgroundProcesses?: boolean; } diff --git a/src/common/config/schemas/providersConfig.ts b/src/common/config/schemas/providersConfig.ts index 8748293e27..949ab787a2 100644 --- a/src/common/config/schemas/providersConfig.ts +++ b/src/common/config/schemas/providersConfig.ts @@ -5,6 +5,7 @@ import { ProviderModelEntrySchema } from "./providerModelEntry"; export const CacheTtlSchema = z.enum(["5m", "1h"]); export const ServiceTierSchema = z.enum(["auto", "default", "flex", "priority"]); +export type ServiceTier = z.infer; export const CodexOauthDefaultAuthSchema = z.enum(["oauth", "apiKey"]); export const BaseProviderConfigSchema = z diff --git a/src/common/utils/ai/modelDisplay.ts b/src/common/utils/ai/modelDisplay.ts index 216176ec5f..431d89e917 100644 --- a/src/common/utils/ai/modelDisplay.ts +++ b/src/common/utils/ai/modelDisplay.ts @@ -237,8 +237,10 @@ function parseBedrockModelName(modelId: string): string | null { const knownVendors = ["anthropic", "amazon", "meta", "cohere", "mistral", "ai21"]; const knownRegionPrefixes = ["global", "us", "eu", "ap", "sa"]; + // The early `dotParts.length < 2` return above guarantees both indices exist here, + // so neither access needs a length guard. const firstPart = dotParts[0].toLowerCase(); - const secondPart = dotParts.length > 1 ? dotParts[1].toLowerCase() : ""; + const secondPart = dotParts[1].toLowerCase(); // Format is either: vendor.model or region.vendor.model const isVendor = knownVendors.includes(firstPart); diff --git a/src/node/services/providerModelFactory.ts b/src/node/services/providerModelFactory.ts index fea730ba1f..493d3ba97d 100644 --- a/src/node/services/providerModelFactory.ts +++ b/src/node/services/providerModelFactory.ts @@ -20,6 +20,7 @@ import { import { parseCodexOauthAuth } from "@/node/utils/codexOauthAuth"; import type { Config, ProviderConfig, ProvidersConfig } from "@/node/config"; import type { MuxProviderOptions } from "@/common/types/providerOptions"; +import type { ServiceTier } from "@/common/config/schemas/providersConfig"; import type { ExternalSecretResolver } from "@/common/types/secrets"; import { isOpReference } from "@/common/utils/opRef"; import { isProviderDisabledInConfig } from "@/common/utils/providers/isProviderDisabled"; @@ -1279,7 +1280,7 @@ export class ProviderModelFactory { if (configServiceTier && muxProviderOptions.openai?.serviceTier == null) { muxProviderOptions.openai = { ...muxProviderOptions.openai, - serviceTier: configServiceTier as "auto" | "default" | "flex" | "priority", + serviceTier: configServiceTier as ServiceTier, }; } if (configWireFormat === "responses" || configWireFormat === "chatCompletions") { diff --git a/src/node/services/streamContextBuilder.ts b/src/node/services/streamContextBuilder.ts index 7185bc8c75..22a422f355 100644 --- a/src/node/services/streamContextBuilder.ts +++ b/src/node/services/streamContextBuilder.ts @@ -167,48 +167,52 @@ export async function buildPlanInstructions( // Read plan content for agent transition (plan-like → exec). // Only read if switching to the built-in exec agent and last assistant was plan-like. + // The `effectiveAgentId === "exec"` gate used to also include "orchestrator" + // (extracted to an `isPlanHandoffAgent` boolean) before #3224 ripped that agent + // out; the boolean is now redundant with a single equality check, so inline it. let planContentForTransition: string | undefined; - const isPlanHandoffAgent = effectiveAgentId === "exec"; - if (isPlanHandoffAgent && !chatHasStartHerePlanSummary) { - const lastAssistantMessage = [...requestPayloadMessages] - .reverse() - .find((m) => m.role === "assistant"); - const lastAgentId = lastAssistantMessage?.metadata?.agentId; - if (lastAgentId && planResult.content.trim()) { - let lastAgentIsPlanLike = false; - if (lastAgentId === effectiveAgentId) { - lastAgentIsPlanLike = agentIsPlanLike; - } else { - try { - const lastDefinition = await readAgentDefinition( - runtime, - agentDiscoveryPath, - lastAgentId - ); - const lastChain = await resolveAgentInheritanceChain({ - runtime, - workspacePath: agentDiscoveryPath, - agentId: lastAgentId, - agentDefinition: lastDefinition, - workspaceId, - }); - lastAgentIsPlanLike = isPlanLikeInResolvedChain(lastChain); - } catch (error) { - workspaceLog.warn("Failed to resolve last agent definition for plan handoff", { - lastAgentId, - error: getErrorMessage(error), - }); + if (effectiveAgentId === "exec") { + if (chatHasStartHerePlanSummary) { + workspaceLog.debug( + "Skipping plan content injection for plan handoff transition: Start Here already includes the plan in chat history." + ); + } else { + const lastAssistantMessage = [...requestPayloadMessages] + .reverse() + .find((m) => m.role === "assistant"); + const lastAgentId = lastAssistantMessage?.metadata?.agentId; + if (lastAgentId && planResult.content.trim()) { + let lastAgentIsPlanLike = false; + if (lastAgentId === effectiveAgentId) { + lastAgentIsPlanLike = agentIsPlanLike; + } else { + try { + const lastDefinition = await readAgentDefinition( + runtime, + agentDiscoveryPath, + lastAgentId + ); + const lastChain = await resolveAgentInheritanceChain({ + runtime, + workspacePath: agentDiscoveryPath, + agentId: lastAgentId, + agentDefinition: lastDefinition, + workspaceId, + }); + lastAgentIsPlanLike = isPlanLikeInResolvedChain(lastChain); + } catch (error) { + workspaceLog.warn("Failed to resolve last agent definition for plan handoff", { + lastAgentId, + error: getErrorMessage(error), + }); + } } - } - if (lastAgentIsPlanLike) { - planContentForTransition = planResult.content; + if (lastAgentIsPlanLike) { + planContentForTransition = planResult.content; + } } } - } else if (isPlanHandoffAgent && chatHasStartHerePlanSummary) { - workspaceLog.debug( - "Skipping plan content injection for plan handoff transition: Start Here already includes the plan in chat history." - ); } return { effectiveAdditionalInstructions, planFilePath, planContentForTransition }; diff --git a/src/node/services/taskService.ts b/src/node/services/taskService.ts index bea5c35487..90ec917d5a 100644 --- a/src/node/services/taskService.ts +++ b/src/node/services/taskService.ts @@ -88,6 +88,17 @@ export type TaskKind = "agent"; export type AgentTaskStatus = NonNullable; +/** + * Resolved per-agent AI settings (canonical model + optional thinking level). + * + * `thinkingLevel` is optional because internal callers read these settings off of + * partial workspace metadata where the field may be missing on older entries. + */ +interface ResolvedWorkspaceAiSettings { + model: string; + thinkingLevel?: ThinkingLevel; +} + export interface AgentTaskStatusLookup { exists: boolean; taskStatus: AgentTaskStatus | null; @@ -382,11 +393,11 @@ export class TaskService { // fall back to legacy workspace settings for older configs. private resolveWorkspaceAISettings( workspace: { - aiSettingsByAgent?: Record; - aiSettings?: { model: string; thinkingLevel?: ThinkingLevel }; + aiSettingsByAgent?: Record; + aiSettings?: ResolvedWorkspaceAiSettings; }, agentId: string | undefined - ): { model: string; thinkingLevel?: ThinkingLevel } | undefined { + ): ResolvedWorkspaceAiSettings | undefined { const normalizedAgentId = typeof agentId === "string" && agentId.trim().length > 0 ? normalizeAgentId(agentId, "") @@ -400,8 +411,8 @@ export class TaskService { private resolveTaskAISettings(params: { cfg: ReturnType; parentMeta: { - aiSettingsByAgent?: Record; - aiSettings?: { model: string; thinkingLevel?: ThinkingLevel }; + aiSettingsByAgent?: Record; + aiSettings?: ResolvedWorkspaceAiSettings; }; agentId: string; modelString?: string; @@ -450,8 +461,8 @@ export class TaskService { parentWorkspaceId: string, parentEntry: { workspace: { - aiSettingsByAgent?: Record; - aiSettings?: { model: string; thinkingLevel?: ThinkingLevel }; + aiSettingsByAgent?: Record; + aiSettings?: ResolvedWorkspaceAiSettings; }; }, fallbackModel: string, diff --git a/src/node/services/tools/file_edit_insert.ts b/src/node/services/tools/file_edit_insert.ts index fd0354b2b2..12a7f6a6f6 100644 --- a/src/node/services/tools/file_edit_insert.ts +++ b/src/node/services/tools/file_edit_insert.ts @@ -44,8 +44,6 @@ function guardFailure(error: string): InsertOperationFailure { }; } -type GuardAnchors = Pick; - export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) => { return tool({ description: TOOL_DEFINITIONS.file_edit_insert.description, @@ -175,7 +173,7 @@ function insertContent( function insertWithGuards( originalContent: string, contentToInsert: string, - anchors: GuardAnchors + anchors: InsertContentOptions ): InsertOperationSuccess | InsertOperationFailure { const anchorResult = resolveGuardAnchor(originalContent, anchors); if (!anchorResult.success) { @@ -216,7 +214,7 @@ function findUniqueSubstringIndex( function resolveGuardAnchor( originalContent: string, - { insert_before, insert_after }: GuardAnchors + { insert_before, insert_after }: InsertContentOptions ): GuardResolutionSuccess | InsertOperationFailure { const fileEol = detectFileEol(originalContent); diff --git a/src/node/services/tools/task_await.ts b/src/node/services/tools/task_await.ts index b8c6680039..5d05b28d69 100644 --- a/src/node/services/tools/task_await.ts +++ b/src/node/services/tools/task_await.ts @@ -15,10 +15,22 @@ import { import { getErrorMessage } from "@/common/utils/errors"; import { ForegroundWaitBackgroundedError, + type AgentTaskStatus, type AgentTaskStatusLookup, type AgentTaskTimestamps, } from "@/node/services/taskService"; +// Status values for which task_await still treats an agent task as live and +// should surface the live status (plus an `elapsed_ms` field) instead of +// awaiting a report. Centralised here so the timeout=0 and "timed out" error +// branches below stay in lockstep when shared fields are added — see #3234, +// which extended both branches symmetrically with `getAgentTaskElapsedField`. +type AgentTaskActiveStatus = "queued" | "running" | "awaiting_report"; + +function isAgentTaskActiveStatus(status: AgentTaskStatus | null): status is AgentTaskActiveStatus { + return status === "queued" || status === "running" || status === "awaiting_report"; +} + function coerceTimeoutMs(timeoutSecs: unknown): number | undefined { if (typeof timeoutSecs !== "number" || !Number.isFinite(timeoutSecs)) return undefined; if (timeoutSecs < 0) return undefined; @@ -247,7 +259,7 @@ export const createTaskAwaitTool: ToolFactory = (config: ToolConfiguration) => { // current task status instead of awaiting. if (timeoutMs === 0) { const status = taskService.getAgentTaskStatus(taskId); - if (status === "queued" || status === "running" || status === "awaiting_report") { + if (isAgentTaskActiveStatus(status)) { return { status, taskId, ...getAgentTaskElapsedField(taskId) }; } @@ -299,12 +311,9 @@ export const createTaskAwaitTool: ToolFactory = (config: ToolConfiguration) => { } catch (error: unknown) { if (error instanceof ForegroundWaitBackgroundedError) { const currentStatus = taskService.getAgentTaskStatus(taskId); - const normalizedStatus = - currentStatus === "queued" || - currentStatus === "running" || - currentStatus === "awaiting_report" - ? currentStatus - : ("running" as const); + const normalizedStatus = isAgentTaskActiveStatus(currentStatus) + ? currentStatus + : ("running" as const); return { status: normalizedStatus, taskId, @@ -323,7 +332,7 @@ export const createTaskAwaitTool: ToolFactory = (config: ToolConfiguration) => { } if (/timed out/i.test(message)) { const status = taskService.getAgentTaskStatus(taskId); - if (status === "queued" || status === "running" || status === "awaiting_report") { + if (isAgentTaskActiveStatus(status)) { return { status, taskId, ...getAgentTaskElapsedField(taskId) }; } if (!status) {