diff --git a/.storybook/preview.tsx b/.storybook/preview.tsx index e3e6bd69a76..4507eaa10e7 100644 --- a/.storybook/preview.tsx +++ b/.storybook/preview.tsx @@ -197,6 +197,14 @@ const preview: Preview = { styles: { width: "375px", height: "667px" }, type: "mobile", }, + // Mirrors the Pixel snapshot matrix's named "phone" width (390px): + // breakpoint-pinned stories must render locally at the exact width CI + // captures, or wrap-point regressions hide between the two. + pixelPhone: { + name: "Pixel phone (390px)", + styles: { width: "390px", height: "844px" }, + type: "mobile", + }, mobile2: { name: "iPhone XR", styles: { width: "414px", height: "896px" }, diff --git a/docs/agents/agent-skills.mdx b/docs/agents/agent-skills.mdx index e8afff75a50..b843fe7098b 100644 --- a/docs/agents/agent-skills.mdx +++ b/docs/agents/agent-skills.mdx @@ -239,6 +239,47 @@ Substitution rules: Use the `argument-hint` frontmatter field to document the expected arguments in invocation UIs. +## Per-skill model routing + +Mechanical skills (session wrap-up, worktree helpers, PR chores) rarely need your frontier model. Skill invocations can be routed to a **model class** — an indirection that survives model churn, since bindings name a class and only the class map names concrete models. + +Configure the three canonical classes — `large`, `medium`, `small` — in **Settings → Models → Model Classes** (a model plus an optional thinking level per class). Canonical names keep skill bindings portable across machines. The classes are stored in `~/.xum/config.json`, where values use the [one-shot override syntax](/config/models#one-shot-overrides): a model alias or full `provider:model` id, with an optional `+thinking` suffix (named level or model-relative numeric index). Hand-edited custom class names in config.json also work and are preserved by the Settings editor: + +```json +{ + "modelClasses": { + "large": "fable+max", + "medium": "sonnet+high", + "small": "haiku+0" + } +} +``` + +Bind skills to classes in either of two places: + +- **Skill frontmatter** — the spec-standard `metadata` map, so the binding travels with the skill and other agent tools ignore it: + + ```yaml + metadata: + model-class: small + ``` + + For project skills, class routing only applies in **trusted projects** — repo-controlled content must not silently reroute your conversation to a different configured provider. In an untrusted project a project skill is never routed: not by its frontmatter, and not by a `skillModelClasses` entry either, since project skills win name collisions and a repo-shipped shadow would otherwise inherit a binding you meant for your own skill. Global and built-in skills route normally. + +- **Config routing table** — for skills you don't own, `skillModelClasses` in `~/.xum/config.json` maps skill names to classes and **wins over frontmatter**: + + ```json + { + "skillModelClasses": { "done": "small", "wt": "small" } + } + ``` + +Routing applies to the slash invocation's send only: the workspace's selected model is untouched, and your next message streams on it again. If auto-compaction triggers, the threshold is computed against the routed model's context window, and the compaction request starts from whichever of the two models — the workspace's or the routed one — has the larger context window (it must read the full uncompacted history). As with any compaction, settings configured for the Compact agent take precedence over that base. + +Broken bindings fail loudly: when a bound class exists but its value is malformed, or no configured provider route can serve its model (a retired model, a removed provider or key), the send fails with an error naming the mapping to fix — and the Model Classes editor shows the same "no configured route" warning inline. A dangling `skillModelClasses` table entry (naming a class you deleted) also errors, since the table is your own explicit routing intent. Frontmatter bindings to a class you never defined are simply ignored, so skills you don't own can ship `model-class` metadata without ever breaking your sends; infrastructure hiccups (an unreadable skill or config) likewise fall back to the workspace model instead of failing the send. + +To override routing for one invocation, compose a one-shot prefix with the skill: `/sonnet+high /done` runs the skill on Sonnet regardless of its class. A model-carrying one-shot always wins over class routing; a thinking-only one-shot (`/+2 /done`) layers on top of it — the skill still routes to its class model, at the overridden thinking level. Numeric thinking indices are model-relative and resolve against the model that actually streams: in `/+0 /done`, the `0` means the class model's lowest allowed level, not the workspace model's. Both overrides survive compact-and-retry: the rebuilt send keeps the one-shot's model and thinking instead of falling back to routing or ambient settings. + ## Dynamic context injection (experiment) Enable the **Skill dynamic context injection** experiment (Settings → Experiments) to let skills pull live command output into their instructions. When you invoke a skill, any line whose entire content is `` !`command` `` runs in the workspace, and the line is replaced with a fenced block containing the command’s output before the model sees the skill: diff --git a/docs/config/models.mdx b/docs/config/models.mdx index 79d2d099063..c2ebeb204a9 100644 --- a/docs/config/models.mdx +++ b/docs/config/models.mdx @@ -66,12 +66,15 @@ Override the model or thinking level for a single message using slash commands. ### Syntax -| Command | Effect | -| --------------------------- | ---------------------------------------- | -| `/sonnet explain this code` | Use Sonnet for one message | -| `/opus+high deep review` | Use Opus with high thinking | -| `/haiku+0 quick answer` | Use Haiku at its lowest thinking level | -| `/+2 analyze this` | Keep current model, set thinking level 2 | +| Command | Effect | +| --------------------------- | ------------------------------------------- | +| `/sonnet explain this code` | Use Sonnet for one message | +| `/opus+high deep review` | Use Opus with high thinking | +| `/haiku+0 quick answer` | Use Haiku at its lowest thinking level | +| `/+2 analyze this` | Keep current model, set thinking level 2 | +| `/haiku+0 /done` | Run the `done` skill on Haiku for this send | + +One-shot prefixes compose with [skill invocations](/agents/agent-skills): `/haiku+0 /done cleanup` invokes the skill normally (arguments, snapshots) while overriding the model for that send. An explicit one-shot also wins over the skill's own [model-class routing](/agents/agent-skills#per-skill-model-routing). ### Thinking levels diff --git a/src/browser/features/ChatInput/index.tsx b/src/browser/features/ChatInput/index.tsx index ff583132084..77f7f023b1b 100644 --- a/src/browser/features/ChatInput/index.tsx +++ b/src/browser/features/ChatInput/index.tsx @@ -1821,6 +1821,9 @@ const ChatInputInner: React.FC = (props) => { api, discovery: skillDiscovery, signal: resolutionSignal, + // One-shot × skill composition ("/haiku+0 /done") ships for workspace + // sends; the creation composer has no one-shot support to compose with. + composeOneShot: variant === "workspace", }); if (!isSendScopeCurrent()) return; parsed = resolution.parsed; @@ -1971,6 +1974,9 @@ const ChatInputInner: React.FC = (props) => { try { const modelOneShot = parsed?.type === "model-oneshot" ? parsed : null; + // Model/thinking override from either a bare one-shot ("/haiku+0 msg") + // or one composed with a skill invocation ("/haiku+0 /done args"). + const oneShotOverride = modelOneShot ?? skillInvocation?.oneShot ?? null; // Mirror the creation-composer /goal bypass: with attachments present, // send the raw text as a normal message instead of processing the // command, which would drop the files. Transferred staging-failure @@ -1993,7 +1999,7 @@ const ChatInputInner: React.FC = (props) => { // the composer, it must not restore stale command text over the newer turn. asyncCommandTokenRef.current++; - const modelOverride = modelOneShot?.modelString; + const modelOverride = oneShotOverride?.modelString; // Regular message (or / one-shot override) - send directly via API const messageTextForSend = @@ -2032,11 +2038,25 @@ const ChatInputInner: React.FC = (props) => { } } + // Composed one-shot sends highlight the full "/haiku+0 /done" prefix: + // the transcript badge check requires rawCommand.startsWith(commandPrefix), + // and the combined prefix also keeps the explicit override visible. + const composedPrefixMatch = skillInvocation?.oneShot + ? new RegExp(`^\\S+\\s+/${skillInvocation.descriptor.name}(?=\\s|$)`).exec( + messageText.trim() + ) + : null; const skillMuxMetadata = skillInvocation ? buildSkillInvocationMetadata( - appendStagedAttachmentNotice(messageText, sendAttachments), + // Trimmed like the view parseCommand accepted: the transcript + // badge requires rawCommand.startsWith(commandPrefix), so leading + // whitespace (" /haiku+0 /done") would silently drop the badge + // (and the composed prefix highlight) while the skill turn still + // sends. + appendStagedAttachmentNotice(messageText.trim(), sendAttachments), skillInvocation.descriptor, - skillInvocation.argumentText + skillInvocation.argumentText, + composedPrefixMatch?.[0] ) : undefined; const promptMuxMetadata: MuxMessageMetadata | undefined = mcpPromptInvocation @@ -2052,8 +2072,14 @@ const ChatInputInner: React.FC = (props) => { const policyModel = modelOverride ?? baseModel; // Preflight: if the message includes PDFs, ensure the selected model can accept them. + // Routable skill invocations (no explicit model override) may stream on a + // class model with different PDF capabilities than the workspace model, so + // this local check would judge the wrong model both ways — defer to the + // backend gate, which validates against the routed model and rejects with + // a persisted, visible error. + const pdfPreflightModelIsAuthoritative = !(skillInvocation && !modelOverride); const pdfAttachments = attachments.filter(isPdfAttachment); - if (pdfAttachments.length > 0) { + if (pdfAttachments.length > 0 && pdfPreflightModelIsAuthoritative) { const caps = getModelCapabilitiesResolved(policyModel, providersConfig); if (caps && !caps.supportsPdfInput) { const pdfCapableKnownModels = Object.values(KNOWN_MODELS) @@ -2177,6 +2203,10 @@ const ChatInputInner: React.FC = (props) => { compactionMessageText: actualMessageText, appendStagedNotice: appendStagedNoticeToUserMessage, modelOneShot, + // One-shot composed with a skill invocation ("/haiku+0 /done"): + // option overrides ride the invocation, not a bare model-oneshot. + skillOneShot: skillInvocation?.oneShot ?? null, + hasSkillInvocation: skillInvocation != null, policyModel, transferredDraftProjectDiscovery, additionalSystemContextHydrated, @@ -2223,17 +2253,29 @@ const ChatInputInner: React.FC = (props) => { setDraft(preSendDraft); setDraftReviews(preSendReviews); } else { - // Track telemetry for successful message send - telemetry.messageSent( - props.workspaceId, - effectiveModel, - sendMessageOptions.agentId ?? agentId ?? WORKSPACE_DEFAULTS.agentId, - finalMessageText.length, - runtimeType, - sendMessageOptions.thinkingLevel ?? "off" - ); + // Track telemetry for successful message send. Skill class routing + // can swap the model and thinking backend-side; the send result + // reports both so usage is attributed to what actually streams. A + // QUEUED skill send acknowledges before it dispatches — routed or + // not, the backend's dequeue-dispatch capture owns its attribution + // (recording here would double-count, and a later cancellation or + // rejection would make the event wholly false). + const queuedSkillSend = result.data?.queued === true && skillInvocation != null; + if (!queuedSkillSend) { + telemetry.messageSent( + props.workspaceId, + result.data?.routedModel ?? effectiveModel, + sendMessageOptions.agentId ?? agentId ?? WORKSPACE_DEFAULTS.agentId, + finalMessageText.length, + runtimeType, + // Fall back to what this send actually carried (sendOptions + // includes a composed one-shot's thinking), not the ambient + // workspace setting. + result.data?.routedThinkingLevel ?? sendOptions.thinkingLevel ?? "off" + ); + } - if (modelOneShot) { + if (oneShotOverride) { trackCommandUsed("model"); } diff --git a/src/browser/features/ChatInput/prepareMessagePayload.ts b/src/browser/features/ChatInput/prepareMessagePayload.ts index f6e97872e03..b33a81480da 100644 --- a/src/browser/features/ChatInput/prepareMessagePayload.ts +++ b/src/browser/features/ChatInput/prepareMessagePayload.ts @@ -13,7 +13,7 @@ import { } from "@/common/types/message"; import { resolveThinkingInput } from "@/common/utils/thinking/policy"; import { appendStagedAttachmentNotice } from "./stagedAttachments"; -import { hasProjectScopedSkillRef } from "./utils"; +import { hasProjectScopedSkillRef, type SkillInvocation } from "./utils"; import type { GoalInterventionPolicy, QueueDispatchMode } from "./types"; type ModelOneShot = Extract, { type: "model-oneshot" }>; @@ -34,6 +34,15 @@ interface PrepareMessagePayloadInput { compactionMessageText?: string; appendStagedNotice?: boolean; modelOneShot?: ModelOneShot | null; + /** + * One-shot override composed with a skill invocation ("/haiku+0 /done"). + * It rides the invocation rather than parsing as a bare model-oneshot, so + * only option building consumes it — the transcript prefix metadata is the + * skill path's job (composedPrefixMatch in the caller). + */ + skillOneShot?: SkillInvocation["oneShot"] | null; + /** True for slash skill invocations (routable unless a model override rides along). */ + hasSkillInvocation?: boolean; policyModel: string; transferredDraftProjectDiscovery: boolean; additionalSystemContextHydrated: boolean; @@ -86,8 +95,12 @@ export function prepareMessagePayload(input: PrepareMessagePayloadInput): Prepar const additionalSystemInstructions = compactionOptions.additionalSystemInstructions ?? input.sendMessageOptions.additionalSystemInstructions; + // Model/thinking override from either a bare one-shot ("/haiku+0 msg") or + // one composed with a skill invocation ("/haiku+0 /done args"). + const oneShotOverride = input.modelOneShot ?? input.skillOneShot ?? null; + const oneShotModelOverride = oneShotOverride?.modelString; const effectiveModel = - input.modelOneShot?.modelString ?? compactionOptions.model ?? input.sendMessageOptions.model; + oneShotModelOverride ?? compactionOptions.model ?? input.sendMessageOptions.model; const trimmedMessageText = input.messageText.trim(); const commandPrefix = input.modelOneShot ? trimmedMessageText @@ -103,7 +116,7 @@ export function prepareMessagePayload(input: PrepareMessagePayloadInput): Prepar ...(rawCommand ? { rawCommand, commandPrefix } : {}), }; - const rawThinkingOverride = input.modelOneShot?.thinkingLevel; + const rawThinkingOverride = oneShotOverride?.thinkingLevel; const thinkingOverride = rawThinkingOverride != null ? resolveThinkingInput(rawThinkingOverride, input.policyModel) @@ -119,9 +132,21 @@ export function prepareMessagePayload(input: PrepareMessagePayloadInput): Prepar ...(input.transferredDraftProjectDiscovery && hasProjectScopedSkillRef(input.agentSkillRefs) ? { disableWorkspaceAgents: true } : {}), - ...(input.modelOneShot?.modelString ? { model: input.modelOneShot.modelString } : {}), + ...(oneShotModelOverride ? { model: oneShotModelOverride } : {}), ...(thinkingOverride ? { thinkingLevel: thinkingOverride } : {}), - ...(input.modelOneShot ? { skipAiSettingsPersistence: true } : {}), + ...(oneShotOverride ? { skipAiSettingsPersistence: true } : {}), + // Only a model-carrying one-shot bypasses class routing; a thinking-only + // override (/+2 /skill) layers on top of routing. + ...(oneShotModelOverride ? { skipSkillModelRouting: true } : {}), + // Numeric thinking is model-relative and thinkingOverride above was + // resolved against the workspace model. A routable skill send may stream + // on a different (class) model, so pass the raw index for the backend to + // re-resolve against whatever model actually streams. + ...(input.hasSkillInvocation === true && + !oneShotModelOverride && + typeof rawThinkingOverride === "number" + ? { oneShotThinkingIndex: rawThinkingOverride } + : {}), ...(input.goalInterventionPolicy ? { goalInterventionPolicy: input.goalInterventionPolicy } : {}), diff --git a/src/browser/features/ChatInput/stagedAttachments.ts b/src/browser/features/ChatInput/stagedAttachments.ts index aad71bf2320..ba270be386d 100644 --- a/src/browser/features/ChatInput/stagedAttachments.ts +++ b/src/browser/features/ChatInput/stagedAttachments.ts @@ -42,6 +42,25 @@ export function appendStagedAttachmentNotice(text: string, attachments: ChatAtta return text.trim().length > 0 ? `${text}\n${notice}` : notice.trimStart(); } +/** + * The raw blocks in a message text, verbatim. For rebuilds + * that regenerate the surrounding model-facing text (compact-and-retry): + * staged attachments are deliberately absent from fileParts, so dropping the + * generated notice would silently lose the file and its workspace path. + */ +export function extractStagedAttachmentNotices(text: string): string[] { + return ( + text + .match(ATTACHED_FILES_BLOCK_PATTERN) + ?.map((block) => block.replace(/^\n/, "")) + // Only GENERATED notices: a skill argument can itself contain an + // example (pasted XML, fenced block) that the rebuilt + // argument text already restores — re-appending it would duplicate + // user content and change the retried prompt. + .filter((block) => isGeneratedStagedAttachmentBlock(block)) ?? [] + ); +} + export function parseStagedAttachmentNotice(text: string): { text: string; attachments: DisplayStagedAttachment[]; diff --git a/src/browser/features/ChatInput/utils.oneShotSkillComposition.test.ts b/src/browser/features/ChatInput/utils.oneShotSkillComposition.test.ts new file mode 100644 index 00000000000..527b56b0675 --- /dev/null +++ b/src/browser/features/ChatInput/utils.oneShotSkillComposition.test.ts @@ -0,0 +1,139 @@ +import { describe, expect, test } from "bun:test"; + +import { KNOWN_MODELS } from "@/common/constants/knownModels"; +import type { AgentSkillDescriptor } from "@/common/types/agentSkill"; +import { parseCommandWithSkillInvocation } from "./utils"; + +function descriptor(name: string): AgentSkillDescriptor { + return { name, description: `${name} description`, scope: "project" }; +} + +describe("parseCommandWithSkillInvocation one-shot composition", () => { + test("composes '/haiku+0 /done args' into a skill invocation with a one-shot override", async () => { + const result = await parseCommandWithSkillInvocation({ + messageText: "/haiku+0 /done now please", + agentSkillDescriptors: [descriptor("done")], + api: null, + discovery: null, + composeOneShot: true, + }); + + expect(result.parsed).toBeNull(); + expect(result.skillInvocation?.descriptor.name).toBe("done"); + expect(result.skillInvocation?.userText).toBe("Using skill done: now please"); + // Arguments are relative to the skill token so $ARGUMENTS substitution + // sees "now please", not the one-shot prefix. + expect(result.skillInvocation?.argumentText).toBe("now please"); + expect(result.skillInvocation?.oneShot).toEqual({ + modelString: KNOWN_MODELS.HAIKU.id, + thinkingLevel: 0, + }); + }); + + test("composes a thinking-only override ('/+2 /done')", async () => { + const result = await parseCommandWithSkillInvocation({ + messageText: "/+2 /done", + agentSkillDescriptors: [descriptor("done")], + api: null, + discovery: null, + composeOneShot: true, + }); + + expect(result.parsed).toBeNull(); + expect(result.skillInvocation?.userText).toBe("Use skill done"); + expect(result.skillInvocation?.oneShot).toEqual({ thinkingLevel: 2 }); + }); + + test("does not compose without the composeOneShot opt-in (creation composer)", async () => { + const result = await parseCommandWithSkillInvocation({ + messageText: "/haiku+0 /done now", + agentSkillDescriptors: [descriptor("done")], + api: null, + discovery: null, + }); + + expect(result.skillInvocation).toBeNull(); + expect(result.parsed?.type).toBe("model-oneshot"); + }); + + test("keeps valid registered-command invocations out of composition ('/haiku+0 /compact')", async () => { + const result = await parseCommandWithSkillInvocation({ + messageText: "/haiku+0 /compact", + agentSkillDescriptors: [descriptor("compact")], + api: null, + discovery: null, + composeOneShot: true, + }); + + expect(result.skillInvocation).toBeNull(); + expect(result.parsed).toMatchObject({ type: "model-oneshot", message: "/compact" }); + }); + + test("mirrors direct invocation for unknown-command remainders, even on command-colliding names", async () => { + // "/compact now" is an invalid compact usage: its handler returns + // unknown-command, and unknown commands are exactly what skill invocation + // consumes. Direct typing already resolves a skill named "compact" here, + // so the composed form must behave identically. + const direct = await parseCommandWithSkillInvocation({ + messageText: "/compact now", + agentSkillDescriptors: [descriptor("compact")], + api: null, + discovery: null, + composeOneShot: true, + }); + const composed = await parseCommandWithSkillInvocation({ + messageText: "/haiku+0 /compact now", + agentSkillDescriptors: [descriptor("compact")], + api: null, + discovery: null, + composeOneShot: true, + }); + + expect(direct.skillInvocation?.descriptor.name).toBe("compact"); + expect(composed.skillInvocation?.descriptor.name).toBe("compact"); + expect(composed.skillInvocation?.oneShot).toEqual({ + modelString: KNOWN_MODELS.HAIKU.id, + thinkingLevel: 0, + }); + }); + + test("keeps nested one-shots out of composition ('/haiku+0 /opus hi')", async () => { + const result = await parseCommandWithSkillInvocation({ + messageText: "/haiku+0 /opus hi", + agentSkillDescriptors: [descriptor("done")], + api: null, + discovery: null, + composeOneShot: true, + }); + + expect(result.skillInvocation).toBeNull(); + expect(result.parsed).toMatchObject({ type: "model-oneshot", message: "/opus hi" }); + }); + + test("falls back to a plain one-shot when the remainder is not a known skill", async () => { + const result = await parseCommandWithSkillInvocation({ + messageText: "/haiku+0 /nothere do it", + agentSkillDescriptors: [descriptor("done")], + api: null, + discovery: null, + composeOneShot: true, + }); + + expect(result.skillInvocation).toBeNull(); + expect(result.parsed).toMatchObject({ type: "model-oneshot", message: "/nothere do it" }); + }); + + test("plain skill invocations are unaffected by the composition flag", async () => { + const result = await parseCommandWithSkillInvocation({ + messageText: "/done now", + agentSkillDescriptors: [descriptor("done")], + api: null, + discovery: null, + composeOneShot: true, + }); + + expect(result.parsed).toBeNull(); + expect(result.skillInvocation?.descriptor.name).toBe("done"); + expect(result.skillInvocation?.oneShot).toBeUndefined(); + }); +}); diff --git a/src/browser/features/ChatInput/utils.ts b/src/browser/features/ChatInput/utils.ts index df052a4ef59..35727dcc5e5 100644 --- a/src/browser/features/ChatInput/utils.ts +++ b/src/browser/features/ChatInput/utils.ts @@ -14,10 +14,12 @@ import { isMcpPromptCommandKey, } from "@/common/utils/tools/mcpPromptCommandKey"; import type { ParsedRuntime } from "@/common/types/runtime"; +import type { ParsedThinkingInput } from "@/common/types/thinking"; import { buildAgentSkillMetadata, dedupeAgentSkillRefs, buildMcpPromptUserText, + buildSkillInvocationUserText, dedupeMcpPromptRefs, type AgentSkillReference, type MCPPromptReference, @@ -38,6 +40,16 @@ export interface SkillInvocation { userText: string; /** Trimmed text after the slash command (e.g. "123 high" for "/fix-issue 123 high"). */ argumentText: string; + /** + * One-shot model/thinking override composed with the invocation + * ("/haiku+0 /done args"). Applies to this send only; carrying + * skipAiSettingsPersistence also bypasses backend per-skill class routing + * (an explicit override wins over the skill's model class). + */ + oneShot?: { + modelString?: string; + thinkingLevel?: ParsedThinkingInput; + }; } export interface MCPPromptInvocation { @@ -89,25 +101,23 @@ function isUnknownSlashCommand(value: ParsedCommand): value is UnknownSlashComma export function buildSkillInvocationMetadata( rawCommand: string, descriptor: AgentSkillDescriptor, - argumentText: string + argumentText: string, + /** + * Overrides the default `/${name}` prefix for composed one-shot invocations + * ("/haiku+0 /done"): the transcript badge only renders when rawCommand + * starts with commandPrefix, so the prefix must include the one-shot token. + */ + commandPrefixOverride?: string ): MuxMessageMetadata { return buildAgentSkillMetadata({ rawCommand, - commandPrefix: `/${descriptor.name}`, + commandPrefix: commandPrefixOverride ?? `/${descriptor.name}`, skillName: descriptor.name, scope: descriptor.scope, arguments: argumentText, }); } -/** - * Format user message text for skill invocation. - * Makes it explicit to the model that a skill was invoked. - */ -function formatSkillInvocationText(skillName: string, userMessage: string): string { - return userMessage ? `Using skill ${skillName}: ${userMessage}` : `Use skill ${skillName}`; -} - // parseCommand() trims before matching, so pasted or draft-restored text with // leading whitespace must be sliced from the same trimmed view or the command // falls through as literal text. @@ -276,7 +286,7 @@ async function resolveSkillInvocation(options: { return { descriptor: skill, - userText: formatSkillInvocationText(skill.name, afterPrefix.trimStart()), + userText: buildSkillInvocationUserText(skill.name, afterPrefix.trimStart()), argumentText: afterPrefix.trim(), }; } @@ -288,6 +298,8 @@ export async function parseCommandWithSkillInvocation(options: { api: APIClient | null; discovery: SkillResolutionTarget | null; signal?: AbortSignal; + /** Allow "/model+thinking /skill args" composition (workspace sends only). */ + composeOneShot?: boolean; }): Promise<{ parsed: ParsedCommand; skillInvocation: SkillInvocation | null; @@ -312,7 +324,7 @@ export async function parseCommandWithSkillInvocation(options: { }; } - const skillInvocation = await resolveSkillInvocation({ + let skillInvocation = await resolveSkillInvocation({ messageText: options.messageText, parsed, agentSkillDescriptors: options.agentSkillDescriptors, @@ -320,6 +332,35 @@ export async function parseCommandWithSkillInvocation(options: { discovery: options.discovery, }); + // Compose one-shot model overrides with skill invocations: "/haiku+0 /done args" + // runs the done skill on Haiku for this send only. Re-running parseCommand on the + // one-shot's message keeps registered commands ("/haiku+0 /compact") and nested + // one-shots out of skill resolution — only unknown-command remainders are + // candidate skills, exactly like a bare "/done args". + if ( + options.composeOneShot === true && + skillInvocation == null && + parsed?.type === "model-oneshot" + ) { + const innerParsed = parseCommand(parsed.message); + const innerInvocation = await resolveSkillInvocation({ + messageText: parsed.message, + parsed: innerParsed, + agentSkillDescriptors: options.agentSkillDescriptors, + api: options.api, + discovery: options.discovery, + }); + if (innerInvocation != null) { + skillInvocation = { + ...innerInvocation, + oneShot: { + ...(parsed.modelString != null ? { modelString: parsed.modelString } : {}), + ...(parsed.thinkingLevel != null ? { thinkingLevel: parsed.thinkingLevel } : {}), + }, + }; + } + } + return { parsed: skillInvocation == null ? parsed : null, skillInvocation, diff --git a/src/browser/features/Settings/Sections/ModelClassesEditor.tsx b/src/browser/features/Settings/Sections/ModelClassesEditor.tsx new file mode 100644 index 00000000000..2cc97a004df --- /dev/null +++ b/src/browser/features/Settings/Sections/ModelClassesEditor.tsx @@ -0,0 +1,242 @@ +import { X } from "lucide-react"; + +import { Button } from "@/browser/components/Button/Button"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/browser/components/SelectPrimitive/SelectPrimitive"; +import { TooltipIfPresent } from "@/browser/components/Tooltip/Tooltip"; +import { useModelClasses } from "@/browser/hooks/useModelClasses"; +import { useModelsFromSettings } from "@/browser/hooks/useModelsFromSettings"; +import { useProvidersConfig } from "@/browser/hooks/useProvidersConfig"; +import { useRouting } from "@/browser/hooks/useRouting"; +import { + getThinkingOptionLabel, + parseThinkingInput, + type ThinkingLevel, +} from "@/common/types/thinking"; +import { isModelServableWithProvidersConfig } from "@/common/utils/ai/modelAvailability"; +import { + buildModelClassValue, + CANONICAL_MODEL_CLASSES, + parseModelClassValue, + splitModelClassValue, +} from "@/common/utils/ai/skillModelClasses"; +import { getThinkingPolicyForModel, resolveThinkingInput } from "@/common/utils/thinking/policy"; + +const MODEL_SELECT_TRIGGER_CLASS = + "border-border-medium bg-background-secondary hover:bg-hover h-7 w-64 cursor-pointer rounded-md border px-2 text-xs transition-colors"; +const THINKING_SELECT_TRIGGER_CLASS = + "border-border-medium bg-background-secondary hover:bg-hover h-7 w-28 cursor-pointer rounded-md border px-2 text-xs transition-colors"; + +/** Sentinel Select value for "no thinking suffix" (Radix rejects empty item values). */ +const THINKING_DEFAULT_OPTION = "default"; + +/** + * Model class editor (Settings → Models). + * + * Model classes are the indirection behind per-skill model routing: skills + * bind to a class name (frontmatter `metadata: model-class`, or the + * `skillModelClasses` table in config.json) and the class maps to a concrete + * model here. When models change, updating the class re-routes every bound + * skill at once. + * + * The editor surfaces exactly the canonical classes (large/medium/small) so + * skill bindings stay portable across machines; hand-edited custom classes in + * config.json keep working and are preserved on save, but are not editable + * here. + */ +export function ModelClassesEditor() { + const { modelClasses, loaded: classesLoaded, pendingWrites, setModelClass } = useModelClasses(); + const { models } = useModelsFromSettings(); + const { config: providersConfig } = useProvidersConfig(); + const routing = useRouting(); + + // Candidates keep their EXACT selection identity: a direct model and its + // explicit gateway form (openai:x vs openrouter:openai/x, coder:anthropic/x) + // dispatch differently and must both stay selectable — deduping by a + // canonical/metadata key would collapse them and hide whichever the + // Settings list happens to order second. Only exact duplicates dedupe + // (SelectItem values must be unique). + const modelCandidates = Array.from(new Set(models)); + + const canonicalNames: readonly string[] = CANONICAL_MODEL_CLASSES; + const customEntries = Object.entries(modelClasses) + .filter(([name]) => !canonicalNames.includes(name)) + .sort(([a], [b]) => a.localeCompare(b)); + + const renderClassRow = (className: string) => { + const rawValue = modelClasses[className]; + const parsed = rawValue ? parseModelClassValue(rawValue) : null; + const { thinkingSuffix } = rawValue ? splitModelClassValue(rawValue) : { thinkingSuffix: null }; + const selectedModel = parsed?.model ?? ""; + // Show numeric (model-relative) suffixes as the level they resolve to for + // the selected model; re-saving through the select writes the named level. + // providersConfig resolves mappedToModel aliases — the send-path resolver + // passes it too, so the ladder shown here matches what routing will use. + const selectedThinking: ThinkingLevel | null = + parsed?.thinkingLevel != null && parsed.model + ? resolveThinkingInput(parsed.thinkingLevel, parsed.model, providersConfig) + : null; + const thinkingOptions = selectedModel + ? getThinkingPolicyForModel(selectedModel, providersConfig) + : []; + // On a model switch, carry the raw thinking suffix only when it stays + // meaningful: numeric suffixes are model-relative by design, named levels + // must exist in the new model's ladder, and an unparseable suffix (the + // "invalid value" repair case) is dropped so picking a model actually + // fixes the row instead of re-persisting a value sanitization-era builds + // would have deleted. + const carrySuffixTo = (nextModel: string): string | null => { + if (parsed == null || thinkingSuffix == null) { + return null; + } + const parsedSuffix = parseThinkingInput(thinkingSuffix); + if (parsedSuffix == null) { + return null; + } + if (typeof parsedSuffix === "number") { + return thinkingSuffix; + } + return getThinkingPolicyForModel(nextModel, providersConfig).includes(parsedSuffix) + ? thinkingSuffix + : null; + }; + // Ensure the selected model is offerable even if hidden from the picker + // list (e.g. a hand-configured custom model). + const rowModelCandidates = + selectedModel && !modelCandidates.includes(selectedModel) + ? [selectedModel, ...modelCandidates] + : modelCandidates; + // Proactive churn warning: the class points at a model no configured + // route can serve (skill sends bound to it will fail with the same + // verdict). Null providersConfig = still loading — say nothing yet. The + // verdict also waits for routing.loaded: judging against the default + // ["direct"] priority would flash a false warning on gateway-routed + // setups every time Settings opens. + const modelUnavailable = + parsed != null && + providersConfig != null && + routing.loaded && + !isModelServableWithProvidersConfig({ + canonicalModel: parsed.model, + routePriority: routing.routePriority, + routeOverrides: routing.routeOverrides, + providersConfig, + }); + // State publishes on the write's ack, so a second edit made before the ack + // would compose against the still-old rendered value and overwrite the + // first edit. Disable the row's controls until its write settles. + const rowWritePending = (pendingWrites[className] ?? 0) > 0; + const rowDisabled = !classesLoaded || rowWritePending; + + return ( +
+ {className} + + + {rawValue !== undefined && ( + + )} + {rawValue !== undefined && parsed == null && ( + + invalid value: {rawValue} + + )} + {modelUnavailable && ( + + no configured route can serve this model — update this class + + )} +
+ ); + }; + + return ( +
+
Model Classes
+

+ Size classes for per-skill model routing. A skill bound to a class (frontmatter{" "} + metadata: model-class, or the{" "} + skillModelClasses table in config.json) runs on the + class's model for that invocation only — your workspace model is untouched. When models + change, update the class here and every bound skill follows. +

+ +
{canonicalNames.map((name) => renderClassRow(name))}
+ + {customEntries.length > 0 && ( + // break-all: hand-edited values can be long unbroken model ids that + // would otherwise overflow the panel's right edge at phone widths. +

+ Custom classes (edit in config.json):{" "} + {customEntries.map(([name, value]) => `${name} → ${value}`).join(", ")} +

+ )} +
+ ); +} diff --git a/src/browser/features/Settings/Sections/ModelClassesEditor.ui.test.tsx b/src/browser/features/Settings/Sections/ModelClassesEditor.ui.test.tsx new file mode 100644 index 00000000000..535c0643b5e --- /dev/null +++ b/src/browser/features/Settings/Sections/ModelClassesEditor.ui.test.tsx @@ -0,0 +1,238 @@ +import { cleanup, render, fireEvent, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; +import { installDom } from "../../../../../tests/ui/dom"; +import { getAppConfigStore } from "@/browser/stores/AppConfigStore"; +import type { APIClient } from "@/browser/contexts/API"; + +let apiMock: { + config: { + getConfig: ReturnType; + updateModelClass: ReturnType; + onConfigChanged: ReturnType; + }; +} | null = null; + +/** Providers map for the availability warning; null = still loading (warning suppressed). */ +let providersConfigMock: Record | null = + null; + +void mock.module("@/browser/contexts/API", () => ({ + useOptionalAPI: () => (apiMock ? { api: apiMock } : null), + // useRouting (imported by the editor) reads the API through useAPI. + useAPI: () => ({ api: apiMock }), +})); + +void mock.module("@/browser/hooks/useProvidersConfig", () => ({ + useProvidersConfig: () => ({ config: providersConfigMock, loading: providersConfigMock == null }), +})); + +void mock.module("@/browser/hooks/useModelsFromSettings", () => ({ + useModelsFromSettings: () => ({ + models: ["anthropic:claude-haiku-4-5", "anthropic:claude-sonnet-5", "anthropic:claude-fable-5"], + hiddenModelsForSelector: [], + }), +})); + +import { ModelClassesEditor } from "./ModelClassesEditor"; + +function createApiMock(modelClasses: Record) { + return { + config: { + getConfig: mock(() => Promise.resolve({ modelClasses })), + updateModelClass: mock(() => Promise.resolve(undefined)), + onConfigChanged: mock((_input: undefined, opts: { signal?: AbortSignal }) => + Promise.resolve( + (async function* (): AsyncGenerator { + // Stay OPEN like the real stream: an iterator that ends reads as a + // dead subscription and correctly marks the hook stale/unloaded, + // which would refuse the writes these tests exercise. Resolve only + // on abort (cleanup). + await new Promise((resolve) => { + if (opts.signal?.aborted) { + resolve(); + return; + } + opts.signal?.addEventListener("abort", () => resolve(), { once: true }); + }); + yield* [] as void[]; + })() + ) + ), + }, + }; +} + +describe("ModelClassesEditor", () => { + let restoreDom: (() => void) | null = null; + + beforeEach(() => { + restoreDom = installDom(); + }); + + afterEach(() => { + cleanup(); + restoreDom?.(); + restoreDom = null; + apiMock = null; + providersConfigMock = null; + getAppConfigStore().setClient(null); + }); + + test("renders the three canonical class rows; clear button only on configured classes", async () => { + apiMock = createApiMock({ small: "anthropic:claude-haiku-4-5+0" }); + // Row presence is asserted via the labeled row groups, not the select + // triggers: other suites in the same process (TasksSection) mock + // SelectPrimitive with native elements, and bun's mock.module leaks across + // test files, so select internals are not stable to assert on. + const { getByRole, queryByLabelText } = render(); + + await waitFor(() => { + expect(apiMock?.config.getConfig).toHaveBeenCalled(); + expect(queryByLabelText("Clear model class small")).not.toBeNull(); + }); + + for (const name of ["large", "medium", "small"]) { + expect(getByRole("group", { name: `Model class ${name}` })).toBeTruthy(); + } + // Unset classes have nothing to clear. + expect(queryByLabelText("Clear model class large")).toBeNull(); + expect(queryByLabelText("Clear model class medium")).toBeNull(); + }); + + test("clearing a canonical class issues a per-entry delete", async () => { + // Only the edited entry travels: other classes (concurrent consumers' + // edits, hand-edited values this build cannot parse) are untouched by + // construction because the backend merges inside its config transaction. + apiMock = createApiMock({ + small: "anthropic:claude-haiku-4-5+0", + "my-custom": "anthropic:claude-fable-5+max", + }); + const { getByLabelText, queryByLabelText } = render(); + + await waitFor(() => expect(queryByLabelText("Clear model class small")).not.toBeNull()); + fireEvent.click(getByLabelText("Clear model class small")); + + await waitFor(() => expect(apiMock?.config.updateModelClass).toHaveBeenCalled()); + expect(apiMock?.config.updateModelClass).toHaveBeenCalledTimes(1); + expect(apiMock?.config.updateModelClass).toHaveBeenCalledWith({ + className: "small", + model: null, + }); + }); + + test("a write's ack refetches so concurrent peer edits surface", async () => { + // A peer consumer's notification-triggered fetch can be fenced by this + // write's ack (the ack's version bump discards in-flight snapshots), and + // no further notification is guaranteed: the ack must follow up with an + // authoritative fetch, or the peer's entry would stay invisible + // indefinitely and a later local edit could overwrite it. + let currentMap: Record = { small: "anthropic:claude-haiku-4-5+0" }; + apiMock = createApiMock({}); + apiMock.config.getConfig = mock(() => Promise.resolve({ modelClasses: currentMap })); + apiMock.config.updateModelClass = mock((input: { className: string; model: string | null }) => { + const merged = { ...currentMap }; + if (input.model == null) { + delete merged[input.className]; + } else { + merged[input.className] = input.model; + } + // A peer consumer's concurrent edit is already in the backend state the + // ack's refetch reads back. + merged.medium = "anthropic:claude-sonnet-5+1"; + currentMap = merged; + return Promise.resolve(undefined); + }); + const { getByLabelText, queryByLabelText } = render(); + + await waitFor(() => expect(queryByLabelText("Clear model class small")).not.toBeNull()); + fireEvent.click(getByLabelText("Clear model class small")); + + await waitFor(() => expect(queryByLabelText("Clear model class medium")).not.toBeNull()); + expect(queryByLabelText("Clear model class small")).toBeNull(); + }); + + test("a fetch resolving after the subscription dies cannot enable editing", async () => { + // Transport death race: the subscription ends (without abort-driven + // cleanup) while the post-subscribe getConfig is still pending. Letting + // that late fetch land as fresh would re-enable full-map writes on a map + // whose peer edits are invisible — the resolved map must stay fenced + // until the resubscribe's own fetch re-establishes truth. + // Object property (not a let binding): TS keeps a closure-assigned let + // narrowed to its null initializer, but property narrowing resets at the + // waitFor call below. + const configGate: { + resolve: ((value: { modelClasses: Record }) => void) | null; + } = { resolve: null }; + apiMock = { + config: { + getConfig: mock( + () => + new Promise<{ modelClasses: Record }>((resolve) => { + configGate.resolve = resolve; + }) + ), + updateModelClass: mock(() => Promise.resolve(undefined)), + onConfigChanged: mock((_input: undefined, _opts: { signal?: AbortSignal }) => + Promise.resolve( + (async function* (): AsyncGenerator { + // Ends immediately: a dead subscription, not cleanup. + await Promise.resolve(); + yield* [] as void[]; + })() + ) + ), + }, + }; + const { queryByLabelText } = render(); + + // The hook subscribes, starts the fetch, then observes the stream die. + await waitFor(() => expect(apiMock?.config.getConfig).toHaveBeenCalled()); + configGate.resolve?.({ modelClasses: { small: "anthropic:claude-haiku-4-5+0" } }); + // Let any (wrongly) accepted state publish before asserting. + await new Promise((resolve) => setTimeout(resolve, 50)); + + // Whether the fetch was discarded outright (no row value surfaces) or + // published display-only (row rendered but disabled), editing must be + // impossible: a click on a disabled clear button is inert. + const clearButton = queryByLabelText("Clear model class small"); + if (clearButton) { + fireEvent.click(clearButton); + } + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(apiMock.config.updateModelClass).not.toHaveBeenCalled(); + }); + + test("lists custom classes as config-managed instead of hiding them", async () => { + apiMock = createApiMock({ "my-custom": "anthropic:claude-fable-5+max" }); + const { findByText } = render(); + + expect(await findByText(/my-custom → anthropic:claude-fable-5\+max/)).toBeTruthy(); + }); + + test("flags an unparseable configured value instead of silently dropping it", async () => { + apiMock = createApiMock({ small: "garbage" }); + const { findByText } = render(); + + expect(await findByText(/invalid value: garbage/)).toBeTruthy(); + }); + + test("warns when no configured route can serve a class model", async () => { + apiMock = createApiMock({ small: "anthropic:claude-haiku-4-5+0" }); + // The warning gates on useRouting's `loaded`, which reads the shared + // AppConfigStore singleton — prime it like useRouting.test does. + getAppConfigStore().setClient(apiMock as unknown as APIClient); + providersConfigMock = { anthropic: { isConfigured: false } }; + const { findByText } = render(); + + expect(await findByText(/no configured route can serve this model/)).toBeTruthy(); + }); + + test("does not warn when the class model has a configured route", async () => { + apiMock = createApiMock({ small: "anthropic:claude-haiku-4-5+0" }); + providersConfigMock = { anthropic: { isConfigured: true, isEnabled: true } }; + const { queryByText, queryByLabelText } = render(); + + await waitFor(() => expect(queryByLabelText("Clear model class small")).not.toBeNull()); + expect(queryByText(/no configured route can serve this model/)).toBeNull(); + }); +}); diff --git a/src/browser/features/Settings/Sections/ModelsSection.stories.tsx b/src/browser/features/Settings/Sections/ModelsSection.stories.tsx index 07df912205f..3e7efabf81f 100644 --- a/src/browser/features/Settings/Sections/ModelsSection.stories.tsx +++ b/src/browser/features/Settings/Sections/ModelsSection.stories.tsx @@ -70,6 +70,18 @@ export const ModelsConfigured: Story = { ); return setupSettingsStory({ + modelClasses: { + large: "anthropic:claude-opus-4-8+max", + // xai is deliberately unconfigured in this story: exercises the + // "no configured route" warning on the medium row. + medium: "xai:grok-beta", + small: "anthropic:claude-sonnet-4-20250514+0", + // Hand-edited custom class with a long unbroken model id: pins + // the custom-classes line's wrap treatment at phone width (the + // pinned ModelsConfiguredPhone variant snapshots this). + "batch-experiments": + "someproxy:organization-research/experimental-preview-20260815-extended-context", + }, providersConfig: { anthropic: { apiKeySet: true, @@ -121,3 +133,22 @@ export const ModelsConfigured: Story = { ); }, }; + +/** + * Pinned phone-width snapshot of the configured state: the Model Classes rows + * wrap their label/select layout at narrow widths, and without an explicit + * Pixel phone variant CI would only ever snapshot the desktop layout the + * wrapping is meant to protect against. globals.viewport mirrors the Pixel + * matrix so local Storybook shows the same width. + */ +export const ModelsConfiguredPhone: Story = { + ...ModelsConfigured, + globals: { + viewport: { value: "pixelPhone", isRotated: false }, + }, + parameters: { + pixel: { + matrix: { themes: ["dark", "light"], viewports: ["phone"] }, + }, + }, +}; diff --git a/src/browser/features/Settings/Sections/ModelsSection.tsx b/src/browser/features/Settings/Sections/ModelsSection.tsx index 3d775eed592..91aa531d59a 100644 --- a/src/browser/features/Settings/Sections/ModelsSection.tsx +++ b/src/browser/features/Settings/Sections/ModelsSection.tsx @@ -2,6 +2,7 @@ import { useCallback, useEffect, useMemo, useState } from "react"; import { ArrowRight, Info, Loader2, Plus, ShieldCheck } from "lucide-react"; import { useProviderOptions } from "@/browser/hooks/useProviderOptions"; import { Button } from "@/browser/components/Button/Button"; +import { ModelClassesEditor } from "./ModelClassesEditor"; import { ModelFallbacksEditor } from "./ModelFallbacksEditor"; import { ProviderIcon } from "@/browser/components/ProviderIcon/ProviderIcon"; import { @@ -591,6 +592,8 @@ export function ModelsSection() { + +
diff --git a/src/browser/features/Settings/Sections/ProvidersSection.test.tsx b/src/browser/features/Settings/Sections/ProvidersSection.test.tsx index 25c8894a169..1a66ea54175 100644 --- a/src/browser/features/Settings/Sections/ProvidersSection.test.tsx +++ b/src/browser/features/Settings/Sections/ProvidersSection.test.tsx @@ -68,6 +68,10 @@ void mock.module("@/browser/hooks/useRouting", () => ({ useRouting: () => ({ routePriority: ["direct"], routeOverrides: {}, + // Consumed by ModelClassesEditor's warning gate; bun mock.module leaks + // across test files, so this stub must stay shape-complete for suites + // that run after this file in the same process. + loaded: true, resolveRoute: () => ({ route: "direct", isAuto: true, displayName: "Direct" }), availableRoutes: () => [], setRoutePreferences: () => undefined, diff --git a/src/browser/features/Settings/Sections/settingsStoryUtils.tsx b/src/browser/features/Settings/Sections/settingsStoryUtils.tsx index ba12725f39c..9c56e4e1f91 100644 --- a/src/browser/features/Settings/Sections/settingsStoryUtils.tsx +++ b/src/browser/features/Settings/Sections/settingsStoryUtils.tsx @@ -132,6 +132,8 @@ export function SettingsSectionStory(props: SettingsSectionStoryProps) { interface SetupSettingsStoryOptions { layoutPresets?: LayoutPresetsConfig; + /** Initial model classes for Settings → Models → Model Classes. */ + modelClasses?: Record; providersConfig?: Record< string, { @@ -186,6 +188,7 @@ export function setupSettingsStory(options: SetupSettingsStoryOptions): APIClien taskSettings: options.taskSettings, serverAuthSessions: options.serverAuthSessions, layoutPresets: options.layoutPresets, + modelClasses: options.modelClasses, }); } diff --git a/src/browser/hooks/useCompactAndRetry.followUp.test.ts b/src/browser/hooks/useCompactAndRetry.followUp.test.ts index 729dad7935c..87e468a5e96 100644 --- a/src/browser/hooks/useCompactAndRetry.followUp.test.ts +++ b/src/browser/hooks/useCompactAndRetry.followUp.test.ts @@ -90,6 +90,66 @@ describe("buildFollowUpFromSource", () => { ]); }); + test("rebuilds the model-facing invocation text for a composed one-shot skill turn", () => { + // dispatchPendingFollowUp sends text verbatim (no slash parsing): the + // retry must stream the same payload the original send did, not the + // displayed raw command — while the overrides and metadata survive. + const followUp = buildFollowUpFromSource( + userMessage({ + content: "/haiku+0 /done finish", + commandPrefix: "/haiku+0 /done", + agentSkill: { skillName: "done", scope: "global", arguments: "finish" }, + agentSkillRefs: [{ skillName: "done", scope: "global", source: "slash" }], + }) + ); + + expect(followUp.text).toBe("Using skill done: finish"); + // parseCommand canonicalizes the "haiku" alias. + expect(followUp.model).toBe("anthropic:claude-haiku-4-5"); + expect(followUp.skipSkillModelRouting).toBe(true); + expect(followUp.muxMetadata?.type).toBe("agent-skill"); + if (followUp.muxMetadata?.type === "agent-skill") { + expect(followUp.muxMetadata.rawCommand).toBe("/haiku+0 /done finish"); + expect(followUp.muxMetadata.commandPrefix).toBe("/haiku+0 /done"); + } + }); + + test("keeps generated attached-files notices through the skill text rebuild", () => { + // Staged attachments are deliberately absent from fileParts: dropping + // the notice would silently lose the file and its workspace path. Only + // the GENERATED notice format counts — an example + // inside the user's own argument text is already restored by the + // argument rebuild and must not be appended a second time. + const generatedNotice = + "\n" + + "The user attached file(s) that were saved into the workspace filesystem. These are not native model attachments; use filesystem tools such as `bash`, `file_read`, or archive tools to inspect them if needed.\n" + + "\n" + + "- `data.csv` (`text/csv`, 12 B): `.xum/user-attachments/data.csv`\n" + + ""; + const userExample = "not generated"; + const followUp = buildFollowUpFromSource( + userMessage({ + content: `/done finish ${userExample}\n${generatedNotice}`, + agentSkill: { skillName: "done", scope: "global", arguments: `finish ${userExample}` }, + agentSkillRefs: [{ skillName: "done", scope: "global", source: "slash" }], + }) + ); + + expect(followUp.text).toBe(`Using skill done: finish ${userExample}\n${generatedNotice}`); + }); + + test("rebuilds argument-less skill invocations with the bare form", () => { + const followUp = buildFollowUpFromSource( + userMessage({ + content: "/done", + agentSkill: { skillName: "done", scope: "global", arguments: "" }, + agentSkillRefs: [{ skillName: "done", scope: "global", source: "slash" }], + }) + ); + + expect(followUp.text).toBe("Use skill done"); + }); + test("does not duplicate the slash skill ref when preserving displayed refs", () => { const followUp = buildFollowUpFromSource( userMessage({ diff --git a/src/browser/hooks/useCompactAndRetry.ts b/src/browser/hooks/useCompactAndRetry.ts index 282ec651e3d..f060f59d059 100644 --- a/src/browser/hooks/useCompactAndRetry.ts +++ b/src/browser/hooks/useCompactAndRetry.ts @@ -15,6 +15,9 @@ import { type CompactionSuggestion, } from "@/browser/utils/compaction/suggestion"; import { executeCompaction } from "@/browser/utils/chatCommands"; +import { extractStagedAttachmentNotices } from "@/browser/features/ChatInput/stagedAttachments"; +import { parseCommand } from "@/browser/utils/slashCommands/parser"; +import { resolveThinkingInput } from "@/common/utils/thinking/policy"; import { CUSTOM_EVENTS, createCustomEvent } from "@/common/constants/events"; import { AGENT_AI_DEFAULTS_KEY } from "@/common/constants/storage"; import type { FilePart, ProvidersConfigMap } from "@/common/orpc/types"; @@ -22,6 +25,7 @@ import type { AgentAiDefaults } from "@/common/types/agentAiDefaults"; import { buildAgentSkillMetadata, buildMcpPromptUserText, + buildSkillInvocationUserText, withAgentSkillRefs, withMcpPromptRefs, type CompactionFollowUpInput, @@ -56,20 +60,64 @@ function findTriggerUserMessage( * Preserves skill metadata if the original message was a skill invocation. */ export function buildFollowUpFromSource( - source: Extract + source: Extract, + // Null ctx degrades gracefully: numeric one-shot thinking rides along as an + // index for the backend to resolve instead of resolving here. + ctx: { providersConfig: ProvidersConfigMap | null; currentModel: string | null } = { + providersConfig: null, + currentModel: null, + } ): CompactionFollowUpInput { const slashMcpPromptRef = source.mcpPromptRefs?.find((ref) => ref.source === "slash"); + // A composed one-shot skill send ("/haiku+0 /done args") stores the full + // typed text as content; re-parse it so the rebuilt follow-up keeps the + // explicit model AND thinking overrides instead of falling back to class + // routing / ambient workspace thinking. + // trimStart matches parseCommand's own tolerance for leading whitespace — + // a column-zero guard would silently drop the preserved one-shot. + const parsedOneShot = + source.agentSkill && source.content.trimStart().startsWith("/") + ? parseCommand(source.content) + : null; + const oneShot = parsedOneShot?.type === "model-oneshot" ? parsedOneShot : null; + const oneShotModel = oneShot?.modelString; + const rawThinking = oneShot?.thinkingLevel; + + // Numeric thinking is model-relative. With an explicit model it resolves + // right here; without one the send may get class-routed, so the raw index + // rides along for the backend to resolve against whatever model streams + // (the resolved fallback below applies only if routing doesn't happen). + let thinkingLevel: CompactionFollowUpInput["thinkingLevel"]; + let oneShotThinkingIndex: number | undefined; + if (rawThinking != null) { + if (typeof rawThinking !== "number") { + thinkingLevel = rawThinking; + } else if (oneShotModel != null) { + thinkingLevel = resolveThinkingInput(rawThinking, oneShotModel, ctx.providersConfig); + } else { + oneShotThinkingIndex = rawThinking; + thinkingLevel = ctx.currentModel + ? resolveThinkingInput(rawThinking, ctx.currentModel, ctx.providersConfig) + : undefined; + } + } + const carriesOneShot = oneShotModel != null || rawThinking != null; + const skillMetadata = source.agentSkill && source.agentSkill.skillName !== slashMcpPromptRef?.commandKey ? buildAgentSkillMetadata({ rawCommand: source.content, + // Preserve the displayed prefix (composed one-shots render + // "/haiku+0 /done") — without it the rebuilt transcript loses the + // command badge, which keys its highlighting on this value. + commandPrefix: source.commandPrefix, skillName: source.agentSkill.skillName, scope: source.agentSkill.scope, arguments: source.agentSkill.arguments, }) : undefined; - // MCP slash messages display raw commands but send transformed text. Rebuild + // Slash messages display raw commands but send transformed text. Rebuild // provider content and preserve slash metadata so retried rows remain editable // as their original invocation. let text = source.content; @@ -78,7 +126,18 @@ export function buildFollowUpFromSource( // original send from a trimmed view); rawCommand keeps source.content // verbatim so the retried row displays exactly like the original. const trimmedContent = source.content.trimStart(); - if (slashMcpPromptRef && trimmedContent.startsWith("/")) { + if (skillMetadata && source.agentSkill) { + // The displayed content is the raw command ("/haiku+0 /done finish"), + // but dispatchPendingFollowUp sends text VERBATIM (no slash parsing): + // the retry must carry the same model-facing payload the original send + // streamed, with the raw command retained in metadata only. Generated + // notices ride the content (staged attachments are + // deliberately absent from fileParts) and must survive the rebuild. + text = [ + buildSkillInvocationUserText(source.agentSkill.skillName, source.agentSkill.arguments ?? ""), + ...extractStagedAttachmentNotices(source.content), + ].join("\n"); + } else if (slashMcpPromptRef && trimmedContent.startsWith("/")) { const argumentText = trimmedContent.replace(/^\/\S+/, "").trimStart(); text = buildMcpPromptUserText( slashMcpPromptRef.serverName, @@ -96,6 +155,12 @@ export function buildFollowUpFromSource( text, fileParts: source.fileParts, reviews: source.reviews, + ...(oneShotModel != null ? { model: oneShotModel, skipSkillModelRouting: true } : {}), + ...(thinkingLevel != null ? { thinkingLevel } : {}), + ...(oneShotThinkingIndex != null ? { oneShotThinkingIndex } : {}), + // The original one-shot send never persisted its overrides as workspace + // defaults; the re-dispatch must not either. + ...(carriesOneShot ? { skipAiSettingsPersistence: true } : {}), // Inline skill refs must survive alongside prompt refs; withAgentSkillRefs // dedupes against the slash ref that buildAgentSkillMetadata already added. muxMetadata: withAgentSkillRefs( @@ -310,7 +375,10 @@ export function useCompactAndRetry(props: { workspaceId: string }): CompactAndRe } // For normal messages (not /compact), build follow-up content directly. - const followUpContent = buildFollowUpFromSource(source); + const followUpContent = buildFollowUpFromSource(source, { + providersConfig, + currentModel: workspaceState?.currentModel ?? null, + }); const result = await executeCompaction({ api, workspaceId: props.workspaceId, @@ -332,7 +400,14 @@ export function useCompactAndRetry(props: { workspaceId: string }): CompactAndRe setIsRetryingWithCompaction(false); } } - }, [api, compactionSuggestion, props.workspaceId, triggerUserMessage]); + }, [ + api, + compactionSuggestion, + props.workspaceId, + triggerUserMessage, + providersConfig, + workspaceState?.currentModel, + ]); /** * Auto-compact on context_exceeded. Runs silently - never touches chat input. @@ -349,7 +424,10 @@ export function useCompactAndRetry(props: { workspaceId: string }): CompactAndRe try { const sendMessageOptions = getSendOptionsFromStorage(props.workspaceId); - const followUpContent = buildFollowUpFromSource(triggerUserMessage); + const followUpContent = buildFollowUpFromSource(triggerUserMessage, { + providersConfig, + currentModel: workspaceState?.currentModel ?? null, + }); const result = await executeCompaction({ api, @@ -372,7 +450,14 @@ export function useCompactAndRetry(props: { workspaceId: string }): CompactAndRe setIsRetryingWithCompaction(false); } } - }, [api, compactionSuggestion?.modelId, props.workspaceId, triggerUserMessage]); + }, [ + api, + compactionSuggestion?.modelId, + props.workspaceId, + triggerUserMessage, + providersConfig, + workspaceState?.currentModel, + ]); // Auto-trigger compaction on context_exceeded for seamless recovery. // Only auto-compact if we have a compaction suggestion; otherwise show manual UI. diff --git a/src/browser/hooks/useModelClasses.ts b/src/browser/hooks/useModelClasses.ts new file mode 100644 index 00000000000..4fcf32ada27 --- /dev/null +++ b/src/browser/hooks/useModelClasses.ts @@ -0,0 +1,322 @@ +import { useEffect, useRef, useState } from "react"; +import { useOptionalAPI } from "@/browser/contexts/API"; + +export interface ModelClassesState { + /** Class name → model value in one-shot syntax ("haiku+0"). */ + modelClasses: Record; + /** + * True while the map reflects live backend truth (first fetch landed and + * the config subscription is delivering). Consumers gate their controls on + * this: editing a row whose current value cannot be trusted invites + * blind overwrites of that entry. + */ + loaded: boolean; + /** + * Classes with a write still in flight (state publishes on the write's + * ack). Editors must disable a pending row's controls: a second edit built + * from the still-unpublished rendered state would compose against the old + * value and overwrite the first edit. + */ + pendingWrites: Record; + // Arrow-function property type so consumers can destructure without + // tripping @typescript-eslint/unbound-method. + /** Set (or clear, with null/empty) one class's model value. */ + setModelClass: (className: string, value: string | null) => void; +} + +/** + * Reads/writes the model-classes map (skill routing indirection) from app + * config. Fetch on mount, subscribe to config changes, publish local edits on + * their write's ack. Writes are PER-ENTRY and merged inside the backend's + * config transaction, so a concurrent Settings consumer editing a different + * class — or a hand-edited custom class — can never be deleted by a stale + * client-side map snapshot. + */ +export function useModelClasses(): ModelClassesState { + const api = useOptionalAPI()?.api ?? null; + const [modelClasses, setMap] = useState>({}); + const [loaded, setLoaded] = useState(false); + // Ignore stale config fetches so backend refreshes can't overwrite newer optimistic edits. + const fetchVersionRef = useRef(0); + // Bumped whenever the API client changes: write acknowledgements (and their + // failure refetches) belong to the client that issued them — an old client's + // late ack must not publish its entry over state fetched from the + // replacement client, nor invalidate the replacement's in-flight fetch. + const clientGenerationRef = useRef(0); + // Populated by the subscription effect below; lets the write-failure revert + // in setModelClass reuse the same stale-guarded fetch. A ref (not a + // useCallback) keeps this within the repo's React Compiler conventions — + // no manual memoization for identity stabilization. + const refetchRef = useRef<() => Promise>(() => { + // No-op until the subscription effect installs the real fetch. + return Promise.resolve(); + }); + // Whether the config-change subscription is currently delivering. Fetches + // may only (re)mark the hook loaded while it is: a fetch that outlives a + // dead subscription (the post-subscribe fetch racing the stream's death, + // or a write-path revert issued during the resubscribe backoff) would + // otherwise re-mark the editor live on state that can no longer track + // peer edits. + const subscriptionLiveRef = useRef(false); + // The most recently started fetch (set synchronously at dispatch, cleared + // when it settles while still the latest). The write chain drains this + // before unlocking a row: an awaited refetch can be superseded mid-flight + // by a newer notification-triggered fetch (the newer call bumps the + // version, so the awaited one discards its result at the stale guard) — + // unlocking then would re-enable editing on a map still about to change. + const latestFetchRef = useRef | null>(null); + // Serializes writes so rapid edits persist in order and the last one wins. + const writeChainRef = useRef>(Promise.resolve()); + // Per-class in-flight write counts; consumers disable pending rows. + const [pendingWrites, setPendingWrites] = useState>({}); + + useEffect(() => { + // A client swap (disconnect/reconnect) invalidates anything fetched from + // the previous client: mark the hook unloaded until THIS client's fetch + // lands, and bump the fetch version so an in-flight fetch against the + // old client cannot re-mark the hook loaded. + fetchVersionRef.current++; + clientGenerationRef.current++; + subscriptionLiveRef.current = false; + setLoaded(false); + // The serialization chain and pending-row counts belong to the old + // client too: queueing behind an old-client request could block forever + // if it never settles, and orphaned pending counts would keep rows + // disabled. Old completions skip their own bookkeeping via the + // generation guards below. + writeChainRef.current = Promise.resolve(); + setPendingWrites({}); + + const getConfig = api?.config?.getConfig; + const onConfigChanged = api?.config?.onConfigChanged; + if (!getConfig || !onConfigChanged) { + return; + } + + const fetchConfig = async () => { + const fetchVersion = ++fetchVersionRef.current; + const run = (async () => { + try { + const config = await getConfig(); + if (fetchVersion !== fetchVersionRef.current) { + return; + } + setMap(config.modelClasses ?? {}); + // The loaded upgrade requires a LIVE subscription: a fetch resolving + // after the stream died reflects truth at a moment peer edits were + // already invisible, so it must not re-enable the editor — the + // resubscribe's own post-subscribe fetch does that. + if (subscriptionLiveRef.current) { + setLoaded(true); + } + } catch { + // A failed refresh leaves the rendered map possibly BEHIND another + // process's edit (the notification that triggered this fetch): mark + // the hook unloaded so the editor's controls disable until a later + // fetch re-establishes truth. Stale-version failures change + // nothing — a newer fetch owns the state. While the subscription + // itself is still LIVE, no further notification is guaranteed (the + // write's own notification often precedes its ack), so schedule an + // authoritative retry rather than leaving every control disabled + // until an unrelated config change or reconnect. Transport retry + // backoff, not component coordination. + if (fetchVersion === fetchVersionRef.current) { + setLoaded(false); + setTimeout(() => { + if ( + !signal.aborted && + subscriptionLiveRef.current && + fetchVersion === fetchVersionRef.current + ) { + void fetchConfig(); + } + }, 2_000); + } + } + })(); + latestFetchRef.current = run; + void run.finally(() => { + if (latestFetchRef.current === run) { + latestFetchRef.current = null; + } + }); + await run; + }; + refetchRef.current = fetchConfig; + + const abortController = new AbortController(); + const { signal } = abortController; + let iterator: AsyncIterator | null = null; + + (async () => { + while (!signal.aborted) { + try { + const subscribedIterator = await onConfigChanged(undefined, { signal }); + if (signal.aborted) { + void subscribedIterator.return?.(); + return; + } + iterator = subscribedIterator; + subscriptionLiveRef.current = true; + // Authoritative fetch only AFTER the subscription is live: the + // stream has no replay, so an edit landing between a + // pre-subscription read and registration would be silently missed — + // the hook would stay loaded on a stale map. If subscribing fails, + // no fetch runs and the editor stays disabled (loaded=false) + // rather than editable-but-stale. + void fetchConfig(); + for await (const _ of subscribedIterator) { + if (signal.aborted) { + break; + } + void fetchConfig(); + } + } catch { + // Aborted cleanup or a transport failure — classified below. + } + if (signal.aborted) { + return; + } + // The subscription ended WITHOUT cleanup (transport interruption + // while the API object survived): peer edits are invisible from this + // moment. Drop liveness and fence any in-flight fetch (it raced the + // same dead stream and would otherwise resolve after this and + // re-mark the hook loaded), go unloaded, then re-establish the + // subscription — its post-subscribe fetch re-loads the editor once + // notifications flow again. The delay is transport retry backoff, + // not component coordination. + subscriptionLiveRef.current = false; + fetchVersionRef.current++; + setLoaded(false); + await new Promise((resolve) => setTimeout(resolve, 2_000)); + } + })(); + + return () => { + abortController.abort(); + void iterator?.return?.(); + }; + }, [api]); + + const setModelClass = (className: string, value: string | null) => { + const key = className.trim(); + // Refuse edits while the rendered value cannot be trusted (before the + // first fetch, or while the subscription is down): the user would be + // blindly overwriting an entry they cannot see. + if (!key || !loaded) { + return; + } + + // Guarded lookup rather than a chained call: in partial-API environments + // (story mocks, tests) a missing route must not throw synchronously. + const updateModelClass = api?.config?.updateModelClass; + if (!updateModelClass) { + return; + } + + const trimmed = value?.trim() ?? ""; + setPendingWrites((current) => ({ ...current, [key]: (current[key] ?? 0) + 1 })); + + // Await whichever fetch currently owns the latest version, not just the + // one this chain started: a peer notification can supersede our refetch + // mid-flight (bumping the version so ours discards its result), and + // unlocking the row on the superseded await would re-enable editing + // before the authoritative map displays. + const awaitLatestFetch = async (): Promise => { + while (latestFetchRef.current != null) { + const inFlight = latestFetchRef.current; + await inFlight; + if (latestFetchRef.current === inFlight) { + return; + } + } + }; + + // The ack/revert below is only meaningful for the client that took the + // write: after a client swap, the old client's late completion must not + // publish over (or refetch under) the replacement client's state. + const writeGeneration = clientGenerationRef.current; + + // Persist BEFORE publishing: routing reads the backend map at send time, + // so optimistically advertising the new mapping would let a quick + // follow-up skill invocation stream on the OLD route while the editor + // claims the new one. The selects update on the write's ack instead. + writeChainRef.current = writeChainRef.current + .then(async () => { + if (clientGenerationRef.current !== writeGeneration) { + return; + } + // Per-entry write, merged inside the backend's config transaction: + // no map composition happens client-side, so a stale local snapshot + // can never delete a peer consumer's class or a hand-edited entry + // this build cannot parse. + await updateModelClass({ className: key, model: trimmed ? trimmed : null }); + if (clientGenerationRef.current !== writeGeneration) { + return; + } + // The ack is the freshest truth for this entry. Invalidate in-flight + // fetches whose snapshot may predate this write, patch the entry + // locally, then refetch authoritatively: the write's own + // config-change notification usually arrives BEFORE this ack, so the + // version bump fences that notification's fetch too — without an + // explicit refetch, a concurrent peer edit it carried would stay + // invisible indefinitely (no further notification is guaranteed) and + // a later local edit of that class would overwrite the peer's value. + fetchVersionRef.current++; + setMap((current) => { + const next = { ...current }; + if (!trimmed) { + delete next[key]; + } else { + next[key] = trimmed; + } + return next; + }); + // AWAITED (never fire-and-forget): the row must stay pending until + // the authoritative map is displayed — clearing pendingWrites while + // a fetch is still in flight would re-enable the row on the locally + // patched value and let the next edit overwrite a peer's unseen + // write of the SAME class. fetchConfig never rejects, and the drain + // covers newer fetches that superseded ours. + await refetchRef.current(); + await awaitLatestFetch(); + }) + .catch(async () => { + // If the write fails, re-fetch so the UI reverts to the backend's + // actual value rather than displaying a class routing never applies. + // A stale-generation failure is not ours to handle: refetchRef already + // points at the replacement client's fetch, which owns its own state. + // AWAITED like the ack path (including the latest-fetch drain): + // clearing pendingWrites while the revert is in flight would + // re-enable the row on stale state and let the next edit overwrite + // a peer's unseen write of the same class. + if (clientGenerationRef.current !== writeGeneration) { + return; + } + await refetchRef.current(); + await awaitLatestFetch(); + }) + .finally(() => { + // A client swap already wiped this write's bookkeeping; decrementing + // here would corrupt the replacement client's fresh pending counts. + if (clientGenerationRef.current !== writeGeneration) { + return; + } + setPendingWrites((current) => { + const count = (current[key] ?? 0) - 1; + if (count > 0) { + return { ...current, [key]: count }; + } + const { [key]: _drop, ...rest } = current; + return rest; + }); + }); + }; + + return { + modelClasses, + loaded, + pendingWrites, + setModelClass, + }; +} diff --git a/src/browser/hooks/useRouting.ts b/src/browser/hooks/useRouting.ts index b8b8539adc5..4db6585ea8d 100644 --- a/src/browser/hooks/useRouting.ts +++ b/src/browser/hooks/useRouting.ts @@ -9,7 +9,10 @@ import { type RouteContext, } from "@/common/routing"; import { normalizeToCanonical } from "@/common/utils/ai/models"; -import { isGatewayModelAccessibleFromAuthoritativeCatalog } from "@/common/utils/providers/gatewayModelCatalog"; +import { + isRouteGatewayModelAccessible, + isRouteProviderConfigured, +} from "@/common/utils/ai/modelAvailability"; import { useProvidersConfig } from "./useProvidersConfig"; @@ -34,6 +37,13 @@ export interface RoutingState { routePriority: string[]; /** Per-model route overrides */ routeOverrides: Record; + /** + * True once the routing config fetch has landed. Until then routePriority + * is the built-in default — availability verdicts computed against it can + * be wrong for gateway-routed setups, so consumers gating UI on route + * reachability must wait for this. + */ + loaded: boolean; /** What route will be used for a given canonical model? */ resolveRoute(canonicalModel: string): { @@ -72,23 +82,23 @@ export function useRouting(): RoutingState { const appConfig = useSyncExternalStore(store.subscribe, store.getSnapshot); const routePriority = appConfig?.routePriority ?? DEFAULT_ROUTE_PRIORITY; const routeOverrides = appConfig?.routeOverrides ?? EMPTY_ROUTE_OVERRIDES; - + // Until the store's first snapshot lands, routePriority is the built-in + // default — availability verdicts computed against it can be wrong for + // gateway-routed setups, so consumers gating UI on route reachability wait. + const loaded = appConfig != null; + + // Shared predicates: the send-path availability check + // (isModelServableWithProvidersConfig) uses these same definitions, so the + // Settings picker and skill-routing verdicts cannot drift apart. const isConfigured = useCallback( (provider: string) => - providersConfig?.[provider]?.isConfigured === true && - providersConfig?.[provider]?.isEnabled !== false, + providersConfig != null && isRouteProviderConfigured(providersConfig, provider), [providersConfig] ); const isGatewayModelAccessible = useCallback( (gateway: string, modelId: string) => - isGatewayModelAccessibleFromAuthoritativeCatalog( - gateway, - modelId, - providersConfig?.[gateway]?.models, - providersConfig?.[gateway]?.discoveredModels, - providersConfig?.[gateway]?.removedModels - ), + providersConfig == null || isRouteGatewayModelAccessible(providersConfig, gateway, modelId), [providersConfig] ); @@ -209,5 +219,6 @@ export function useRouting(): RoutingState { setRoutePreferences, setRoutePriority, setRouteOverride, + loaded, }; } diff --git a/src/browser/stories/mocks/orpc.ts b/src/browser/stories/mocks/orpc.ts index d2e8981c729..6e11d02814d 100644 --- a/src/browser/stories/mocks/orpc.ts +++ b/src/browser/stories/mocks/orpc.ts @@ -155,6 +155,8 @@ export interface MockORPCClientOptions { agentAiDefaults?: AgentAiDefaults; /** Agent definitions to expose via agents.list */ agentDefinitions?: AgentDefinitionDescriptor[]; + /** Initial model classes for config.getConfig (Settings → Models → Model Classes) */ + modelClasses?: Record; /** Coder lifecycle preferences for config.getConfig (e.g., Settings → Coder section) */ coderWorkspaceArchiveBehavior?: CoderWorkspaceArchiveBehavior; /** What to do with xum-managed worktrees when archiving a chat. */ @@ -403,6 +405,7 @@ export function createMockORPCClient(options: MockORPCClientOptions = {}): APICl userPreferences: initialUserPreferences, taskSettings: initialTaskSettings, agentAiDefaults: initialAgentAiDefaults, + modelClasses: initialModelClasses, coderWorkspaceArchiveBehavior: initialCoderWorkspaceArchiveBehavior = "stop", worktreeArchiveBehavior: initialWorktreeArchiveBehavior = "keep", chatTranscriptFullWidth: initialChatTranscriptFullWidth = false, @@ -639,6 +642,7 @@ export function createMockORPCClient(options: MockORPCClientOptions = {}): APICl }; let layoutPresets = initialLayoutPresets ?? DEFAULT_LAYOUT_PRESETS_CONFIG; + let modelClasses: Record | undefined = initialModelClasses; const mockStats: ChatStats = { consumers: [], @@ -774,6 +778,7 @@ export function createMockORPCClient(options: MockORPCClientOptions = {}): APICl runtimeEnablement, defaultRuntime, agentAiDefaults, + modelClasses, muxGovernorUrl, heartbeatDefaultPrompt, heartbeatDefaultIntervalMs, @@ -826,6 +831,17 @@ export function createMockORPCClient(options: MockORPCClientOptions = {}): APICl notifyConfigChanged(); return Promise.resolve(undefined); }, + updateModelClass: (input: { className: string; model: string | null }) => { + const merged = { ...modelClasses }; + if (input.model == null) { + delete merged[input.className]; + } else { + merged[input.className] = input.model; + } + modelClasses = Object.keys(merged).length > 0 ? merged : undefined; + notifyConfigChanged(); + return Promise.resolve(undefined); + }, updateMuxGatewayPrefs: (input: { muxGatewayEnabled: boolean; muxGatewayModels: string[]; diff --git a/src/browser/utils/chatCommands.test.ts b/src/browser/utils/chatCommands.test.ts index 631260e3a30..b6f94a4a770 100644 --- a/src/browser/utils/chatCommands.test.ts +++ b/src/browser/utils/chatCommands.test.ts @@ -1242,6 +1242,59 @@ describe("prepareCompactionMessage", () => { expect(metadata.parsed.followUpContent?.agentId).toBe("code"); }); + test("carried one-shot overrides win over ambient preserved send options", () => { + // A compact-and-retry rebuild of "/haiku+0 /skill" carries the one-shot's + // model, thinking, and persistence semantics in followUpContent; the + // ambient stored options (different thinking, no skip flags) must not + // clobber them. + const sendMessageOptions = createBaseOptions(); + + const { metadata } = prepareCompactionMessage({ + workspaceId: "ws-1", + followUpContent: { + text: "/haiku+0 /done finish up", + model: "anthropic:claude-3-5-haiku", + skipSkillModelRouting: true, + thinkingLevel: "off", + skipAiSettingsPersistence: true, + }, + sendMessageOptions, + }); + + expectCompactionMetadata(metadata); + + const followUp = metadata.parsed.followUpContent; + expect(followUp?.model).toBe("anthropic:claude-3-5-haiku"); + expect(followUp?.skipSkillModelRouting).toBe(true); + expect(followUp?.thinkingLevel).toBe("off"); + expect(followUp?.skipAiSettingsPersistence).toBe(true); + }); + + test("a thinking-only carried one-shot keeps its raw index for routed re-resolution", () => { + const sendMessageOptions = createBaseOptions(); + + const { metadata } = prepareCompactionMessage({ + workspaceId: "ws-1", + followUpContent: { + text: "/+0 /done finish up", + thinkingLevel: "medium", + oneShotThinkingIndex: 0, + skipAiSettingsPersistence: true, + }, + sendMessageOptions, + }); + + expectCompactionMetadata(metadata); + + const followUp = metadata.parsed.followUpContent; + // No model override: the re-dispatch stays routable... + expect(followUp?.model).toBe("anthropic:claude-sonnet-4-6"); + expect(followUp?.skipSkillModelRouting).toBeUndefined(); + // ...and the raw index survives so the backend can re-ladder it. + expect(followUp?.oneShotThinkingIndex).toBe(0); + expect(followUp?.thinkingLevel).toBe("medium"); + }); + test("does not create followUpContent when no text or images provided", () => { const sendMessageOptions = createBaseOptions(); const { metadata } = prepareCompactionMessage({ diff --git a/src/browser/utils/chatCommands.ts b/src/browser/utils/chatCommands.ts index 383e6a69488..0af9e5581c6 100644 --- a/src/browser/utils/chatCommands.ts +++ b/src/browser/utils/chatCommands.ts @@ -1514,6 +1514,24 @@ export function prepareCompactionMessage(options: CompactionOptions): { model: existingModel ?? options.sendMessageOptions.model, agentId: existingAgentId ?? options.sendMessageOptions.agentId ?? WORKSPACE_DEFAULTS.agentId, ...pickPreservedSendOptions(options.sendMessageOptions), + // One-shot overrides reconstructed from the original send (model / + // thinking carried by the follow-up content) must win over the ambient + // stored options that pickPreservedSendOptions just spread — otherwise a + // compact-and-retry of "/haiku+0 /skill" re-dispatches with the + // workspace's thinking and persistence semantics instead of the + // one-shot's. + ...(options.followUpContent.thinkingLevel != null + ? { thinkingLevel: options.followUpContent.thinkingLevel } + : {}), + ...(options.followUpContent.oneShotThinkingIndex != null + ? { oneShotThinkingIndex: options.followUpContent.oneShotThinkingIndex } + : {}), + ...(options.followUpContent.skipSkillModelRouting != null + ? { skipSkillModelRouting: options.followUpContent.skipSkillModelRouting } + : {}), + ...(options.followUpContent.skipAiSettingsPersistence != null + ? { skipAiSettingsPersistence: options.followUpContent.skipAiSettingsPersistence } + : {}), }; } diff --git a/src/common/config/schemas/appConfigOnDisk.ts b/src/common/config/schemas/appConfigOnDisk.ts index 4a26dc81457..e8c721de4d8 100644 --- a/src/common/config/schemas/appConfigOnDisk.ts +++ b/src/common/config/schemas/appConfigOnDisk.ts @@ -170,6 +170,20 @@ export const AppConfigOnDiskSchema = z * runtime sanitization rules (drop self, de-dupe, cap length). */ modelFallbacks: ModelFallbacksSchema.optional(), + /** + * Named model classes (e.g. large/medium/small → a model alias or + * "provider:model" id with an optional "+thinking" suffix, one-shot + * syntax). Indirection layer so bindings survive model churn; consumed by + * per-skill model routing (see skillModelClasses and the skill frontmatter + * metadata "model-class" key). + */ + modelClasses: z.record(z.string(), z.string()).optional(), + /** + * Per-skill routing table (skill name → class name in modelClasses). + * Wins over a skill's own frontmatter metadata "model-class" binding, so + * skills the user does not own can be routed without editing them. + */ + skillModelClasses: z.record(z.string(), z.string()).optional(), defaultModel: z.string().optional(), advisorModelString: z.string().optional(), advisorThinkingLevel: ThinkingLevelSchema.optional(), diff --git a/src/common/constants/ui.ts b/src/common/constants/ui.ts index 9a7a4c74431..f9f4228d401 100644 --- a/src/common/constants/ui.ts +++ b/src/common/constants/ui.ts @@ -69,6 +69,18 @@ Write in a factual, dense style. Every sentence should convey essential context. */ export const FORCE_COMPACTION_BUFFER_PERCENT = 5; +/** + * Headroom (percentage points of the routed model's context window) reserved + * for the pending turn when deciding whether a skill-routed send must compact + * first. Routed sends deliberately ignore the user's compaction threshold — + * a cheap skill invocation must not force an unrequested workspace-wide + * compaction of a history far under the workspace model's limit — but the + * recorded usage excludes the new message, attachments, and skill snapshot, + * so requiring a full 100% would let a near-limit history overrun the routed + * model at request startup instead of compacting. + */ +export const ROUTED_SEND_COMPACTION_HEADROOM_PERCENT = 10; + /** * Duration (ms) to show "copied" feedback after copying to clipboard */ diff --git a/src/common/orpc/schemas.ts b/src/common/orpc/schemas.ts index 17e054d6d68..c7344ed32db 100644 --- a/src/common/orpc/schemas.ts +++ b/src/common/orpc/schemas.ts @@ -162,6 +162,7 @@ export { } from "./schemas/agentDefinition"; export { + SendMessageAcceptedSchema, SendMessageErrorSchema, StreamErrorTypeSchema, NameGenerationErrorSchema, diff --git a/src/common/orpc/schemas/api.ts b/src/common/orpc/schemas/api.ts index 8a87a56e0f5..e88fc934c7e 100644 --- a/src/common/orpc/schemas/api.ts +++ b/src/common/orpc/schemas/api.ts @@ -14,6 +14,7 @@ import { CUSTOM_PROVIDER_TYPES } from "@/common/utils/providers/customProviders" import { ChatStatsSchema, SessionUsageFileSchema } from "./chatStats"; import { AdditionalSystemContextSchema, WorkspaceInstructionsSchema } from "./instructions"; import { + SendMessageAcceptedSchema, NameGenerationErrorSchema, ProjectRemoveErrorSchema, SendMessageErrorSchema, @@ -1578,7 +1579,7 @@ export const workspace = { fileParts: z.array(FilePartSchema).optional(), }), }), - output: ResultSchema(z.object({}), SendMessageErrorSchema), + output: ResultSchema(SendMessageAcceptedSchema, SendMessageErrorSchema), }, answerAskUserQuestion: { input: z @@ -2531,6 +2532,8 @@ export const config = { routeOverrides: z.record(z.string(), z.string()).optional(), minThinkingLevelByModel: z.record(z.string(), ThinkingLevelSchema).optional(), modelFallbacks: ModelFallbacksSchema.optional(), + modelClasses: z.record(z.string(), z.string()).optional(), + skillModelClasses: z.record(z.string(), z.string()).optional(), defaultModel: z.string().optional(), advisorModelString: AdvisorModelStringSchema, advisorThinkingLevel: AdvisorThinkingLevelSchema, @@ -2612,6 +2615,21 @@ export const config = { }), output: z.void(), }, + updateModelClass: { + input: z.object({ + // One class per call, merged inside the backend's config transaction: + // a full-map replacement composed client-side would race a concurrent + // Settings consumer and silently delete its just-written entry. + // Canonical slots are large/medium/small; hand-edited custom names are + // preserved by construction (the backend only touches this key). + className: z.string().min(1), + // One-shot syntax ("haiku+0"), stored verbatim — unparseable values + // fail loudly at send time and are flagged inline by the editor. + // null clears the class. + model: z.string().nullable(), + }), + output: z.void(), + }, updateCoderPrefs: { input: z .object({ diff --git a/src/common/orpc/schemas/errors.ts b/src/common/orpc/schemas/errors.ts index 602a941f206..b9aec7235a6 100644 --- a/src/common/orpc/schemas/errors.ts +++ b/src/common/orpc/schemas/errors.ts @@ -1,4 +1,25 @@ import { z } from "zod"; +import { ThinkingLevelSchema } from "../../types/thinking"; + +/** + * Success payload of an accepted (non-queued) send — the single source for + * both the oRPC output schema (workspace.sendMessage) and the backend's + * SendMessageAccepted type, so the wire shape and the compile-time shape can + * never drift. + */ +export const SendMessageAcceptedSchema = z.object({ + // Class model applied by skill routing — lets the frontend attribute send + // telemetry to the model that actually streams. Absent when no routing + // occurred or the send was queued for later dispatch. + routedModel: z.string().optional(), + // Thinking level routing replaced (class suffix or re-resolved numeric + // one-shot); absent when the ambient thinking level applies. + routedThinkingLevel: ThinkingLevelSchema.optional(), + // True when the send was QUEUED behind a busy turn: class routing has not + // resolved yet, so the absence of routedModel means "unknown", not + // "unrouted" — telemetry must not attribute the ambient model to it. + queued: z.boolean().optional(), +}); /** * Discriminated union for all possible sendMessage errors. diff --git a/src/common/orpc/schemas/stream.ts b/src/common/orpc/schemas/stream.ts index 6108159fcbe..3b71eb097ec 100644 --- a/src/common/orpc/schemas/stream.ts +++ b/src/common/orpc/schemas/stream.ts @@ -828,6 +828,22 @@ export const SendMessageOptionsSchema = z.object({ * When true, skip persisting AI settings (e.g., for one-shot or compaction sends). */ skipAiSettingsPersistence: z.boolean().optional(), + /** + * Explicit model override marker: suppresses per-skill model-class routing + * for this send. Deliberately decoupled from skipAiSettingsPersistence — + * several internal senders (heartbeats, compaction, continuations) set that + * flag for persistence reasons only and must not silently lose routing. + */ + skipSkillModelRouting: z.boolean().optional(), + /** + * Raw numeric one-shot thinking index ("/+2 /skill"). Numeric thinking is + * model-relative (0 = the model's lowest allowed level), and the frontend + * resolves `thinkingLevel` against the workspace model before routing is + * known — so when skill class routing swaps the model, the backend + * re-resolves this raw index against the routed model's ladder instead of + * inheriting a level indexed on the wrong model. + */ + oneShotThinkingIndex: z.number().int().min(0).optional(), experiments: ExperimentsSchema.optional(), /** * When true, workspace-specific agent definitions are disabled. diff --git a/src/common/types/errors.ts b/src/common/types/errors.ts index cd449383db3..3ed6027cdb9 100644 --- a/src/common/types/errors.ts +++ b/src/common/types/errors.ts @@ -6,6 +6,7 @@ import type z from "zod"; import type { NameGenerationErrorSchema, + SendMessageAcceptedSchema, SendMessageErrorSchema, StreamErrorTypeSchema, } from "../orpc/schemas"; @@ -20,6 +21,18 @@ import type { */ export type SendMessageError = z.infer; +/** + * Success payload of an accepted (non-queued) send. `routedModel` is the class + * model applied by skill routing — exposed so the frontend can attribute + * send telemetry to the model that actually streams; undefined when no + * routing occurred or the send was queued for later dispatch. + * `routedThinkingLevel` is the effective thinking level the routed stream + * runs at — class suffix, re-resolved numeric one-shot, or a named/ambient + * level riding through — after per-model floor enforcement; absent only when + * the send carries no thinking level at all. + */ +export type SendMessageAccepted = z.infer; + /** * Stream error types - categorizes errors during AI streaming * Used across backend (StreamManager) and frontend (StreamErrorMessage) diff --git a/src/common/types/message.ts b/src/common/types/message.ts index 128d63f0b08..a7e009b8e98 100644 --- a/src/common/types/message.ts +++ b/src/common/types/message.ts @@ -48,6 +48,29 @@ export interface UserMessageContent { export interface CompactionFollowUpInput extends UserMessageContent { /** Message metadata to apply to the queued follow-up user message (e.g., preserve /skill display) */ muxMetadata?: MuxMessageMetadata; + /** + * Explicit model override reconstructed from the original send (a composed + * "/model /skill" one-shot). Without it, a compact-and-retry rebuild would + * re-dispatch the skill through class routing even though the user + * explicitly overrode the model for that invocation. + */ + model?: string; + /** Rides with `model`: an explicit one-shot must keep bypassing class routing on re-dispatch. */ + skipSkillModelRouting?: boolean; + /** + * Explicit one-shot thinking reconstructed from the original send + * ("/haiku+0 /skill", "/+high /skill"). Resolved against `model` when + * present, else against the workspace model at rebuild time. + */ + thinkingLevel?: ThinkingLevel; + /** + * Raw numeric one-shot thinking index ("/+2 /skill"). Model-relative: the + * re-dispatched send re-resolves it against the routed class model when + * skill routing applies (see SendMessageOptions.oneShotThinkingIndex). + */ + oneShotThinkingIndex?: number; + /** One-shot overrides carried through compaction must not persist as new workspace defaults. */ + skipAiSettingsPersistence?: boolean; } /** @@ -67,6 +90,8 @@ type PreservedSendOptions = Pick< | "strictAgentResolution" | "allowAgentSetGoal" | "skipAiSettingsPersistence" + | "skipSkillModelRouting" + | "oneShotThinkingIndex" >; /** @@ -107,6 +132,16 @@ export function pickPreservedSendOptions(options: SendMessageOptions): Preserved ...(options.skipAiSettingsPersistence !== undefined ? { skipAiSettingsPersistence: options.skipAiSettingsPersistence } : {}), + // A one-shot's routing bypass and raw thinking index must survive into a + // compaction follow-up, or the re-dispatch re-routes/re-ladders the send. + // Same omit-if-unset rule as above: an explicit undefined would clobber + // the original preserved value when spread over a persisted follow-up. + ...(options.skipSkillModelRouting !== undefined + ? { skipSkillModelRouting: options.skipSkillModelRouting } + : {}), + ...(options.oneShotThinkingIndex !== undefined + ? { oneShotThinkingIndex: options.oneShotThinkingIndex } + : {}), }; } @@ -146,8 +181,47 @@ export type StartupRetrySendOptions = Pick< * goal-scoped compaction follow-ups (Codex P2 PRRT_kwDOPxxmWM6cIv2E). */ goalId?: string; + /** + * Pre-skill-routing compaction context for routed turns (durable subset), + * restored on startup retry so the routed compaction policy survives a + * relaunch. One level deep by construction — the nested pick never + * receives a compactionBaseOptions of its own. + */ + compactionBaseOptions?: Omit; + /** + * True when the routed turn carried project-scope skill content at + * acceptance: every resumed dispatch (same-session auto-retry, startup + * recovery) must re-verify Project Trust before replaying the persisted + * routed options — the resume path bypasses the send gates. + */ + routedProjectConsent?: boolean; }; +/** + * The SendMessageOptions keys pickStartupRetrySendOptions persists (its + * muxMetadata narrowing and retry-state extras aside). Restore paths that + * re-admit persisted JSON parse AGAINST this whitelist — the general send + * schema is broader, and a corrupted row smuggling a non-durable field like + * editMessageId into a restored send would flip behavioral switches (the + * edit/truncation path) instead of replaying configuration. Keep in sync with + * the function below. + */ +export const STARTUP_RETRY_DURABLE_SEND_OPTION_KEYS = [ + "model", + "agentId", + "thinkingLevel", + "reasoningMode", + "toolPolicy", + "additionalSystemInstructions", + "maxOutputTokens", + "providerOptions", + "experiments", + "disableWorkspaceAgents", + "strictAgentResolution", + "allowAgentSetGoal", + "muxMetadata", +] as const satisfies ReadonlyArray; + /** * Snapshot retry-relevant send options so startup recovery can resume interrupted * turns with the same request configuration (model/provider options/system hints). @@ -155,7 +229,9 @@ export type StartupRetrySendOptions = Pick< export function pickStartupRetrySendOptions( options: SendMessageOptions, agentInitiated?: boolean, - goalKind?: GoalSyntheticMessageKind + goalKind?: GoalSyntheticMessageKind, + compactionBaseOptions?: SendMessageOptions, + routedProjectConsent?: boolean ): StartupRetrySendOptions { const typedMuxMetadata = options.muxMetadata as MuxMessageMetadata | undefined; const workspaceTurnMuxMetadata = @@ -179,6 +255,17 @@ export function pickStartupRetrySendOptions( ...(workspaceTurnMuxMetadata != null ? { muxMetadata: workspaceTurnMuxMetadata } : {}), ...(agentInitiated === true ? { agentInitiated: true } : {}), ...(goalKind != null ? { goalKind } : {}), + // Routed turns persist their pre-routing compaction context (durable + // fields only) so a post-relaunch retry keeps the routed compaction + // policy instead of force-compacting at the workspace threshold against + // the routed window. Absent on non-routed turns and rows written by + // older versions — both fall back to today's behavior. + ...(compactionBaseOptions != null + ? { + compactionBaseOptions: pickStartupRetrySendOptions(compactionBaseOptions), + } + : {}), + ...(routedProjectConsent === true ? { routedProjectConsent: true } : {}), }; } @@ -374,6 +461,17 @@ export function buildMcpPromptUserText( return argumentText ? `${base}: ${argumentText}` : base; } +/** + * Model-facing text for a skill invocation. The transcript displays the raw + * command ("/done finish") from metadata, but the payload the model streams + * on is this rewritten form — every dispatcher that rebuilds a skill turn + * (send, compact-and-retry follow-up) must produce the same text, or the + * retried turn is semantically different from the invocation that failed. + */ +export function buildSkillInvocationUserText(skillName: string, argumentText: string): string { + return argumentText ? `Using skill ${skillName}: ${argumentText}` : `Use skill ${skillName}`; +} + export function isMcpPromptReference(value: unknown): value is MCPPromptReference { if (value === null || typeof value !== "object") return false; const ref = value as Partial; @@ -436,6 +534,19 @@ function isMcpPromptSnapshotBaseShape( * and the user-row append: a snapshot survives only when its invoking user row * exists and still references the same prompt. */ +/** + * Drop user rows persisted by pre-stream gate rejections from provider + * requests. The transcript keeps showing them (they document what was + * rejected and why), but the send never streamed: replaying the text would + * duplicate the prompt once the user retries, and a PDF rejected for model + * incompatibility would re-fail every later request it rides along in. + */ +export function filterPreStreamRejectedRows(messages: MuxMessage[]): MuxMessage[] { + return messages.filter( + (message) => !(message.role === "user" && message.metadata?.preStreamRejected === true) + ); +} + export function filterOrphanedMcpPromptSnapshots(messages: MuxMessage[]): MuxMessage[] { // Drop only genuine expansion rows: the reserved ID prefix marks one even // when corruption removed its metadata, and synthetic rows with a valid @@ -969,6 +1080,14 @@ export interface MuxMetadata { disableWorkspaceAgents?: boolean; // Whether workspace-local agent files were disabled for this user turn /** Snapshot of send options used for this user turn (for startup retry recovery). */ retrySendOptions?: StartupRetrySendOptions; + /** + * Stamped on user rows persisted by pre-stream gate rejections + * (preserveRejectedManualSend): the send never streamed and must never be + * replayed by startup recovery — atomic with the row, unlike the + * preference-file abandon marker which can be lost to a crash between the + * two writes. + */ + preStreamRejected?: boolean; agentId?: string; // Agent id active when this message was sent (assistant messages only) cmuxMetadata?: MuxMessageMetadata; // Command metadata persisted for legacy message formats muxMetadata?: MuxMessageMetadata; // Command metadata used by both frontend and backend message flows diff --git a/src/common/types/project.ts b/src/common/types/project.ts index f3011a9a092..28b967ae9ba 100644 --- a/src/common/types/project.ts +++ b/src/common/types/project.ts @@ -114,6 +114,19 @@ export interface ProjectsConfig { */ modelFallbacks?: ModelFallbacks; + /** + * Named model classes (e.g. large/medium/small → a model alias or + * "provider:model" id with an optional "+thinking" suffix, one-shot syntax). + * Indirection layer so bindings survive model churn; consumed by per-skill + * model routing. + */ + modelClasses?: Record; + /** + * Per-skill routing table (skill name → class name in modelClasses). Wins + * over a skill's own frontmatter metadata "model-class" binding. + */ + skillModelClasses?: Record; + /** * Default model used for new workspaces (shared via ~/.xum/config.json). * Mirrors the browser localStorage cache (DEFAULT_MODEL_KEY). diff --git a/src/common/utils/ai/modelAvailability.test.ts b/src/common/utils/ai/modelAvailability.test.ts new file mode 100644 index 00000000000..1d0eaa88b1b --- /dev/null +++ b/src/common/utils/ai/modelAvailability.test.ts @@ -0,0 +1,231 @@ +import { describe, expect, test } from "bun:test"; + +import type { ProvidersConfigMap } from "@/common/orpc/types"; +import { isModelServableWithProvidersConfig } from "./modelAvailability"; + +const MODEL = "anthropic:claude-haiku-4-5"; + +function providers(entry: { isConfigured: boolean; isEnabled?: boolean }): ProvidersConfigMap { + return { anthropic: entry } as unknown as ProvidersConfigMap; +} + +describe("isModelServableWithProvidersConfig", () => { + test("serves a model whose direct provider is configured", () => { + expect( + isModelServableWithProvidersConfig({ + canonicalModel: MODEL, + routePriority: ["direct"], + providersConfig: providers({ isConfigured: true }), + }) + ).toBe(true); + }); + + test("rejects a model whose provider is not configured", () => { + expect( + isModelServableWithProvidersConfig({ + canonicalModel: MODEL, + routePriority: ["direct"], + providersConfig: providers({ isConfigured: false }), + }) + ).toBe(false); + }); + + test("a disabled provider does not count as configured", () => { + expect( + isModelServableWithProvidersConfig({ + canonicalModel: MODEL, + routePriority: ["direct"], + providersConfig: providers({ isConfigured: true, isEnabled: false }), + }) + ).toBe(false); + }); + + test("the direct fallback serves a configured provider even outside the priority list", () => { + // resolveRoute exhausts the priority list and then falls back to direct, + // so an ordinary send succeeds whenever the direct provider is + // credentialed — availability must agree, or the class gate would reject + // a model the same send-path serves. + expect( + isModelServableWithProvidersConfig({ + canonicalModel: MODEL, + routePriority: [], + providersConfig: providers({ isConfigured: true }), + }) + ).toBe(true); + }); + + test("the direct fallback still requires the provider to be configured", () => { + expect( + isModelServableWithProvidersConfig({ + canonicalModel: MODEL, + routePriority: [], + providersConfig: providers({ isConfigured: false }), + }) + ).toBe(false); + }); + + test("route priority defaults to direct when omitted", () => { + expect( + isModelServableWithProvidersConfig({ + canonicalModel: MODEL, + providersConfig: providers({ isConfigured: true }), + }) + ).toBe(true); + }); + + describe("coder gateway identities", () => { + test("a known-but-unmappable instance never falls back through its name", () => { + // The instance is NAMED "anthropic" but typed openai-compat: the + // factory keeps the raw coder seed and fails the send when Coder is + // unavailable, so availability must not pass it through configured + // direct Anthropic via the name-parsed canonical. + expect( + isModelServableWithProvidersConfig({ + canonicalModel: "coder:anthropic/claude-haiku-4-5", + routePriority: ["direct"], + providersConfig: { + coder: { + isConfigured: false, + discoveredProviders: [{ name: "anthropic", type: "openai-compat" }], + }, + anthropic: { isConfigured: true }, + } as unknown as ProvidersConfigMap, + }) + ).toBe(false); + }); + + test("the coder gateway itself serves its models when configured", () => { + expect( + isModelServableWithProvidersConfig({ + canonicalModel: "coder:anthropic/claude-haiku-4-5", + routePriority: ["direct"], + providersConfig: { + coder: { + isConfigured: true, + discoveredProviders: [{ name: "anthropic", type: "openai-compat" }], + }, + } as unknown as ProvidersConfigMap, + }) + ).toBe(true); + }); + + test("metadata identity beats the name-parsed canonical for the fallback", () => { + // Instance NAMED "openai" but TYPED anthropic: the fallback route is + // anthropic:, so configured OpenAI alone must not claim it… + const crossTyped = { + coder: { + isConfigured: false, + discoveredProviders: [{ name: "openai", type: "anthropic" }], + }, + openai: { isConfigured: true, apiKeySet: true }, + }; + expect( + isModelServableWithProvidersConfig({ + canonicalModel: "coder:openai/claude-haiku-4-5", + routePriority: ["direct"], + providersConfig: crossTyped as unknown as ProvidersConfigMap, + }) + ).toBe(false); + // …while configured Anthropic does. + expect( + isModelServableWithProvidersConfig({ + canonicalModel: "coder:openai/claude-haiku-4-5", + routePriority: ["direct"], + providersConfig: { + ...crossTyped, + anthropic: { isConfigured: true }, + } as unknown as ProvidersConfigMap, + }) + ).toBe(true); + }); + + test("an unknown instance falls back generically like the factory's seed", () => { + // No discovered/additional providers: the factory seeds the + // name-parsed canonical, so availability mirrors that leniency. + expect( + isModelServableWithProvidersConfig({ + canonicalModel: "coder:anthropic/claude-haiku-4-5", + routePriority: ["direct"], + providersConfig: { + coder: { isConfigured: false }, + anthropic: { isConfigured: true }, + } as unknown as ProvidersConfigMap, + }) + ).toBe(true); + }); + }); + + describe("direct OpenAI credential gating", () => { + function openaiProviders(entry: Record): ProvidersConfigMap { + return { openai: { isConfigured: true, ...entry } } as unknown as ProvidersConfigMap; + } + + test("an OAuth-only config cannot serve an OAuth-ineligible model directly", () => { + // gpt-5.5-pro is not in the Codex OAuth allowed set: with no API key the + // factory would reject the direct route (api_key_not_found), so + // availability must not claim it. + expect( + isModelServableWithProvidersConfig({ + canonicalModel: "openai:gpt-5.5-pro", + routePriority: ["direct"], + providersConfig: openaiProviders({ codexOauthSet: true }), + }) + ).toBe(false); + }); + + test("an OAuth-only config serves OAuth-allowed models directly", () => { + expect( + isModelServableWithProvidersConfig({ + canonicalModel: "openai:gpt-5.5", + routePriority: ["direct"], + providersConfig: openaiProviders({ codexOauthSet: true }), + }) + ).toBe(true); + }); + + test("an OAuth-only config pinned to chatCompletions cannot serve OAuth-allowed models directly", () => { + // Codex OAuth only speaks the Responses endpoint: the factory rejects + // chatCompletions + OAuth-only with api_key_not_found, so direct must + // not claim the model (a configured gateway should win instead). + expect( + isModelServableWithProvidersConfig({ + canonicalModel: "openai:gpt-5.5", + routePriority: ["direct"], + providersConfig: openaiProviders({ codexOauthSet: true, wireFormat: "chatCompletions" }), + }) + ).toBe(false); + }); + + test("an API key serves OAuth-ineligible models directly", () => { + expect( + isModelServableWithProvidersConfig({ + canonicalModel: "openai:gpt-5.5-pro", + routePriority: ["direct"], + providersConfig: openaiProviders({ apiKeySet: true }), + }) + ).toBe(true); + }); + + test("a custom openai-compatible provider shadowing the openai id is exempt from OAuth gating", () => { + // Keyless custom endpoints authenticate on their own terms; built-in + // OpenAI credential rules must not mark their models unavailable. + expect( + isModelServableWithProvidersConfig({ + canonicalModel: "openai:gpt-5.5-pro", + routePriority: ["direct"], + providersConfig: openaiProviders({ providerType: "openai-compatible" }), + }) + ).toBe(true); + }); + + test("an API key serves OAuth-preferred models too (factory falls back to the key)", () => { + expect( + isModelServableWithProvidersConfig({ + canonicalModel: "openai:gpt-5.3-codex-spark", + routePriority: ["direct"], + providersConfig: openaiProviders({ apiKeySet: true }), + }) + ).toBe(true); + }); + }); +}); diff --git a/src/common/utils/ai/modelAvailability.ts b/src/common/utils/ai/modelAvailability.ts new file mode 100644 index 00000000000..fa0c9134161 --- /dev/null +++ b/src/common/utils/ai/modelAvailability.ts @@ -0,0 +1,166 @@ +import type { ProvidersConfigMap } from "@/common/orpc/types"; +import { isModelAvailable, resolveRoute } from "@/common/routing"; +import { isGatewayModelAccessibleFromAuthoritativeCatalog } from "@/common/utils/providers/gatewayModelCatalog"; +import { canDirectOpenAIServeModel } from "@/common/utils/providers/codexOauthRouting"; +import { normalizeToCanonical } from "@/common/utils/ai/models"; +import { resolveCoderGatewayMetadataModel } from "@/common/utils/providers/coderGatewayMetadata"; +import { isCustomProviderConfig } from "@/common/utils/providers/customProviders"; +import { resolveCoderWireCanonicalModel } from "@/common/constants/coderOAuth"; +import { PROVIDER_REGISTRY } from "@/common/constants/providers"; + +/** + * Provider-configured predicate shared by the routing UI (useRouting) and the + * send-path availability check below. One definition, two consumers — the + * Settings picker and the skill-routing verdict must never disagree. + */ +export function isRouteProviderConfigured( + providersConfig: ProvidersConfigMap, + provider: string +): boolean { + return ( + providersConfig[provider]?.isConfigured === true && + providersConfig[provider]?.isEnabled !== false + ); +} + +/** Gateway-catalog accessibility predicate; see isRouteProviderConfigured. */ +export function isRouteGatewayModelAccessible( + providersConfig: ProvidersConfigMap, + gateway: string, + modelId: string +): boolean { + return isGatewayModelAccessibleFromAuthoritativeCatalog( + gateway, + modelId, + providersConfig[gateway]?.models, + providersConfig[gateway]?.discoveredModels, + providersConfig[gateway]?.removedModels + ); +} + +/** + * Can the current routing state actually serve this model? + * + * Wraps the routing layer's isModelAvailable with the same provider + * predicates the Settings UI uses (useRouting): a provider counts as + * configured when `isConfigured` is set and it is not disabled, and gateway + * accessibility consults the authoritative model catalog. Route priority and + * per-model overrides are honored, so a gateway that is configured but not in + * the priority list does not count — matching what a send would really do. + * + * Callers that cannot obtain a ProvidersConfigMap (degraded state, minimal + * test mocks) must skip the check rather than pass an empty map: + * "cannot determine" is not "unavailable". + * + * Known one-directional gap: enforced-policy model gating (policyService + * isModelAllowed, applied inside the node-side gateway checker) is not + * consulted here, so this can over-report availability for policy-blocked + * gateway models — the send then fails with the provider's own error rather + * than the actionable class message. It can never spuriously block. + */ +export function isModelServableWithProvidersConfig(args: { + canonicalModel: string; + routePriority?: string[]; + routeOverrides?: Record; + providersConfig: ProvidersConfigMap; +}): boolean { + const providersConfig = args.providersConfig; + // The OAuth gate judges the CANONICAL identity, matching createModel's + // normalization: an explicit gateway value (openrouter:openai/gpt-5.5) + // would otherwise read as "not an openai model" and wrongly fail the + // direct-route check the factory itself would pass. + const canonicalForDirect = normalizeToCanonical(args.canonicalModel); + const isConfigured = (provider: string): boolean => { + if (!isRouteProviderConfigured(providersConfig, provider)) { + return false; + } + // OpenAI's isConfigured can mean Codex-OAuth-only credentials, which + // serve only the OAuth-allowed model set — a direct route the factory + // would reject (api_key_not_found) must not win over a later gateway or + // suppress the actionable class error. + if (provider === "openai") { + return canDirectOpenAIServeModel(canonicalForDirect, providersConfig); + } + return true; + }; + + // Coder values are judged BEFORE any generic routing: the instance segment + // of coder:/ is a deployment name, not a vendor prefix, + // but generic canonicalization parses it as one — a cross-typed instance + // ({name: "anthropic", type: "openai-compat"}) would read as a direct + // anthropic: and pass through configured Anthropic even though the + // factory deliberately keeps the raw gateway-scoped seed for it and fails + // the send with a Coder availability error. A custom provider shadowing + // the coder id is exempt: its model IDs are their own identity and route + // generically. Mirrors the factory's routeSeedModelString derivation. + if (args.canonicalModel.startsWith("coder:") && !isCustomProviderConfig(providersConfig.coder)) { + const coderGatewayModelId = args.canonicalModel.slice("coder:".length); + // The coder gateway itself is the preferred explicit route (matching + // findActiveRouteContext's explicit-gateway-first candidate). + if ( + isConfigured("coder") && + isRouteGatewayModelAccessible(providersConfig, "coder", coderGatewayModelId) + ) { + return true; + } + // Fallback identity comes from the instance METADATA the factory itself + // uses (a google-typed coder:vertex/gemini-x falls back to + // google:gemini-x, not to the wire protocol's openai identity), gated on + // the prefix being a REGISTERED provider — an unknown vendor + // (coder:ai-gateway/acme/foo → acme:foo) is not directly routable even + // if a custom provider happens to share the name. + const derived = resolveCoderGatewayMetadataModel(args.canonicalModel, providersConfig); + if ( + derived != null && + Object.hasOwn(PROVIDER_REGISTRY, derived.slice(0, derived.indexOf(":"))) + ) { + return isModelServableWithProvidersConfig({ ...args, canonicalModel: derived }); + } + const coderMetadata = providersConfig.coder as + | { discoveredProviders?: unknown; additionalProviders?: unknown } + | undefined; + if (resolveCoderWireCanonicalModel(coderGatewayModelId, coderMetadata) != null) { + // KNOWN but unmappable instance (openai-compat fronts arbitrary + // upstreams; vendor-less IDs carry no catalog identity): the factory + // retains the raw Coder seed and rejects the send when the gateway + // cannot serve it — no generic fallback exists. + return false; + } + // Unknown instance: the factory seeds generic canonicalization — fall + // through to the same checks below. + } + + if ( + isModelAvailable( + args.canonicalModel, + args.routePriority ?? ["direct"], + args.routeOverrides ?? {}, + isConfigured, + (gateway, modelId) => isRouteGatewayModelAccessible(providersConfig, gateway, modelId) + ) + ) { + return true; + } + // Mirror resolveRoute's FINAL fallback: with the priority list exhausted + // (including a routePriority that omits "direct" entirely), resolution + // still lands on the direct provider — an ordinary send succeeds whenever + // that provider is credentialed, so the class gate must not reject a model + // the same send-path would serve. + const fallback = resolveRoute( + args.canonicalModel, + args.routePriority ?? ["direct"], + args.routeOverrides ?? {}, + isConfigured, + (gateway, modelId) => isRouteGatewayModelAccessible(providersConfig, gateway, modelId) + ); + // Configuration alone is not enough for catalog-gated routes: an + // unmappable coder:/ (tombstoned or absent from the + // discovered AI Bridge catalog) is rejected by the factory with + // model_not_available even though the instance is configured. The + // accessibility predicate fails open for providers without a catalog, so + // applying it unconditionally only ever removes false positives. + return ( + isConfigured(fallback.routeProvider) && + isRouteGatewayModelAccessible(providersConfig, fallback.routeProvider, fallback.routeModelId) + ); +} diff --git a/src/common/utils/ai/skillModelClasses.test.ts b/src/common/utils/ai/skillModelClasses.test.ts new file mode 100644 index 00000000000..c0dedf13b56 --- /dev/null +++ b/src/common/utils/ai/skillModelClasses.test.ts @@ -0,0 +1,204 @@ +import { describe, expect, test } from "bun:test"; + +import { KNOWN_MODELS } from "@/common/constants/knownModels"; +import { + buildModelClassValue, + parseModelClassValue, + resolveSkillModelClassBinding, + splitModelClassValue, +} from "./skillModelClasses"; + +describe("parseModelClassValue", () => { + test("resolves a bare alias without a thinking level", () => { + expect(parseModelClassValue("haiku")).toEqual({ model: KNOWN_MODELS.HAIKU.id }); + }); + + test("resolves alias + numeric thinking (deferred as an index)", () => { + expect(parseModelClassValue("haiku+0")).toEqual({ + model: KNOWN_MODELS.HAIKU.id, + thinkingLevel: 0, + }); + }); + + test("resolves alias + named thinking", () => { + expect(parseModelClassValue("sonnet+high")).toEqual({ + model: KNOWN_MODELS.SONNET.id, + thinkingLevel: "high", + }); + }); + + test("accepts full provider:model ids (unlike the composer one-shot parser)", () => { + expect(parseModelClassValue("anthropic:claude-fable-5+max")).toEqual({ + model: KNOWN_MODELS.FABLE.id, + thinkingLevel: "max", + }); + }); + + test("rejects unknown model input", () => { + expect(parseModelClassValue("not-a-model")).toBeNull(); + expect(parseModelClassValue("")).toBeNull(); + }); + + test("rejects an invalid thinking suffix instead of ignoring it", () => { + expect(parseModelClassValue("haiku+bogus")).toBeNull(); + expect(parseModelClassValue("haiku+")).toBeNull(); + }); +}); + +describe("splitModelClassValue / buildModelClassValue", () => { + test("round-trips a raw thinking suffix so numeric levels survive model changes", () => { + // "+0" is model-relative user intent ("lowest"): swapping the model in an + // editor must not concretize it to the old model's floor. + const { thinkingSuffix } = splitModelClassValue("haiku+0"); + expect(thinkingSuffix).toBe("0"); + expect(buildModelClassValue(KNOWN_MODELS.SONNET.id, thinkingSuffix)).toBe( + `${KNOWN_MODELS.SONNET.id}+0` + ); + }); + + test("handles suffix-less values", () => { + expect(splitModelClassValue("sonnet")).toEqual({ modelPart: "sonnet", thinkingSuffix: null }); + expect(buildModelClassValue("sonnet", null)).toBe("sonnet"); + }); +}); + +describe("resolveSkillModelClassBinding", () => { + const modelClasses = { + small: "haiku+0", + large: "anthropic:claude-fable-5+max", + }; + + test("binds via frontmatter metadata and resolves numeric thinking to a concrete level", () => { + const binding = resolveSkillModelClassBinding({ + skillName: "done", + frontmatterMetadata: { "model-class": "small" }, + modelClasses, + }); + // Haiku's lowest allowed thinking level is "off": index 0 must resolve + // model-relatively, not to the literal level "0". + expect(binding).toEqual({ + status: "resolved", + className: "small", + model: KNOWN_MODELS.HAIKU.id, + thinkingLevel: "off", + }); + }); + + test("config routing table wins over frontmatter metadata", () => { + const binding = resolveSkillModelClassBinding({ + skillName: "done", + frontmatterMetadata: { "model-class": "small" }, + modelClasses, + skillModelClasses: { done: "large" }, + }); + expect(binding).toMatchObject({ + status: "resolved", + model: KNOWN_MODELS.FABLE.id, + thinkingLevel: "max", + }); + }); + + test("table entries for other skills do not shadow the metadata binding", () => { + const binding = resolveSkillModelClassBinding({ + skillName: "done", + frontmatterMetadata: { "model-class": "small" }, + modelClasses, + skillModelClasses: { review: "large" }, + }); + expect(binding).toMatchObject({ status: "resolved", model: KNOWN_MODELS.HAIKU.id }); + }); + + test("frontmatter bindings to an undefined class stay inert (skills the user does not own)", () => { + expect( + resolveSkillModelClassBinding({ + skillName: "done", + frontmatterMetadata: { "model-class": "tiny" }, + modelClasses, + }) + ).toEqual({ status: "unbound" }); + }); + + test("a dangling table binding reports unknown-class (user's own routing intent)", () => { + expect( + resolveSkillModelClassBinding({ + skillName: "done", + modelClasses, + skillModelClasses: { done: "tiny" }, + }) + ).toEqual({ status: "unknown-class", className: "tiny" }); + }); + + test("reports an invalid class value instead of swallowing it", () => { + expect( + resolveSkillModelClassBinding({ + skillName: "done", + frontmatterMetadata: { "model-class": "small" }, + modelClasses: { small: "not-a-model" }, + }) + ).toEqual({ status: "invalid-value", className: "small", value: "not-a-model" }); + }); + + test("skills without any binding are unbound", () => { + expect(resolveSkillModelClassBinding({ skillName: "done", modelClasses })).toEqual({ + status: "unbound", + }); + }); + + test("frontmatter bindings are inert until the user configures model classes", () => { + // A skill shipping `metadata: model-class` must not error for users who + // never opted into model classes. + expect( + resolveSkillModelClassBinding({ + skillName: "done", + frontmatterMetadata: { "model-class": "small" }, + }) + ).toEqual({ status: "unbound" }); + }); + + test("a config-table binding is explicit intent and errors even without a class map", () => { + expect( + resolveSkillModelClassBinding({ + skillName: "done", + skillModelClasses: { done: "small" }, + }) + ).toEqual({ status: "unknown-class", className: "small" }); + }); + + test("a class without a thinking suffix overrides only the model", () => { + const binding = resolveSkillModelClassBinding({ + skillName: "done", + frontmatterMetadata: { "model-class": "medium" }, + modelClasses: { medium: "sonnet" }, + }); + expect(binding).toEqual({ + status: "resolved", + className: "medium", + model: KNOWN_MODELS.SONNET.id, + }); + }); +}); + +describe("splitModelClassValue / plus-bearing model ids", () => { + test("keeps a plus-bearing custom model id intact when the tail is not a thinking token", () => { + expect(splitModelClassValue("proxy:model+v2")).toEqual({ + modelPart: "proxy:model+v2", + thinkingSuffix: null, + }); + expect(parseModelClassValue("proxy:model+v2")).toEqual({ model: "proxy:model+v2" }); + }); + + test("splits on the LAST plus when the tail is a thinking token", () => { + expect(splitModelClassValue("proxy:model+v2+high")).toEqual({ + modelPart: "proxy:model+v2", + thinkingSuffix: "high", + }); + expect(parseModelClassValue("proxy:model+v2+high")).toEqual({ + model: "proxy:model+v2", + thinkingLevel: "high", + }); + }); + + test("still parses ordinary alias+level values", () => { + expect(splitModelClassValue("haiku+0")).toEqual({ modelPart: "haiku", thinkingSuffix: "0" }); + }); +}); diff --git a/src/common/utils/ai/skillModelClasses.ts b/src/common/utils/ai/skillModelClasses.ts new file mode 100644 index 00000000000..7b3cbb4dc02 --- /dev/null +++ b/src/common/utils/ai/skillModelClasses.ts @@ -0,0 +1,201 @@ +import type { ProvidersConfigMap } from "@/common/orpc/types"; +import { + parseThinkingInput, + type ParsedThinkingInput, + type ThinkingLevel, +} from "@/common/types/thinking"; +import { normalizeModelInput } from "@/common/utils/ai/normalizeModelInput"; +import { resolveThinkingInput } from "@/common/utils/thinking/policy"; + +/** + * Skill frontmatter `metadata` key naming the model class a skill prefers to + * run on (e.g. `metadata: { model-class: small }`). The spec-standard metadata + * map is used instead of a new frontmatter field so skills stay portable: + * other agent tools ignore unknown metadata entries, and mux already parses + * and preserves the map. + */ +export const SKILL_MODEL_CLASS_METADATA_KEY = "model-class"; + +export interface ModelClassTarget { + model: string; + /** Deferred: numeric indices are model-relative until resolved. */ + thinkingLevel?: ParsedThinkingInput; +} + +/** + * Parse a `modelClasses` config value. Values use the one-shot override + * syntax: a model alias or full "provider:model" id with an optional + * `+thinking` suffix ("haiku+0", "sonnet+high", "anthropic:claude-fable-5+max"). + * + * Unlike the composer's one-shot key parser (which only accepts known aliases + * so unknown slash commands aren't swallowed), full model ids are accepted + * here — config values are explicit user intent with no shadowing risk. + * + * Returns null when the model or thinking part is invalid (callers fail open). + */ +export function parseModelClassValue(value: string): ModelClassTarget | null { + const trimmed = value.trim(); + if (!trimmed) { + return null; + } + + const { modelPart, thinkingSuffix } = splitModelClassValue(trimmed); + + const normalized = normalizeModelInput(modelPart); + if (normalized.model == null) { + return null; + } + + if (thinkingSuffix == null) { + return { model: normalized.model }; + } + + const thinkingLevel = parseThinkingInput(thinkingSuffix); + if (thinkingLevel == null) { + return null; + } + + return { model: normalized.model, thinkingLevel }; +} + +/** + * Split a class value into its model part and raw thinking suffix. + * + * Custom providers can expose model ids that themselves contain "+" + * (proxy:model+v2), so the suffix is the text after the LAST "+" and only + * when it parses as a thinking token (named level or numeric index) — + * otherwise the whole string is the model. A model id whose final segment + * happens to look like a thinking token (proxy:model+2) resolves toward the + * suffix reading; bind such a model by adding an explicit level + * (proxy:model+2+low) or renaming the exposed id. Editors preserve the raw + * suffix across model changes so a model-relative numeric level + * ("+0" = lowest allowed) keeps its meaning on the new model. + */ +export function splitModelClassValue(value: string): { + modelPart: string; + thinkingSuffix: string | null; +} { + const plusIndex = value.lastIndexOf("+"); + if (plusIndex === -1) { + return { modelPart: value, thinkingSuffix: null }; + } + const candidate = value.slice(plusIndex + 1); + if (parseThinkingInput(candidate) == null) { + // Not a thinking token: the "+" belongs to the model id itself. + return { modelPart: value, thinkingSuffix: null }; + } + return { modelPart: value.slice(0, plusIndex), thinkingSuffix: candidate }; +} + +/** Inverse of splitModelClassValue: build a `model[+thinking]` class value. */ +export function buildModelClassValue(model: string, thinkingSuffix: string | null): string { + return thinkingSuffix ? `${model}+${thinkingSuffix}` : model; +} + +/** + * Class names surfaced as fixed slots in the Settings → Models editor. The + * config map accepts arbitrary names (hand-edited custom classes are preserved + * and keep routing), but skills are portable across machines only when they + * bind to this shared vocabulary. + */ +export const CANONICAL_MODEL_CLASSES = ["large", "medium", "small"] as const; + +export type SkillModelClassBinding = + | { status: "unbound" } + | { status: "unknown-class"; className: string } + | { status: "invalid-value"; className: string; value: string } + | { status: "resolved"; className: string; model: string; thinkingLevel?: ThinkingLevel }; + +/** + * Resolve the model-class binding for a slash-invoked skill. + * + * Class-binding precedence: the config-side routing table + * (`skillModelClasses[skillName]`) wins over the skill's own frontmatter + * `metadata["model-class"]` — local config is explicit user intent and works + * for skills the user does not own. + * + * Broken bindings are reported, not swallowed: a bound skill whose class is + * missing or malformed returns a distinct status so callers can raise an + * actionable error instead of silently running on an unintended (possibly + * expensive) model. One deliberate exception: frontmatter bindings are inert + * while the user has no `modelClasses` configured at all — skills shipping + * `metadata: model-class` must not break for users who never opted into model + * classes. A config-table binding is explicit local intent and always counts. + */ +export function resolveSkillModelClassBinding(args: { + skillName: string; + frontmatterMetadata?: Record; + modelClasses?: Record; + skillModelClasses?: Record; + providersConfig?: ProvidersConfigMap | null; +}): SkillModelClassBinding { + const tableClass = args.skillModelClasses?.[args.skillName]; + const boundViaTable = typeof tableClass === "string" && tableClass.trim().length > 0; + const rawClass = boundViaTable + ? tableClass + : args.frontmatterMetadata?.[SKILL_MODEL_CLASS_METADATA_KEY]; + const className = typeof rawClass === "string" ? rawClass.trim() : ""; + if (!className) { + return { status: "unbound" }; + } + + const modelClasses = args.modelClasses ?? {}; + const classValue = modelClasses[className]; + if (typeof classValue !== "string") { + // A frontmatter binding to a class the user never defined stays inert: + // skills the user does not own must not start failing sends just because + // some other class got configured (partial configuration is the normal + // state of the three-slot editor). A config-table binding is the user's + // own explicit routing intent, so a dangling table entry errors loudly. + // Bindings to a class that EXISTS but is broken (invalid value, + // unavailable model) always error — that is the churn signal this + // feature exists to surface. + if (!boundViaTable) { + return { status: "unbound" }; + } + return { status: "unknown-class", className }; + } + + const target = parseModelClassValue(classValue); + if (!target) { + return { status: "invalid-value", className, value: classValue }; + } + + // Numeric thinking indices are model-relative; resolve to a concrete level + // now. Downstream enforceThinkingPolicy still clamps (min-thinking floors). + const thinkingLevel = + target.thinkingLevel != null + ? resolveThinkingInput(target.thinkingLevel, target.model, args.providersConfig) + : undefined; + + return { + status: "resolved", + className, + model: target.model, + ...(thinkingLevel != null ? { thinkingLevel } : {}), + }; +} + +export type SkillModelClassRoutingProblem = + | { kind: "unknown-class"; skillName: string; className: string } + | { kind: "invalid-value"; skillName: string; className: string; value: string } + | { kind: "model-unavailable"; skillName: string; className: string; model: string }; + +/** + * User-facing message for a broken skill model-class binding. The copy always + * names the fix location and the one-shot bypass so a stale mapping never + * strands the user. + */ +export function describeSkillModelClassRoutingProblem( + problem: SkillModelClassRoutingProblem +): string { + const fixHint = `Update it in Settings → Models → Model Classes, or bypass routing with a one-shot override (e.g. "/sonnet /${problem.skillName}").`; + switch (problem.kind) { + case "unknown-class": + return `Skill "${problem.skillName}" is bound to model class "${problem.className}", but no class with that name is configured. ${fixHint}`; + case "invalid-value": + return `Model class "${problem.className}" (used by skill "${problem.skillName}") has an invalid value "${problem.value}". ${fixHint}`; + case "model-unavailable": + return `Model class "${problem.className}" (used by skill "${problem.skillName}") maps to "${problem.model}", but no configured provider route can serve it. ${fixHint}`; + } +} diff --git a/src/common/utils/providers/codexOauthRouting.ts b/src/common/utils/providers/codexOauthRouting.ts index e961342beec..c52b555a1b2 100644 --- a/src/common/utils/providers/codexOauthRouting.ts +++ b/src/common/utils/providers/codexOauthRouting.ts @@ -10,6 +10,7 @@ */ import { isCodexOauthAllowedModel, isCodexOauthRequiredModel } from "@/common/constants/codexOAuth"; +import { isCustomProviderConfig } from "@/common/utils/providers/customProviders"; import type { ProvidersConfigMap } from "@/common/orpc/types"; function asRecord(value: unknown): Record | null { @@ -66,6 +67,42 @@ export function hasOpenAIApiKey(config: unknown): boolean { * required models always route OAuth; otherwise OAuth wins when no API key is * configured or when `codexOauthDefaultAuth` prefers OAuth over a present key. */ +/** + * Can a DIRECT OpenAI route serve this model with the credentials on hand? + * + * `isConfigured` alone over-reports: a Codex-OAuth-only config serves only the + * OAuth-allowed model set. Mirrors providerModelFactory's credential outcome — + * an API key always attempts (OAuth-required models fall back to the key and + * let the API decide), while stored tokens without a key serve only allowed + * models — so availability checks can't claim a direct route the factory + * would reject with api_key_not_found. + */ +export function canDirectOpenAIServeModel( + model: string, + providersConfig: ProvidersConfigMap | null | undefined +): boolean { + const openAIConfig = providersConfig?.openai; + // A custom provider (any wire type) shadowing the built-in "openai" id is + // direct-only and authenticates against its own endpoint (key optional): + // built-in OpenAI credential rules don't apply to it. + if (isCustomProviderConfig(openAIConfig)) { + return true; + } + if (hasOpenAIApiKey(openAIConfig)) { + return true; + } + // Codex OAuth speaks only the Responses endpoint: a provider pinned to the + // Chat Completions wire format cannot be served by OAuth-only credentials — + // createModel rejects that combination with api_key_not_found — so direct + // routing must not win over a configured gateway for it. + if (asRecord(openAIConfig)?.wireFormat === "chatCompletions") { + return false; + } + return ( + hasCodexOauthTokens(openAIConfig) && isCodexOauthAllowedModel(model, providersConfig ?? null) + ); +} + export function wouldRouteOpenAIThroughCodexOauth( model: string, providersConfig: ProvidersConfigMap | null | undefined diff --git a/src/node/config.modelClasses.test.ts b/src/node/config.modelClasses.test.ts new file mode 100644 index 00000000000..1f6b719e602 --- /dev/null +++ b/src/node/config.modelClasses.test.ts @@ -0,0 +1,132 @@ +import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; + +import { Config } from "@/node/config"; + +describe("Config model classes persistence", () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "mux-model-classes-")); + }); + + afterEach(async () => { + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + it("updateModelClass fails loudly when the write cannot be verified", async () => { + const config = new Config(tempDir); + await config.updateModelClass("small", "haiku+0"); + expect(config.loadConfigOrDefault().modelClasses).toEqual({ small: "haiku+0" }); + + // saveConfig swallows write errors; the strict read-back must reject + // instead of letting the editor publish an unpersisted value as truth. + const original = config.loadConfigOrDefault.bind(config); + let threwOnce = false; + const loadSpy = spyOn(config, "loadConfigOrDefault").mockImplementation(((options?: { + throwOnError?: boolean; + }) => { + if (options?.throwOnError && !threwOnce) { + threwOnce = true; + throw new Error("transient read failure"); + } + return original(options); + }) as typeof config.loadConfigOrDefault); + try { + // eslint-disable-next-line @typescript-eslint/await-thenable -- bun's expect(...).rejects returns a thenable the rule cannot see + await expect(config.updateModelClass("small", "sonnet+1")).rejects.toThrow( + /verify the model class/ + ); + } finally { + loadSpy.mockRestore(); + } + }); + + it("merges per entry: other classes survive edits and deletes", async () => { + const config = new Config(tempDir); + // "my-local-llm" has no provider prefix (unparseable to this build) — + // it must survive verbatim because the backend only touches the edited + // key, never re-serializing the map from a client snapshot. + await config.editConfig((cfg) => ({ + ...cfg, + modelClasses: { small: "haiku+0", tiny: "my-local-llm" }, + })); + + await config.updateModelClass("large", "anthropic:claude-fable-5+max"); + expect(config.loadConfigOrDefault().modelClasses).toEqual({ + small: "haiku+0", + tiny: "my-local-llm", + large: "anthropic:claude-fable-5+max", + }); + + await config.updateModelClass("small", null); + expect(config.loadConfigOrDefault().modelClasses).toEqual({ + tiny: "my-local-llm", + large: "anthropic:claude-fable-5+max", + }); + + // Clearing the last entries drops the map entirely (no empty object + // left behind in config.json). + await config.updateModelClass("tiny", null); + await config.updateModelClass("large", null); + expect(config.loadConfigOrDefault().modelClasses).toBeUndefined(); + }); + + it("concurrent edits of different classes both persist", async () => { + // The review scenario behind the per-entry RPC: two live Settings + // consumers dispatch simultaneously. Full-map replacement snapshots + // would let the later serialized write delete the earlier consumer's + // entry; the transactional merge must keep both. + const config = new Config(tempDir); + await Promise.all([ + config.updateModelClass("small", "haiku+0"), + config.updateModelClass("large", "anthropic:claude-fable-5+max"), + ]); + expect(config.loadConfigOrDefault().modelClasses).toEqual({ + small: "haiku+0", + large: "anthropic:claude-fable-5+max", + }); + }); + + it("round-trips modelClasses and skillModelClasses through editConfig saves", async () => { + const config = new Config(tempDir); + await config.editConfig((cfg) => ({ + ...cfg, + modelClasses: { small: "haiku+0", large: "anthropic:claude-fable-5+max" }, + skillModelClasses: { done: "small" }, + })); + + // A fresh instance re-reads from disk: the fields must survive the + // whitelist-based saveConfig serialization. + const reloaded = new Config(tempDir).loadConfigOrDefault(); + expect(reloaded.modelClasses).toEqual({ + small: "haiku+0", + large: "anthropic:claude-fable-5+max", + }); + expect(reloaded.skillModelClasses).toEqual({ done: "small" }); + + // An unrelated edit (another full save cycle) must not strip them. + const second = new Config(tempDir); + await second.editConfig((cfg) => ({ ...cfg, defaultModel: "anthropic:claude-opus-5" })); + const reloadedAgain = new Config(tempDir).loadConfigOrDefault(); + expect(reloadedAgain.modelClasses?.small).toBe("haiku+0"); + expect(reloadedAgain.skillModelClasses?.done).toBe("small"); + }); + + it("drops non-string entries on load instead of failing (self-healing)", async () => { + await fs.writeFile( + path.join(tempDir, "config.json"), + JSON.stringify({ + projects: [], + modelClasses: { small: "haiku+0", bad: 42 }, + skillModelClasses: 7, + }) + ); + + const loaded = new Config(tempDir).loadConfigOrDefault(); + expect(loaded.modelClasses).toEqual({ small: "haiku+0" }); + expect(loaded.skillModelClasses).toBeUndefined(); + }); +}); diff --git a/src/node/config/index.ts b/src/node/config/index.ts index c44bb04cb86..983fa2da9ad 100644 --- a/src/node/config/index.ts +++ b/src/node/config/index.ts @@ -1615,6 +1615,12 @@ export class Config { const modelFallbacks = normalizeModelFallbacks(parsed.modelFallbacks); + // Lenient on read: malformed entries never break config load. Values + // are judged at send time by resolveSkillModelClassBinding — a bound + // class with a bad value fails that send with an actionable error. + const modelClasses = parseOptionalStringRecord(parsed.modelClasses); + const skillModelClasses = parseOptionalStringRecord(parsed.skillModelClasses); + const defaultModel = normalizeOptionalModelString(parsed.defaultModel); const advisorModelString = parseOptionalNonEmptyString(parsed.advisorModelString); const advisorThinkingLevel = parseOptionalThinkingLevel(parsed.advisorThinkingLevel); @@ -1740,6 +1746,8 @@ export class Config { routeOverrides, minThinkingLevelByModel, modelFallbacks, + modelClasses, + skillModelClasses, defaultModel, advisorModelString, advisorThinkingLevel, @@ -1933,6 +1941,16 @@ export class Config { data.modelFallbacks = modelFallbacks; } + const modelClasses = parseOptionalStringRecord(config.modelClasses); + if (modelClasses !== undefined) { + data.modelClasses = modelClasses; + } + + const skillModelClasses = parseOptionalStringRecord(config.skillModelClasses); + if (skillModelClasses !== undefined) { + data.skillModelClasses = skillModelClasses; + } + const apiServerBindHost = parseOptionalNonEmptyString(config.apiServerBindHost); if (apiServerBindHost) { data.apiServerBindHost = apiServerBindHost; @@ -2128,6 +2146,8 @@ export class Config { routeOverrides: config.routeOverrides, minThinkingLevelByModel: config.minThinkingLevelByModel, modelFallbacks: config.modelFallbacks, + modelClasses: config.modelClasses, + skillModelClasses: config.skillModelClasses, defaultModel: config.defaultModel, advisorModelString: config.advisorModelString ?? null, advisorThinkingLevel: config.advisorThinkingLevel ?? null, @@ -2270,6 +2290,50 @@ export class Config { })); } + async updateModelClass(className: string, model: string | null): Promise { + // One entry per call, merged inside the editConfig transaction (which + // serializes read-modify-write under the config queue): a full-map + // replacement composed by a client would race a concurrent Settings + // consumer's edit of a DIFFERENT class and silently delete it. Values + // are stored verbatim — entries this build cannot parse (hand-edited + // custom models, future syntax) must survive edits of other classes. + // Broken values already fail loudly at send time and are flagged inline + // by the editor. + await this.editConfig((config) => { + const merged = { ...config.modelClasses }; + if (model == null) { + delete merged[className]; + } else { + merged[className] = model; + } + return { + ...config, + modelClasses: Object.keys(merged).length > 0 ? merged : undefined, + }; + }); + // saveConfig swallows write errors (read-only or full filesystem), and + // the editor publishes this RPC's resolution as the freshest backend + // truth — routing would then keep reading the OLD class while Settings + // displays the new one, with no notification to repair the split. Read + // back STRICTLY and fail loudly instead. Only THIS entry is verified: a + // concurrent peer edit of another class between the save and this read + // is legitimate, not a persistence failure. + let persisted: Record | undefined; + try { + persisted = this.loadConfigOrDefault({ throwOnError: true }).modelClasses; + } catch { + throw new Error( + "Could not verify the model class was persisted to config.json; the change was not applied." + ); + } + const persistedValue = persisted?.[className]; + if (model == null ? persistedValue !== undefined : persistedValue !== model) { + throw new Error( + "Failed to persist the model class to config.json; the change was not applied." + ); + } + } + async updateModelPreferences(input: { defaultModel?: string; hiddenModels?: string[]; diff --git a/src/node/orpc/router.ts b/src/node/orpc/router.ts index 27dc287b80e..79578fccb04 100644 --- a/src/node/orpc/router.ts +++ b/src/node/orpc/router.ts @@ -269,6 +269,12 @@ export const router = (authToken?: string) => { .output(schemas.config.updateModelFallbacks.output) .handler(({ context, input }) => context.config.updateModelFallbacks(input.modelFallbacks)), + updateModelClass: t + .input(schemas.config.updateModelClass.input) + .output(schemas.config.updateModelClass.output) + .handler(({ context, input }) => + context.config.updateModelClass(input.className, input.model) + ), updateModelPreferences: t .input(schemas.config.updateModelPreferences.input) .output(schemas.config.updateModelPreferences.output) diff --git a/src/node/services/agentSession.budgetGate.test.ts b/src/node/services/agentSession.budgetGate.test.ts index 000f2b18680..f998fbe9a88 100644 --- a/src/node/services/agentSession.budgetGate.test.ts +++ b/src/node/services/agentSession.budgetGate.test.ts @@ -230,7 +230,11 @@ describe("AgentSession.sendMessage budget gate", () => { }); const userTypedText = "Switch to my local model and keep going"; - const result = await session.sendMessage(userTypedText, UNPRICED_OPTIONS); + // The scenario this guards is the queue-drain path (composer already + // cleared): sendQueuedMessages stamps dequeued on dispatch, and the + // preservation contract is scoped to dequeued sends — a rejected DIRECT + // send restores the composer draft instead of appending a row. + const result = await session.sendMessage(userTypedText, UNPRICED_OPTIONS, { dequeued: true }); expect(result.success).toBe(false); const history = await historyService.getHistoryFromLatestBoundary(workspaceId); @@ -281,8 +285,11 @@ describe("AgentSession.sendMessage budget gate", () => { budgetCents: 500, }); + // enqueuedAtMs + dequeued mirror what sendQueuedMessages stamps on a + // queue-drained dispatch — the preservation contract is dequeued-scoped. const result = await session.sendMessage("Typed before the goal existed", UNPRICED_OPTIONS, { enqueuedAtMs, + dequeued: true, }); expect(result.success).toBe(false); @@ -410,7 +417,9 @@ describe("AgentSession.sendMessage budget gate", () => { // Step 3: the previously-queued unpriced send now drains. The AS gate // re-evaluates with the freshly-budgeted goal and rejects. const queuedText = "Queued unpriced"; - const racedResult = await session.sendMessage(queuedText, UNPRICED_OPTIONS); + const racedResult = await session.sendMessage(queuedText, UNPRICED_OPTIONS, { + dequeued: true, + }); expect(racedResult.success).toBe(false); if (!racedResult.success) { expect(racedResult.error.type).toBe("unknown"); diff --git a/src/node/services/agentSession.continueMessageAgentId.test.ts b/src/node/services/agentSession.continueMessageAgentId.test.ts index a0e0db679a8..e8f5bfc1cfb 100644 --- a/src/node/services/agentSession.continueMessageAgentId.test.ts +++ b/src/node/services/agentSession.continueMessageAgentId.test.ts @@ -519,9 +519,14 @@ describe("AgentSession continue-message agentId fallback", () => { expect(sendCount).toBe(1); }); - test("startup recovery retries pending follow-up after an initial send failure", async () => { + test("a failed follow-up dispatch preserves the prompt instead of re-dispatching", async () => { + // The follow-up text is the USER's prompt (their composer cleared when + // compaction started). A failed dispatch must move it into a durable + // rejected transcript row and clear the summary's marker — the row is + // the durable copy now, and re-dispatching the same failing send on + // every recovery pass would loop the failure instead of surfacing it. let sendCount = 0; - const { internals } = await createSession([ + const { historyService, internals } = await createSession([ compactionSummaryMessage("summary-retry", { text: "follow up retry", model: "openai:gpt-4o", @@ -530,26 +535,29 @@ describe("AgentSession continue-message agentId fallback", () => { ]); internals.sendMessage = mock(() => { sendCount += 1; - if (sendCount === 1) { - return Promise.resolve({ - success: false, - error: { type: "runtime_start_failed", message: "startup failed" }, - }); - } - return Promise.resolve({ success: true as const }); + return Promise.resolve({ + success: false, + error: { type: "runtime_start_failed", message: "startup failed" }, + }); }); internals.scheduleStartupRecovery(); await internals.startupRecoveryPromise; expect(sendCount).toBe(1); - expect(internals.startupRecoveryScheduled).toBe(false); + const history = await historyService.getLastMessages("ws", 5); + expect(history.success).toBe(true); + const preserved = history.success + ? history.data.find((msg) => msg.metadata?.preStreamRejected === true) + : undefined; + expect(preserved?.role).toBe("user"); + expect(preserved?.parts?.[0]).toMatchObject({ type: "text", text: "follow up retry" }); + + // Marker cleared: a second recovery pass finds nothing to dispatch. internals.scheduleStartupRecovery(); await internals.startupRecoveryPromise; - - expect(sendCount).toBe(2); - expect(internals.startupRecoveryScheduled).toBe(true); + expect(sendCount).toBe(1); }); // RLM keep-recent floor: post-crash recovery when the compaction summary is diff --git a/src/node/services/agentSession.skillModelRouting.test.ts b/src/node/services/agentSession.skillModelRouting.test.ts new file mode 100644 index 00000000000..598f2ff1b87 --- /dev/null +++ b/src/node/services/agentSession.skillModelRouting.test.ts @@ -0,0 +1,620 @@ +import { afterEach, describe, expect, it, mock, spyOn } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; + +import { KNOWN_MODELS } from "@/common/constants/knownModels"; +import { Ok } from "@/common/types/result"; +import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; +import type { Config } from "@/node/config"; +import type { AIService, StreamMessageOptions } from "@/node/services/aiService"; + +import { createAgentSessionHarness, createStartedTurnHandle } from "./agentSession.testHarness"; + +const USER_MODEL = "anthropic:claude-fable-5"; + +describe("AgentSession.sendMessage (per-skill model routing)", () => { + let historyCleanup: (() => Promise) | undefined; + const tempDirs: string[] = []; + const sessions: Array<{ dispose: () => void }> = []; + afterEach(async () => { + // Safety net: a failed assertion above a test's own dispose() must not + // leak a live session into the rest of the file, and temp skill trees + // must not accumulate in the OS temp dir. + for (const session of sessions.splice(0)) { + try { + session.dispose(); + } catch { + // Already disposed by the test body. + } + } + await historyCleanup?.(); + for (const dir of tempDirs.splice(0)) { + await fs.rm(dir, { recursive: true, force: true }); + } + }); + + async function createWorkspaceWithSkill(args: { skillName: string; metadataYaml?: string }) { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "mux-skill-routing-")); + tempDirs.push(tmp); + const skillDir = path.join(tmp, ".mux", "skills", args.skillName); + await fs.mkdir(skillDir, { recursive: true }); + const skillMarkdown = `---\nname: ${args.skillName}\ndescription: Test skill\n${args.metadataYaml ?? ""}---\n\nDo the thing.\n`; + await fs.writeFile(path.join(skillDir, "SKILL.md"), skillMarkdown, "utf-8"); + return tmp; + } + + async function createRoutingHarness(args: { + workspacePath: string; + /** Default true: routing fixtures exercise trusted-project behavior. */ + projectTrusted?: boolean; + /** Workspace kind for the metadata (scratch workdirs never route project skills). */ + workspaceKind?: string; + configValues?: { + modelClasses?: Record; + skillModelClasses?: Record; + routePriority?: string[]; + }; + /** When provided, getProvidersConfigSafe sees this map (enables the availability check). */ + providersConfig?: Record; + }) { + const workspaceId = "ws-skill-routing"; + const workspaceMeta = { + id: workspaceId, + name: "ws", + projectName: "proj", + projectPath: args.workspacePath, + namedWorkspacePath: args.workspacePath, + runtimeConfig: { type: "local" }, + ...(args.workspaceKind != null ? { kind: args.workspaceKind } : {}), + } as unknown as FrontendWorkspaceMetadata; + + const streamed: StreamMessageOptions[] = []; + const streamMessage = mock((opts: StreamMessageOptions) => { + streamed.push(opts); + return Promise.resolve(Ok(createStartedTurnHandle())); + }); + + const config = { + srcDir: "/tmp", + sessionsDir: "/tmp", + getSessionDir: mock((_workspaceId: string) => "/tmp"), + loadConfigOrDefault: mock(() => ({ + ...args.configValues, + // Project-scope frontmatter routing requires Project Trust; these + // fixtures write skills into the workspace's own project. + projects: new Map( + args.projectTrusted === false ? [] : [[args.workspacePath, { trusted: true }]] + ), + })), + } as unknown as Config; + + const { session, cleanup, historyService } = await createAgentSessionHarness({ + workspaceId, + config, + aiServiceOverrides: { + getWorkspaceMetadata: mock((_id: string) => Promise.resolve(Ok(workspaceMeta))), + streamMessage: streamMessage as unknown as AIService["streamMessage"], + ...(args.providersConfig != null + ? { getProvidersConfig: mock(() => args.providersConfig) } + : {}), + } as unknown as Partial, + }); + historyCleanup = cleanup; + sessions.push(session); + return { session, streamed, historyService }; + } + + function skillSendOptions(overrides?: Record) { + return { + model: USER_MODEL, + agentId: "exec", + muxMetadata: { + type: "agent-skill", + rawCommand: "/done", + skillName: "done", + scope: "project", + }, + ...overrides, + }; + } + + it("streams a metadata-bound skill on its class model with resolved thinking", async () => { + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + configValues: { modelClasses: { small: "haiku+0" } }, + }); + + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(true); + // The accepted-send payload reports the routed model and thinking so the + // frontend can attribute send telemetry to what actually streams. + expect(result.success && result.data?.routedModel).toBe(KNOWN_MODELS.HAIKU.id); + expect(result.success && result.data?.routedThinkingLevel).toBe("off"); + expect(streamed).toHaveLength(1); + expect(streamed[0].modelString).toBe(KNOWN_MODELS.HAIKU.id); + // "+0" is model-relative: haiku's lowest allowed level is "off". + expect(streamed[0].thinkingLevel).toBe("off"); + session.dispose(); + }); + + it("ignores project-skill frontmatter bindings in untrusted projects", async () => { + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + projectTrusted: false, + configValues: { modelClasses: { small: "haiku+0" } }, + }); + + // Repo-controlled frontmatter must not reroute the transcript to a + // different configured provider without Project Trust: the send streams + // on the workspace model as if the skill were unbound. + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(true); + expect(result.success && result.data?.routedModel).toBeUndefined(); + expect(streamed).toHaveLength(1); + expect(streamed[0].modelString).toBe(USER_MODEL); + session.dispose(); + }); + + it("never routes scratch-workspace project skills via frontmatter, even though scratch is app-trusted", async () => { + // Scratch workdirs hold cloned third-party repos whose .xum/skills are + // discovered; app-level scratch trust (isWorkspaceProjectTrusted) must not + // extend to provider-selection consent. + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + workspaceKind: "scratch", + // Even an (impossible in practice) trusted-projects entry must not help: + // the scratch guard fires before the project-trust lookup. + projectTrusted: true, + configValues: { modelClasses: { small: "haiku+0" } }, + }); + + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(true); + expect(result.success && result.data?.routedModel).toBeUndefined(); + expect(streamed[0].modelString).toBe(USER_MODEL); + session.dispose(); + }); + + it("rejects the routed turn when trust is revoked between routing and materialization", async () => { + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + // Retained by loadConfigOrDefault's closure, so flipping projectTrusted + // below is observed by the next trust read. + const harnessArgs: Parameters[0] = { + workspacePath, + configValues: { modelClasses: { small: "haiku+0" } }, + }; + const { session, streamed } = await createRoutingHarness(harnessArgs); + + // Revoke trust the instant routing consent is granted: the + // materialization-time re-read must reject the routed turn — exempting + // the pre-resolved package would ship the repo-controlled snapshot (and + // keep the class route) to the alternate provider without consent. + const withResolve = session as unknown as { + resolveSkillModelClassOverride: (...resolveArgs: unknown[]) => Promise; + }; + const originalResolve = withResolve.resolveSkillModelClassOverride.bind(session); + spyOn(withResolve, "resolveSkillModelClassOverride").mockImplementation( + async (...resolveArgs: unknown[]) => { + const resolved = await originalResolve(...resolveArgs); + harnessArgs.projectTrusted = false; + return resolved; + } + ); + + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.type).toBe("unknown"); + expect(JSON.stringify(result.error)).toMatch(/trust was revoked/i); + } + expect(streamed).toHaveLength(0); + session.dispose(); + }); + + it("rejects a revoked TABLE-bound route too (no fast path hides the invocation)", async () => { + // The trusted table-binding path used to skip package resolution, which + // left the routed invocation unidentifiable at recheck time — revocation + // then merely omitted the snapshot while the conversation still streamed + // on the class provider. + const workspacePath = await createWorkspaceWithSkill({ skillName: "done" }); + const harnessArgs: Parameters[0] = { + workspacePath, + configValues: { modelClasses: { small: "haiku+0" }, skillModelClasses: { done: "small" } }, + }; + const { session, streamed } = await createRoutingHarness(harnessArgs); + + const withResolve = session as unknown as { + resolveSkillModelClassOverride: (...resolveArgs: unknown[]) => Promise; + }; + const originalResolve = withResolve.resolveSkillModelClassOverride.bind(session); + spyOn(withResolve, "resolveSkillModelClassOverride").mockImplementation( + async (...resolveArgs: unknown[]) => { + const resolved = await originalResolve(...resolveArgs); + harnessArgs.projectTrusted = false; + return resolved; + } + ); + + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(false); + if (!result.success) { + expect(JSON.stringify(result.error)).toMatch(/trust was revoked/i); + } + expect(streamed).toHaveLength(0); + session.dispose(); + }); + + it("preserves a queued send rejected by the trust recheck", async () => { + // A dequeued prompt's composer already cleared: the recheck rejection + // must leave a durable transcript row (like the routing/pricing/PDF + // gates) instead of silently dropping it while the queue moves on. + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const harnessArgs: Parameters[0] = { + workspacePath, + configValues: { modelClasses: { small: "haiku+0" } }, + }; + const { session, streamed, historyService } = await createRoutingHarness(harnessArgs); + + const withResolve = session as unknown as { + resolveSkillModelClassOverride: (...resolveArgs: unknown[]) => Promise; + }; + const originalResolve = withResolve.resolveSkillModelClassOverride.bind(session); + spyOn(withResolve, "resolveSkillModelClassOverride").mockImplementation( + async (...resolveArgs: unknown[]) => { + const resolved = await originalResolve(...resolveArgs); + harnessArgs.projectTrusted = false; + return resolved; + } + ); + + const result = await session.sendMessage("Use skill done", skillSendOptions(), { + dequeued: true, + enqueuedAtMs: 123, + }); + expect(result.success).toBe(false); + expect(streamed).toHaveLength(0); + + const history = await historyService.getLastMessages("ws-skill-routing", 5); + expect(history.success).toBe(true); + const preserved = history.success + ? history.data.find((msg) => msg.metadata?.preStreamRejected === true) + : undefined; + expect(preserved?.role).toBe("user"); + expect(preserved?.parts?.[0]).toMatchObject({ type: "text", text: "Use skill done" }); + session.dispose(); + }); + + it("late revocation surfaces as an accepted pre-stream failure, not a retryable Err", async () => { + // Trust flips AFTER materialization (its internal recheck saw consent): + // the pre-stream consent gate must stop the dispatch — and because the + // turn's rows are durable by then, it must NOT return a pre-acceptance + // Err (the renderer would restore a draft that duplicates the persisted + // row). The visible record is the emitted stream error. + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const harnessArgs: Parameters[0] = { + workspacePath, + configValues: { modelClasses: { small: "haiku+0" } }, + }; + const { session, streamed, historyService } = await createRoutingHarness(harnessArgs); + + const withMaterialize = session as unknown as { + materializeAgentSkillSnapshots: (...materializeArgs: unknown[]) => Promise; + }; + const originalMaterialize = withMaterialize.materializeAgentSkillSnapshots.bind(session); + spyOn(withMaterialize, "materializeAgentSkillSnapshots").mockImplementation( + async (...materializeArgs: unknown[]) => { + const materialized = await originalMaterialize(...materializeArgs); + harnessArgs.projectTrusted = false; + return materialized; + } + ); + + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(true); + expect(result.success && result.data).toBeUndefined(); + expect(streamed).toHaveLength(0); + + // The turn's own row persisted exactly once — no rejected-copy duplicate. + const history = await historyService.getLastMessages("ws-skill-routing", 10); + expect(history.success).toBe(true); + if (history.success) { + const userRows = history.data.filter( + (msg) => msg.role === "user" && !msg.metadata?.synthetic + ); + expect(userRows).toHaveLength(1); + } + session.dispose(); + }); + + it("does not let an untrusted project skill inherit a name-keyed table binding", async () => { + // Shadow protection: the table entry's consent belongs to whatever skill + // the user knew by this name (typically global/built-in), and project + // skills win name collisions — so in an UNTRUSTED project a repo-shipped + // shadow must not route via the table either. + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + projectTrusted: false, + configValues: { + modelClasses: { small: "haiku+0" }, + skillModelClasses: { done: "small" }, + }, + }); + + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(true); + expect(result.success && result.data?.routedModel).toBeUndefined(); + expect(streamed[0].modelString).toBe(USER_MODEL); + session.dispose(); + }); + + it("lets the config skillModelClasses table win over frontmatter metadata", async () => { + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + configValues: { + modelClasses: { small: "haiku+0", big: "anthropic:claude-opus-5+high" }, + skillModelClasses: { done: "big" }, + }, + }); + + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(true); + expect(streamed[0].modelString).toBe(KNOWN_MODELS.OPUS.id); + expect(streamed[0].thinkingLevel).toBe("high"); + session.dispose(); + }); + + it("routes a table-bound skill that has no frontmatter metadata", async () => { + const workspacePath = await createWorkspaceWithSkill({ skillName: "done" }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + configValues: { + modelClasses: { small: "haiku+0" }, + skillModelClasses: { done: "small" }, + }, + }); + + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(true); + expect(streamed[0].modelString).toBe(KNOWN_MODELS.HAIKU.id); + session.dispose(); + }); + + it("never re-routes sends that carry an explicit model override (skipSkillModelRouting)", async () => { + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + configValues: { modelClasses: { small: "haiku+0" } }, + }); + + const result = await session.sendMessage( + "Use skill done", + skillSendOptions({ skipSkillModelRouting: true }) + ); + expect(result.success).toBe(true); + // No routing applied — the accepted-send payload must not name a model. + expect(result.success && result.data?.routedModel).toBeUndefined(); + expect(streamed[0].modelString).toBe(USER_MODEL); + expect(streamed[0].thinkingLevel).toBeUndefined(); + session.dispose(); + }); + + it("still routes sends that only skip settings persistence (thinking-only one-shots)", async () => { + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + configValues: { modelClasses: { small: "haiku+0" } }, + }); + + // "/+2 /done" sets skipAiSettingsPersistence (to protect preferences) with + // no model override — class routing must still apply to the model while + // the explicit thinking level wins over the class default. + const result = await session.sendMessage( + "Use skill done", + skillSendOptions({ skipAiSettingsPersistence: true, thinkingLevel: "medium" }) + ); + expect(result.success).toBe(true); + expect(streamed[0].modelString).toBe(KNOWN_MODELS.HAIKU.id); + expect(streamed[0].thinkingLevel).toBe("medium"); + // The payload reports the effective level even when the one-shot rode + // through unchanged — telemetry must see what the routed stream runs at. + expect(result.success && result.data?.routedThinkingLevel).toBe("medium"); + session.dispose(); + }); + + it("re-resolves a numeric one-shot thinking index against the routed model", async () => { + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + configValues: { modelClasses: { small: "haiku" } }, + }); + + // "/+0 /done" typed on a workspace model whose lowest allowed level is + // "medium": the frontend resolves thinkingLevel against the WORKSPACE + // ladder and passes the raw index alongside. The routed model's ladder + // differs (haiku's index 0 is "off"), so the re-resolved index — not the + // pre-resolved level — must win. + const result = await session.sendMessage( + "Use skill done", + skillSendOptions({ + skipAiSettingsPersistence: true, + thinkingLevel: "medium", + oneShotThinkingIndex: 0, + }) + ); + expect(result.success).toBe(true); + expect(streamed[0].modelString).toBe(KNOWN_MODELS.HAIKU.id); + expect(streamed[0].thinkingLevel).toBe("off"); + session.dispose(); + }); + + it("leaves frontmatter bindings to an undefined class inert (streams the caller's model)", async () => { + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: tiny\n", + }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + configValues: { modelClasses: { small: "haiku+0" } }, + }); + + // Skills the user does not own must not fail sends just because some + // other class is configured — an undefined frontmatter class is inert. + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(true); + expect(streamed[0].modelString).toBe(USER_MODEL); + session.dispose(); + }); + + it("fails the send with an actionable error on a dangling table binding", async () => { + const workspacePath = await createWorkspaceWithSkill({ skillName: "done" }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + // The table is the user's own routing intent: naming a class that no + // longer exists must error loudly, not silently unroute. + configValues: { modelClasses: { small: "haiku+0" }, skillModelClasses: { done: "tiny" } }, + }); + + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(false); + // The error must name the class so the user knows which mapping to fix. + const raw = !result.success && result.error.type === "unknown" ? result.error.raw : ""; + expect(raw).toContain('"tiny"'); + expect(streamed).toHaveLength(0); + session.dispose(); + }); + + it("honors frontmatter routing when a hand-edited table entry is blank", async () => { + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + // A blank table value (hand-edit meaning "no override") must not + // suppress the frontmatter read and silently unroute the skill. + configValues: { modelClasses: { small: "haiku+0" }, skillModelClasses: { done: " " } }, + }); + + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(true); + expect(streamed[0].modelString).toBe(KNOWN_MODELS.HAIKU.id); + session.dispose(); + }); + + it("fails the send with an actionable error when the class value is invalid", async () => { + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + // Hand-edited config can hold values the strict-on-write path would + // have rejected; the send must not silently ignore them. + configValues: { modelClasses: { small: "not-a-model" } }, + }); + + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(false); + const raw = !result.success && result.error.type === "unknown" ? result.error.raw : ""; + expect(raw).toContain('"small"'); + expect(streamed).toHaveLength(0); + session.dispose(); + }); + + it("fails the send when no configured route can serve the class model", async () => { + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + configValues: { modelClasses: { small: "haiku+0" }, routePriority: ["direct"] }, + providersConfig: { anthropic: { isConfigured: false } }, + }); + + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(false); + const raw = !result.success && result.error.type === "unknown" ? result.error.raw : ""; + expect(raw).toContain(KNOWN_MODELS.HAIKU.id); + expect(streamed).toHaveLength(0); + session.dispose(); + }); + + it("routes normally when the class model has a configured route", async () => { + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + configValues: { modelClasses: { small: "haiku+0" }, routePriority: ["direct"] }, + providersConfig: { anthropic: { isConfigured: true, isEnabled: true } }, + }); + + const result = await session.sendMessage("Use skill done", skillSendOptions()); + expect(result.success).toBe(true); + expect(streamed[0].modelString).toBe(KNOWN_MODELS.HAIKU.id); + session.dispose(); + }); + + it("leaves non-skill sends untouched even with routing configured", async () => { + const workspacePath = await createWorkspaceWithSkill({ + skillName: "done", + metadataYaml: "metadata:\n model-class: small\n", + }); + const { session, streamed } = await createRoutingHarness({ + workspacePath, + configValues: { + modelClasses: { small: "haiku+0" }, + skillModelClasses: { done: "small" }, + }, + }); + + const result = await session.sendMessage("plain message", { + model: USER_MODEL, + agentId: "exec", + }); + expect(result.success).toBe(true); + expect(streamed[0].modelString).toBe(USER_MODEL); + session.dispose(); + }); +}); diff --git a/src/node/services/agentSession.startupAutoRetry.test.ts b/src/node/services/agentSession.startupAutoRetry.test.ts index 424892c4b6a..96406246db8 100644 --- a/src/node/services/agentSession.startupAutoRetry.test.ts +++ b/src/node/services/agentSession.startupAutoRetry.test.ts @@ -12,7 +12,11 @@ import type { HistoryService } from "./historyService"; import type { Config } from "@/node/config"; import type { InitStateManager } from "./initStateManager"; import type { WorkspaceChatMessage, SendMessageOptions } from "@/common/orpc/types"; -import { createMuxMessage, pickStartupRetrySendOptions } from "@/common/types/message"; +import { + createMuxMessage, + pickStartupRetrySendOptions, + type StartupRetrySendOptions, +} from "@/common/types/message"; import { DEFAULT_RUNTIME_CONFIG } from "@/common/constants/workspace"; import type { WorkspaceMetadata } from "@/common/types/workspace"; import { Ok } from "@/common/types/result"; @@ -24,6 +28,8 @@ interface AutoRetryResumeRequest { options: SendMessageOptions; agentInitiated?: boolean; goalKind?: typeof GOAL_CONTINUATION_KIND; + /** Routed-turn compaction context; must stay absent for malformed rows. */ + compactionBaseOptions?: SendMessageOptions; } interface RetryableSessionForTests { @@ -744,6 +750,131 @@ describe("AgentSession startup auto-retry recovery", () => { session.dispose(); }); + test("malformed persisted compactionBaseOptions neither marks the row routed nor is forwarded", async () => { + const workspaceId = "startup-retry-child-malformed-routed-context"; + const workspaceMetadata: WorkspaceMetadata = { + id: workspaceId, + name: workspaceId, + projectName: "project", + projectPath: "/tmp/project", + runtimeConfig: DEFAULT_RUNTIME_CONFIG, + parentWorkspaceId: "parent-workspace", + agentId: "explore", + agentType: "explore", + aiSettingsByAgent: { + explore: { model: "openai:gpt-5.5-low", thinkingLevel: "low" }, + }, + }; + const { session, historyService, cleanup } = await createAgentSessionHarness({ + workspaceId, + aiServiceOverrides: { + getWorkspaceMetadata: mock(() => Promise.resolve(Ok(workspaceMetadata))), + }, + }); + cleanups.push(cleanup); + + // chat.jsonl is unchecked JSON: a corrupted non-null compactionBaseOptions + // (boolean here) must not flip child-workspace precedence toward the + // persisted outer model, and must not ride into the resume request as a + // compaction base. + const appendResult = await historyService.appendToHistory( + workspaceId, + createMuxMessage("user-1", "user", "Interrupted routed child turn", { + timestamp: Date.now(), + retrySendOptions: { + model: "anthropic:claude-opus-5", + agentId: "explore", + compactionBaseOptions: true, + } as unknown as StartupRetrySendOptions, + }) + ); + expect(appendResult.success).toBe(true); + + session.ensureStartupAutoRetryCheck(); + + const startupCheckPromise = ( + session as unknown as { startupAutoRetryCheckPromise: Promise | null } + ).startupAutoRetryCheckPromise; + await startupCheckPromise; + + const retryOptions = ( + session as unknown as { + lastAutoRetryResumeRequest?: AutoRetryResumeRequest; + } + ).lastAutoRetryResumeRequest; + expect(retryOptions).toBeDefined(); + if (!retryOptions) { + throw new Error("Expected startup retry options"); + } + + // Child settings win because the row does not count as routed. + expect(retryOptions.options.model).toBe("openai:gpt-5.5-low"); + expect(retryOptions.compactionBaseOptions).toBeUndefined(); + + session.dispose(); + }); + + test("routed-retry context with an invalid model id is rejected like the startup model path", async () => { + const workspaceId = "startup-retry-child-garbage-routed-model"; + const workspaceMetadata: WorkspaceMetadata = { + id: workspaceId, + name: workspaceId, + projectName: "project", + projectPath: "/tmp/project", + runtimeConfig: DEFAULT_RUNTIME_CONFIG, + parentWorkspaceId: "parent-workspace", + agentId: "explore", + agentType: "explore", + aiSettingsByAgent: { + explore: { model: "openai:gpt-5.5-low", thinkingLevel: "low" }, + }, + }; + const { session, historyService, cleanup } = await createAgentSessionHarness({ + workspaceId, + aiServiceOverrides: { + getWorkspaceMetadata: mock(() => Promise.resolve(Ok(workspaceMetadata))), + }, + }); + cleanups.push(cleanup); + + // An object shape with a model that fails provider:model validation must + // not count as routed either — same bar as normalizeStartupModel. + const appendResult = await historyService.appendToHistory( + workspaceId, + createMuxMessage("user-1", "user", "Interrupted routed child turn", { + timestamp: Date.now(), + retrySendOptions: { + model: "anthropic:claude-opus-5", + agentId: "explore", + compactionBaseOptions: { model: "garbage" }, + } as unknown as StartupRetrySendOptions, + }) + ); + expect(appendResult.success).toBe(true); + + session.ensureStartupAutoRetryCheck(); + + const startupCheckPromise = ( + session as unknown as { startupAutoRetryCheckPromise: Promise | null } + ).startupAutoRetryCheckPromise; + await startupCheckPromise; + + const retryOptions = ( + session as unknown as { + lastAutoRetryResumeRequest?: AutoRetryResumeRequest; + } + ).lastAutoRetryResumeRequest; + expect(retryOptions).toBeDefined(); + if (!retryOptions) { + throw new Error("Expected startup retry options"); + } + + expect(retryOptions.options.model).toBe("openai:gpt-5.5-low"); + expect(retryOptions.compactionBaseOptions).toBeUndefined(); + + session.dispose(); + }); + test("replays pending auto-retry schedule during reconnect catch-up", async () => { const workspaceId = "startup-retry-replay-snapshot"; const { session, historyService, cleanup } = await createSessionBundle(workspaceId); diff --git a/src/node/services/agentSession.ts b/src/node/services/agentSession.ts index 3df43c6c9e6..7c9ce67705b 100644 --- a/src/node/services/agentSession.ts +++ b/src/node/services/agentSession.ts @@ -38,7 +38,7 @@ import { SILENT_CONTINUATION_COMPLETION_SUMMARY_MAX_LENGTH, type GoalSyntheticMessageKind, } from "@/constants/goals"; -import type { SendMessageError } from "@/common/types/errors"; +import type { SendMessageAccepted, SendMessageError } from "@/common/types/errors"; import { AgentIdSchema, ChatMuxMessageSchema, @@ -46,11 +46,14 @@ import { SkillNameSchema, } from "@/common/orpc/schemas"; import { ToolPolicySchema } from "@/common/orpc/schemas/stream"; +import { isWorkspaceProjectTrusted } from "@/node/utils/projectTrust"; +import { roundToBase2 } from "@/common/telemetry/utils"; import { normalizeAgentId, resolvePersistedAgentIdCandidates } from "@/common/utils/agentIds"; import { buildStreamErrorEventData, createStreamErrorMessage, createUnknownSendMessageError, + ROUTED_SKILL_TRUST_REVOKED_MESSAGE, type StreamErrorPayload, } from "@/node/services/utils/sendMessageError"; import { @@ -82,10 +85,13 @@ import { enforceThinkingPolicy, lookupMinThinkingLevelOverride, resolveMinimumThinkingLevel, + resolveThinkingInput, } from "@/common/utils/thinking/policy"; import type { ActiveTurnThinkingOverride } from "@/node/services/thinkingOverride"; import { + filterPreStreamRejectedRows, createMuxMessage, + STARTUP_RETRY_DURABLE_SEND_OPTION_KEYS, dedupeAgentSkillRefs, dedupeMcpPromptRefs, filterOrphanedMcpPromptSnapshots, @@ -146,6 +152,8 @@ import { } from "@/common/utils/messages/extractEditedFiles"; import { buildCompactionMessageText } from "@/common/utils/compaction/compactionPrompt"; import type { AutoCompactionUsageState } from "@/common/utils/compaction/autoCompactionCheck"; +import { ROUTED_SEND_COMPACTION_HEADROOM_PERCENT } from "@/common/constants/ui"; +import { getEffectiveContextLimit } from "@/common/utils/compaction/contextLimit"; import { getModelCapabilitiesResolved } from "@/common/utils/ai/modelCapabilities"; import { getExplicitGatewayPrefix, @@ -161,8 +169,16 @@ import { isProviderConfigFixableError, } from "@/common/utils/messages/retryEligibility"; import { createDisplayUsage } from "@/common/utils/tokens/displayUsage"; -import { readAgentSkill } from "@/node/services/agentSkills/agentSkillsService"; +import { + type ResolvedAgentSkill, + readAgentSkill, +} from "@/node/services/agentSkills/agentSkillsService"; import { resolveSkillStorageContext } from "@/node/services/agentSkills/skillStorageContext"; +import { + describeSkillModelClassRoutingProblem, + resolveSkillModelClassBinding, +} from "@/common/utils/ai/skillModelClasses"; +import { isModelServableWithProvidersConfig } from "@/common/utils/ai/modelAvailability"; import { createLoadedSkillSnapshot, extractLoadedSkillSnapshotsFromMessages, @@ -243,6 +259,16 @@ interface AutoRetryResumeRequest { goalKind?: GoalSyntheticMessageKind; /** Goal identity matching goalKind; keeps retried streams goal-scoped. */ goalId?: string; + /** Routed project-skill turn: retries re-verify Project Trust (see resumeStream). */ + routedProjectConsent?: boolean; + /** + * Pre-skill-routing options for a routed turn (see + * activeStreamContext.compactionBaseOptions). A same-session retry must keep + * the routed compaction policy — without this, the retried stream would + * force-compact at the workspace threshold against the routed window and + * summarize on the wrong model. + */ + compactionBaseOptions?: SendMessageOptions; } function stripGoalInterventionPolicy(options: SendMessageOptions): SendMessageOptions { @@ -251,6 +277,64 @@ function stripGoalInterventionPolicy(options: SendMessageOptions): SendMessageOp return streamOptions; } +/** + * retrySendOptions comes from unchecked chat.jsonl JSON: a malformed + * compactionBaseOptions (boolean, string, partial object) must neither mark a + * row as routed — which would flip child-workspace model precedence toward the + * persisted outer model — nor be forwarded as a compaction base. A usable + * durable context needs at least the model that owns the larger window; every + * other field is re-coerced downstream like the outer persisted options. The + * nested field is stripped to uphold pickStartupRetrySendOptions' one-level + * invariant. + */ +function sanitizePersistedCompactionBaseOptions( + value: unknown +): Omit | undefined { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return undefined; + } + const record = value as Record; + if (typeof record.model !== "string" || record.model.trim().length === 0) { + return undefined; + } + // Same bar as the startup model path (normalizeStartupModel): a durable + // model must normalize to a valid provider:model id, or the row must not + // count as routed at all. + const normalizedModel = normalizeSelectedModel(record.model.trim()); + if (!isValidModelFormat(normalizedModel)) { + return undefined; + } + // Whitelist-then-schema-parse the durable subset: these values are spread + // into an internal send (buildAutoCompactionRequest), so a malformed + // sibling (providerOptions: false) must not ride into provider request + // construction, and a schema-VALID but non-durable field must not flip + // behavioral switches there — a smuggled editMessageId would send the + // restored compaction request down the edit/truncation path and delete + // history. The whitelist is the same key set pickStartupRetrySendOptions + // persists; retry-state extras (goalKind, agentInitiated) are dropped since + // the base feeds a fresh internal send, and muxMetadata mirrors the durable + // pick's narrowing (workspace-turn correlation only). + const candidate: Record = {}; + for (const key of STARTUP_RETRY_DURABLE_SEND_OPTION_KEYS) { + if (key in record) { + candidate[key] = record[key]; + } + } + const parsed = SendMessageOptionsSchema.safeParse(candidate); + if (!parsed.success) { + return undefined; + } + const typedMuxMetadata = parsed.data.muxMetadata as MuxMessageMetadata | undefined; + const durable = { + ...parsed.data, + ...(typedMuxMetadata?.type === "workspace-turn-task" + ? { muxMetadata: typedMuxMetadata } + : { muxMetadata: undefined }), + model: normalizedModel, + }; + return durable as Omit; +} + function getGoalStreamOriginKind(input: { isCompaction?: boolean; goalKind?: GoalSyntheticMessageKind; @@ -514,6 +598,9 @@ export async function clearProviderConfigFixableAbandonMarkers( export const CONTEXT_MUTATION_SEND_BLOCKED_MESSAGE = "Workspace history is being cleared or reset. Please wait and try again."; +// ROUTED_SKILL_TRUST_REVOKED_MESSAGE moved to utils/sendMessageError.ts so +// StreamManager's per-step consent gate can share it without an import cycle. + const STARTUP_AUTO_RETRY_HISTORY_FAILURE_BASE_DELAY_MS = 1_000; const STARTUP_AUTO_RETRY_HISTORY_FAILURE_MAX_DELAY_MS = 30_000; const MAX_STARTUP_RECOVERY_DEFERRED_ATTEMPTS = 4; @@ -697,6 +784,14 @@ export class AgentSession { private autoRetryStarting = false; private readonly retryManager: RetryManager; private lastAutoRetryResumeRequest?: AutoRetryResumeRequest; + private readonly telemetryService?: TelemetryService; + /** + * Rows whose durable preStreamRejected stamp FAILED (transient history + * rewrite error): request assembly filters these for the rest of the + * session so the rejected turn cannot reach a provider unstamped. Startup + * recovery re-attempts the durable stamp via the abandon marker. + */ + private readonly unstampedRejectedRowIds = new Set(); /** Startup recovery should run once per session to avoid duplicate retry timers on reconnect. */ private startupRecoveryScheduled = false; private startupRecoveryPromise: Promise | null = null; @@ -857,6 +952,14 @@ export class AgentSession { /** Goal identity matching goalKind, so mid-stream compaction follow-ups stay goal-scoped. */ goalId?: string; workspaceTurnMetadata?: Extract; + /** + * Pre-skill-routing options for compaction requests spawned off this + * stream. A turn routed to a small class model must never compact on that + * model — the compaction model has to fit the full uncompacted history — + * so both the on-send and mid-stream compaction sites build their request + * from these options when present. + */ + compactionBaseOptions?: SendMessageOptions; }; private activeCompactionRequest?: { @@ -918,6 +1021,7 @@ export class AgentSession { this.initStateManager = initStateManager; this.backgroundProcessManager = backgroundProcessManager; this.workspaceGoalService = workspaceGoalService; + this.telemetryService = telemetryService; this.sessionUsageService = sessionUsageService; this.keepBackgroundProcesses = keepBackgroundProcesses ?? false; this.sanitizeCliWorkspaceRegistration = sanitizeCliWorkspaceRegistration; @@ -1259,6 +1363,14 @@ export class AgentSession { "handleStreamFailureForAutoRetry requires a non-empty error.type" ); + // Consent refusals are non-retryable regardless of their generic + // "unknown" classification (see retryActiveStream): the verdict cannot + // change without user action, so never arm the retry manager for them. + if (error.message?.includes(ROUTED_SKILL_TRUST_REVOKED_MESSAGE)) { + this.emitRetryEvent({ type: "auto-retry-abandoned", reason: "pre_stream_rejected" }); + return; + } + // Load persisted preference before scheduling retries so an on-disk opt-out is // honored even when the first failure happens before startup recovery runs. await this.loadAutoRetryEnabledPreference(); @@ -1269,7 +1381,9 @@ export class AgentSession { options: SendMessageOptions | undefined, agentInitiated?: boolean, goalKind?: GoalSyntheticMessageKind, - goalId?: string + goalId?: string, + compactionBaseOptions?: SendMessageOptions, + routedProjectConsent?: boolean ): void { if (!options) { this.lastAutoRetryResumeRequest = undefined; @@ -1281,6 +1395,8 @@ export class AgentSession { ...(agentInitiated === true ? { agentInitiated: true } : {}), ...(goalKind != null ? { goalKind } : {}), ...(goalId != null ? { goalId } : {}), + ...(compactionBaseOptions != null ? { compactionBaseOptions } : {}), + ...(routedProjectConsent === true ? { routedProjectConsent: true } : {}), }; } @@ -1309,7 +1425,24 @@ export class AgentSession { agentInitiated: request.agentInitiated === true ? true : undefined, goalKind: request.goalKind, goalId: request.goalId, + compactionBaseOptions: request.compactionBaseOptions, + routedProjectConsent: request.routedProjectConsent, }); + if ( + !result.success && + result.error.type === "unknown" && + "raw" in result.error && + result.error.raw === ROUTED_SKILL_TRUST_REVOKED_MESSAGE + ) { + // Consent refusals are non-retryable: the verdict cannot change + // without user action (re-granting trust or sending a new turn), and + // RetryManager treats "unknown" as retryable with no attempt limit — + // it would recheck the same revoked trust forever. Persist the + // abandon so startup recovery stays stopped too. + await this.persistStartupAutoRetryAbandon("pre_stream_rejected"); + this.emitRetryEvent({ type: "auto-retry-abandoned", reason: "pre_stream_rejected" }); + return; + } if (result.success) { if (!result.data.started) { // resumeStream can defer when a turn is still PREPARING/COMPLETING. @@ -1341,7 +1474,8 @@ export class AgentSession { }); await this.updateStartupAutoRetryAbandonFromFailure( result.error.type, - this.activeStreamUserMessageId + this.activeStreamUserMessageId, + this.extractRetryFailureMessage(result.error) ); } finally { this.autoRetryStarting = false; @@ -1524,8 +1658,18 @@ export class AgentSession { private async updateStartupAutoRetryAbandonFromFailure( errorType: string, - userMessageId?: string + userMessageId?: string, + errorMessage?: string ): Promise { + // A consent refusal that surfaced through the generic stream error + // pipeline (per-step prepareStep rejection is a plain Error there): + // preserve the recognizable non-retryable classification — the generic + // clear below would erase the repair marker the rejection callback just + // persisted, and a restart would lose the quarantine key with it. + if (errorMessage?.includes(ROUTED_SKILL_TRUST_REVOKED_MESSAGE)) { + await this.persistStartupAutoRetryAbandon("pre_stream_rejected", userMessageId); + return; + } if ( isNonRetryableSendError({ type: errorType }) || isNonRetryableStreamError({ type: errorType }) @@ -1984,18 +2128,29 @@ export class AgentSession { const persistedModel = this.normalizeStartupModel(persistedRetrySendOptions?.model); const assistantModel = this.normalizeStartupModel(lastAssistantMessage?.metadata?.model); const agentSettingsModel = this.normalizeStartupModel(agentSettings?.model); - const baseModel = isChildTaskWorkspace - ? (agentSettingsModel ?? persistedModel ?? assistantModel ?? DEFAULT_MODEL) - : (persistedModel ?? assistantModel ?? agentSettingsModel ?? DEFAULT_MODEL); + // A retry row carrying routed compaction context recorded the CLASS model + // the turn actually streamed on. That persisted model must win even in + // child task workspaces (whose creation-time settings normally take + // precedence) — resuming on the workspace model while restoring a routed + // compaction policy would mismatch both. Agent identity stays the child's. + const persistedCompactionBaseOptions = sanitizePersistedCompactionBaseOptions( + persistedRetrySendOptions?.compactionBaseOptions + ); + const isRoutedRetryRow = persistedCompactionBaseOptions != null; + const baseModel = + isChildTaskWorkspace && !isRoutedRetryRow + ? (agentSettingsModel ?? persistedModel ?? assistantModel ?? DEFAULT_MODEL) + : (persistedModel ?? assistantModel ?? agentSettingsModel ?? DEFAULT_MODEL); const persistedThinkingLevel = coerceThinkingLevel(persistedRetrySendOptions?.thinkingLevel); const assistantThinkingLevel = coerceThinkingLevel( lastAssistantMessage?.metadata?.thinkingLevel ); const agentSettingsThinkingLevel = coerceThinkingLevel(agentSettings?.thinkingLevel); - const baseThinkingLevel = isChildTaskWorkspace - ? (agentSettingsThinkingLevel ?? persistedThinkingLevel ?? assistantThinkingLevel) - : (persistedThinkingLevel ?? assistantThinkingLevel ?? agentSettingsThinkingLevel); + const baseThinkingLevel = + isChildTaskWorkspace && !isRoutedRetryRow + ? (agentSettingsThinkingLevel ?? persistedThinkingLevel ?? assistantThinkingLevel) + : (persistedThinkingLevel ?? assistantThinkingLevel ?? agentSettingsThinkingLevel); // Pro reasoning mode threads alongside thinkingLevel from the same sources // (assistant message metadata does not carry it), so startup retries do not @@ -2149,6 +2304,19 @@ export class AgentSession { retryRequest.agentInitiated = true; } + // Routed turns persist their pre-routing compaction context; restore it so + // the post-relaunch retry keeps the routed compaction policy instead of + // force-compacting at the workspace threshold against the routed window. + if (persistedCompactionBaseOptions != null) { + retryRequest.compactionBaseOptions = persistedCompactionBaseOptions; + } + + // Routed project-skill turns re-verify Project Trust on every resumed + // dispatch (the resume path bypasses the send gates). + if (persistedRetrySendOptions?.routedProjectConsent === true) { + retryRequest.routedProjectConsent = true; + } + return retryRequest; } @@ -2214,6 +2382,11 @@ export class AgentSession { } const autoRetryEnabled = await this.loadAutoRetryEnabledPreference(); + // Quarantine repair runs regardless of the auto-retry preference (which + // loadAutoRetryEnabledPreference just hydrated): the hazard is the next + // MANUAL send's request including unstamped rejected rows, not automatic + // replay. + await this.repairUnstampedRejectedTurn(); if (!autoRetryEnabled) { this.resetStartupAutoRetryHistoryReadBackoff(); return "completed"; @@ -2266,6 +2439,15 @@ export class AgentSession { this.shouldUseUserMessageForRetry(message) ); + // Pre-stream gate rejections never streamed and must never be replayed: + // the row-level stamp is atomic with the row itself, so it holds even + // when the crash landed between the row append and the preference-file + // abandon write below. + if (startupRetryUserMessage?.metadata?.preStreamRejected === true) { + this.emitRetryEvent({ type: "auto-retry-abandoned", reason: "pre_stream_rejected" }); + return "completed"; + } + if (this.startupAutoRetryAbandon) { const abandonReason = this.startupAutoRetryAbandon.reason; const abandonMatchesCurrentTail = @@ -2274,7 +2456,8 @@ export class AgentSession { if ( abandonMatchesCurrentTail && - (isNonRetryableSendError({ type: abandonReason }) || + (abandonReason === "pre_stream_rejected" || + isNonRetryableSendError({ type: abandonReason }) || isNonRetryableStreamError({ type: abandonReason })) ) { this.emitRetryEvent({ type: "auto-retry-abandoned", reason: abandonReason }); @@ -2293,8 +2476,25 @@ export class AgentSession { return "completed"; } - const { agentInitiated, goalKind, goalId, ...resumeOptions } = retryRequest; - this.setAutoRetryResumeState(resumeOptions, agentInitiated, goalKind, goalId); + // compactionBaseOptions is retry-state metadata, not a send option: it + // must feed the resume state's routed-compaction context, never ride + // inside the replayed SendMessageOptions themselves. + const { + agentInitiated, + goalKind, + goalId, + compactionBaseOptions, + routedProjectConsent, + ...resumeOptions + } = retryRequest; + this.setAutoRetryResumeState( + resumeOptions, + agentInitiated, + goalKind, + goalId, + compactionBaseOptions, + routedProjectConsent + ); } // Disk reads above may race with user actions; retry once the current work settles @@ -3005,6 +3205,8 @@ export class AgentSession { message: string, options?: SendMessageOptions & { fileParts?: FilePart[] }, internal?: { + /** Queue-dispatched entry (see SendMessageInternalOptions.dequeued). */ + dequeued?: boolean; synthetic?: boolean; agentInitiated?: boolean; goalContinuation?: boolean; @@ -3075,7 +3277,7 @@ export class AgentSession { */ admissionStale?: () => boolean; } - ): Promise> { + ): Promise> { this.assertNotDisposed("sendMessage"); assert(typeof message === "string", "sendMessage requires a string message"); @@ -3193,16 +3395,63 @@ export class AgentSession { // PRRT_kwDOPxxmWM5_s-jo). For synthetic sends (compaction, goal // continuation, etc.) the user did not type the message, so we just // return Err and let the synthetic caller log/handle it. + // Resolve per-skill model routing before any gate or mutation below: the + // pricing gate and PDF preflight must judge the model that will actually + // stream, and a broken class binding must reject the send BEFORE the edit + // path truncates history (see the invariant comment on the edit branch). + // Mirroring the pricing gate: a manual send rejected here is persisted and + // surfaced as a stream-error — a bare Err would let sendQueuedMessages() + // drop the user's queued input with no visible feedback. + const typedMuxMetadata = options?.muxMetadata as MuxMessageMetadata | undefined; + const skillModelOverride = options + ? await this.resolveSkillModelClassOverride(typedMuxMetadata, options) + : null; + if (await cancelBeforeAcceptance()) { + return Ok(undefined); + } + if (skillModelOverride?.kind === "config-error") { + const routingError = createUnknownSendMessageError(skillModelOverride.message); + // Preservation exists for dequeued sends whose composer already cleared — + // a rejected EDIT must not append the edited text as a new tail turn + // (the original message is untouched and the browser restores the draft). + if (isManualUserMessage && options?.editMessageId == null && internal?.dequeued === true) { + // Queue authoring time rides along like the pricing rejection below: + // without it, a skill queued before a later goal activation reads as + // post-goal and wrongly pauses the fresh goal (and the unstamped row + // is misclassified again after restart). + const persisted = await this.preserveRejectedManualSend( + message, + options, + routingError, + internal?.enqueuedAtMs + ); + if (persisted) { + await this.applyManualUserMessageGoalSafety({ + policy: "pause", + enqueuedAtMs: internal?.enqueuedAtMs, + }); + } + } + return Err(routingError); + } + // The model every downstream gate must validate: the routed class model + // when routing applies, else the caller's model. + const effectiveModelForGates = skillModelOverride?.model ?? options?.model; + if (this.workspaceGoalService) { const pricingGate = await this.workspaceGoalService.assertPricedModelForBudgetedGoal( this.workspaceId, - options?.model + effectiveModelForGates ); if (await cancelBeforeAcceptance()) { return Ok(undefined); } if (!pricingGate.success) { - if (isManualUserMessage) { + // Like the class-routing and PDF gates: preservation is for dequeued + // sends whose composer already cleared — a rejected EDIT (now + // reachable here via routed skill edits) must not append the edited + // text as a new tail turn. + if (isManualUserMessage && options?.editMessageId == null && internal?.dequeued === true) { const persisted = await this.preserveRejectedManualSend( message, options, @@ -3307,16 +3556,48 @@ export class AgentSession { (part) => normalizeMediaType(part.mediaType) === PDF_MEDIA_TYPE ); - if (pdfParts.length > 0) { + if (pdfParts.length > 0 && effectiveModelForGates != null) { + // Judge the routed class model when skill routing applies — the + // workspace model's PDF support is irrelevant to what will stream. const caps = getModelCapabilitiesResolved( - options.model, + effectiveModelForGates, this.aiService.getProvidersConfig() ); + // Rejections persist + surface like the pricing/routing gates: routable + // skill sends skip the browser PDF preflight and can arrive here from + // the queue drain, where a bare Err would silently discard the user's + // text and attachment (the composer already cleared on queue accept). + const rejectPdf = async ( + errorMessage: string + ): Promise> => { + const pdfError = createUnknownSendMessageError(errorMessage); + // See the class-routing gate above: preservation is for dequeued + // sends, never for rejected edits (which would duplicate the turn). + if ( + isManualUserMessage && + options?.editMessageId == null && + internal?.dequeued === true + ) { + // Same queue-timestamp threading as the routing and pricing gates. + const persisted = await this.preserveRejectedManualSend( + message, + options, + pdfError, + internal?.enqueuedAtMs + ); + if (persisted) { + await this.applyManualUserMessageGoalSafety({ + policy: "pause", + enqueuedAtMs: internal?.enqueuedAtMs, + }); + } + } + return Err(pdfError); + }; + if (caps && !caps.supportsPdfInput) { - return Err( - createUnknownSendMessageError(`Model ${options.model} does not support PDF input.`) - ); + return rejectPdf(`Model ${effectiveModelForGates} does not support PDF input.`); } if (caps?.maxPdfSizeMb !== undefined) { @@ -3326,10 +3607,8 @@ export class AgentSession { if (bytes !== null && bytes > maxBytes) { const actualMb = (bytes / (1024 * 1024)).toFixed(1); const label = part.filename ?? "PDF"; - return Err( - createUnknownSendMessageError( - `${label} is ${actualMb}MB, but ${options.model} allows up to ${caps.maxPdfSizeMb}MB per PDF.` - ) + return rejectPdf( + `${label} is ${actualMb}MB, but ${effectiveModelForGates} allows up to ${caps.maxPdfSizeMb}MB per PDF.` ); } } @@ -3390,6 +3669,20 @@ export class AgentSession { }; using _editAdmission = editAdmission; + // Edit turns materialize skill snapshots BEFORE truncation (see below); + // the persistence section reuses this instead of materializing again. + let preTruncationSkillSnapshots: { + messages: MuxMessage[]; + carriesProjectSkillContent: boolean; + } | null = null; + // Whether project-scope skill content rides this routed turn: seeded + // from the invoked package's scope, widened by materialization (an + // inline $project-skill ref travels on a globally-invoked routed turn + // too). The late consent gates key on this. + let routedTurnCarriesProjectContent = + skillModelOverride?.kind === "override" && + skillModelOverride.resolvedPackage?.package.scope === "project"; + if (editMessageId) { // Ensure no in-flight completion code can append after we truncate. if (this.isBusy()) { @@ -3463,12 +3756,67 @@ export class AgentSession { // so the user can re-evaluate, and start the edit stream with an empty queue. this.restoreQueueToInput(); + // Provider-selection consent can be revoked while the edit waited for + // idle above: recheck BEFORE the destructive truncation — rejecting + // after it would leave a partial edit (deleted tail, no replacement + // turn) with only the renderer draft restored. + if ( + skillModelOverride?.kind === "override" && + skillModelOverride.resolvedPackage?.package.scope === "project" && + !(await this.isRoutedProjectSkillTurnStillTrusted()) + ) { + return Err(createUnknownSendMessageError(ROUTED_SKILL_TRUST_REVOKED_MESSAGE)); + } + + // Materialize skill snapshots BEFORE the destructive truncation: the + // materialization awaits (skill reads, dynamic context injection) are + // where mid-send trust revocation and unresolvable-skill errors + // surface, and any rejection there must land while the edited row and + // tail still exist. Recent-snapshot dedupe is skipped — it would + // compare against rows the truncation below is about to delete and + // wrongly suppress a snapshot the rewritten history needs. The rows + // are APPENDED later, in the same persistence order as a plain send. + try { + preTruncationSkillSnapshots = await this.materializeAgentSkillSnapshots( + typedMuxMetadata, + options?.disableWorkspaceAgents, + skillModelOverride?.kind === "override" && skillModelOverride.resolvedPackage != null + ? new Map([ + [ + skillModelOverride.resolvedPackage.package.directoryName, + skillModelOverride.resolvedPackage, + ], + ]) + : undefined, + skillModelOverride?.kind === "override", + true + ); + } catch (error) { + return Err(createUnknownSendMessageError(getErrorMessage(error))); + } + if (preTruncationSkillSnapshots?.carriesProjectSkillContent) { + routedTurnCarriesProjectContent = true; + } + // Find the truncation target: the edited message or any immediately-preceding snapshots. // (snapshots are persisted immediately before their corresponding user message) // Pre-boundary edits are user-confirmed by the composer, so fall back to full-history lookup // when the edit target is outside the active context window. const truncateTargetId = await this.getEditTruncateTargetId(editMessageId); + // Last recheck immediately before the destructive truncation: the + // materialization and truncate-target reads above are awaits, and a + // rejection AFTER truncation cannot restore the discarded tail. Uses + // the widened flag (inline project refs discovered by the + // materialization above included). + if ( + skillModelOverride?.kind === "override" && + routedTurnCarriesProjectContent && + !(await this.isRoutedProjectSkillTurnStillTrusted()) + ) { + return Err(createUnknownSendMessageError(ROUTED_SKILL_TRUST_REVOKED_MESSAGE)); + } + this.clearUsageState(); const truncateResult = await this.historyService.truncateAfterMessage( this.workspaceId, @@ -3502,7 +3850,11 @@ export class AgentSession { historyService: this.historyService, aiService: this.aiService, workspaceId: this.workspaceId, - abandonedMessages: truncateResult.data.removedMessages, + // Quarantined rejected rows (failed durable stamp) are transcript- + // only: the side-channel summarizer must not distill them either. + abandonedMessages: truncateResult.data.removedMessages.filter( + (msg) => !this.unstampedRejectedRowIds.has(msg.id) + ), experiments: options?.experiments, isExperimentEnabled: typeof this.aiService.isExperimentEnabled === "function" @@ -3554,8 +3906,7 @@ export class AgentSession { // toolPolicy is properly typed via Zod schema inference const typedToolPolicy = options?.toolPolicy; - // muxMetadata is z.any() in schema - cast to proper type - const typedMuxMetadata = options?.muxMetadata as MuxMessageMetadata | undefined; + // typedMuxMetadata was hoisted above the routing/pricing gates. const acpPromptId = normalizeAcpPromptId(options?.acpPromptId) ?? extractAcpPromptId(typedMuxMetadata); const delegatedToolNames = @@ -3574,13 +3925,120 @@ export class AgentSession { ...(delegatedToolNames != null ? { delegatedToolNames } : {}), }); + // Apply the per-skill routing override resolved at the top of sendMessage + // (before the gates and the edit branch). Applied before the user message + // is created so startup retries (retrySendOptions) replay the routed + // model, and before the compaction threshold check so context-limit math + // uses the model that will actually stream. preRoutingOptions feeds the + // compaction REQUEST below: a turn routed to a small model must never + // compact on that small model — the compaction model has to fit the full + // uncompacted history. + const preRoutingOptions = optionsForStream; + let muxMetadataForMessage = typedMuxMetadata; + let routedThinkingLevel: ThinkingLevel | undefined; + if (skillModelOverride != null) { + modelForStream = skillModelOverride.model; + // Numeric one-shot thinking is model-relative: the frontend resolved + // options.thinkingLevel against the workspace model before routing was + // known, so "/+0 /skill" must be re-resolved here to mean the ROUTED + // model's lowest level, not the workspace model's. + const reroutedOneShotThinking = + options.oneShotThinkingIndex != null + ? resolveThinkingInput( + options.oneShotThinkingIndex, + skillModelOverride.model, + this.getProvidersConfigSafe() + ) + : undefined; + // Precedence: explicit numeric one-shot (re-resolved above) > class + // thinking > ambient options. skipAiSettingsPersistence marks one-shot + // sends, so a named "/+high /skill" keeps the user's level rather than + // the class default. + routedThinkingLevel = + reroutedOneShotThinking ?? + (skillModelOverride.thinkingLevel != null && options.skipAiSettingsPersistence !== true + ? skillModelOverride.thinkingLevel + : undefined); + optionsForStream = { + ...optionsForStream, + model: skillModelOverride.model, + ...(routedThinkingLevel != null ? { thinkingLevel: routedThinkingLevel } : {}), + }; + // The persisted request metadata must advertise the model that will + // actually stream: the pending-turn label and downstream consumers read + // requestedModel from the user message. + if (muxMetadataForMessage != null) { + muxMetadataForMessage = { + ...muxMetadataForMessage, + requestedModel: skillModelOverride.model, + }; + } + } + // RLM keep-recent floor: stamp compaction requests (manual /compact, // mid-stream forced, idle) with the durable tail-start sequence before the - // row is persisted. No-op when RLM is off. + // row is persisted. No-op when RLM is off. Takes the routing-aware + // metadata so a routed re-stamp (requestedModel) survives; the two stamps + // touch disjoint metadata types (compaction-request vs agent-skill). const stampedMuxMetadata = - isCompactionRequest && typedMuxMetadata?.type === "compaction-request" - ? await this.withKeepRecentTailStamp(typedMuxMetadata, optionsForStream) - : typedMuxMetadata; + isCompactionRequest && muxMetadataForMessage?.type === "compaction-request" + ? await this.withKeepRecentTailStamp(muxMetadataForMessage, optionsForStream) + : muxMetadataForMessage; + + // Routed sends report the class model and the effective thinking level + // back to the caller so successful-send telemetry attributes the + // invocation to what actually streams. The level is whatever the stream + // will receive (class suffix, re-resolved numeric one-shot, or a named + // one-shot / ambient level riding through), clamped by the same per-model + // floor enforcement the stream applies — "/+off /skill" routed onto a + // floor-medium model reports medium, not off. + const sendAccepted: SendMessageAccepted | undefined = + skillModelOverride != null + ? { + routedModel: skillModelOverride.model, + ...(optionsForStream.thinkingLevel != null + ? { + routedThinkingLevel: this.enforceThinkingFloorsForModel( + skillModelOverride.model, + optionsForStream.thinkingLevel, + this.getProvidersConfigSafe() + ), + } + : {}), + } + : undefined; + + // Which options a routed turn's compaction (on-send or mid-stream forced) + // must run with: the compaction request has to read the FULL uncompacted + // history, so it needs whichever model has the larger usable window. + // Routing usually shrinks the window (the user's model wins), but a class + // can also route UP — repeated routed turns can then grow the history past + // the user's model, and summarizing on it would just context-error again. + const compactionBaseOptionsForRoutedTurn = ((): SendMessageOptions | undefined => { + if (skillModelOverride == null) { + return undefined; + } + const providersConfigForWindows = this.getProvidersConfigSafe(); + const userModel = preRoutingOptions.model; + if (userModel == null) { + return optionsForStream; + } + const userLimit = getEffectiveContextLimit( + userModel, + this.is1MContextEnabledForModel(userModel, preRoutingOptions, providersConfigForWindows), + providersConfigForWindows + ); + const routedLimit = getEffectiveContextLimit( + skillModelOverride.model, + this.is1MContextEnabledForModel( + skillModelOverride.model, + optionsForStream, + providersConfigForWindows + ), + providersConfigForWindows + ); + return (routedLimit ?? 0) > (userLimit ?? 0) ? optionsForStream : preRoutingOptions; + })(); const userMessage = createMuxMessage( messageId, @@ -3590,8 +4048,17 @@ export class AgentSession { timestamp: Date.now(), toolPolicy: typedToolPolicy, disableWorkspaceAgents: options?.disableWorkspaceAgents, - retrySendOptions: pickStartupRetrySendOptions(optionsForStream, agentInitiated, goalKind), - muxMetadata: stampedMuxMetadata, // Pass through frontend metadata as black-box + retrySendOptions: pickStartupRetrySendOptions( + optionsForStream, + agentInitiated, + goalKind, + compactionBaseOptionsForRoutedTurn, + // Durable consent seed (invoked package's scope; this row persists + // before materialization can widen the flag for inline refs — the + // in-memory resume state below carries the final value). + routedTurnCarriesProjectContent + ), + muxMetadata: stampedMuxMetadata, // Frontend metadata; requestedModel re-stamped when routing applied ...(acpPromptId != null ? { acpPromptId } : {}), ...(goalKind != null ? { kind: goalKind } : {}), // Scope goal-loop rows to their goal so a replaced goal's continuation @@ -3660,8 +4127,18 @@ export class AgentSession { // before dispatching a risky user turn near the context limit. // `shouldForceCompact` remains a stricter (threshold + buffer) signal for // mid-stream forcing where we want to avoid abrupt interruptions too early. + // + // Skill-routed sends compact only when the content genuinely risks + // overrunning the routed model's window: applying the threshold to the + // (smaller) routed window would let a one-off cheap-skill invocation + // force an unrequested, irreversible, workspace-wide compaction of a + // session far under its own model's limit. The headroom accounts for + // the pending turn (new message, attachments, skill snapshot), which + // the recorded usage doesn't include yet. const shouldCompactBeforeSend = - compactionResult.usagePercentage >= compactionResult.thresholdPercentage; + skillModelOverride != null + ? compactionResult.usagePercentage >= 100 - ROUTED_SEND_COMPACTION_HEADROOM_PERCENT + : compactionResult.usagePercentage >= compactionResult.thresholdPercentage; if (shouldCompactBeforeSend) { const followUpFileParts = effectiveFileParts?.map((part) => ({ url: part.url, @@ -3687,10 +4164,15 @@ export class AgentSession { } } + // Pre-routing options/model: the deferred follow-up re-enters + // sendMessage with the same skill metadata and re-resolves routing at + // dispatch time. Persisting the routed model here would pin a stale + // decision — if the binding is gone by dispatch, the user's prompt + // would stream on the routed model with no routing decision behind it. const followUpContent = this.buildAutoCompactionFollowUp({ messageText: message, - options: optionsForStream, - modelForStream, + options: preRoutingOptions, + modelForStream: preRoutingOptions.model, fileParts: followUpFileParts, agentInitiated, goalKind, @@ -3708,10 +4190,49 @@ export class AgentSession { const autoCompactionRequest = this.buildAutoCompactionRequest({ followUpContent, - baseOptions: optionsForStream, + // The compaction request must run on the model able to read the full + // history — usually the user's pre-routing model, or the routed model + // when the class routes UP to a larger window. The deferred follow-up + // re-enters sendMessage with the same skill metadata and re-routes + // itself either way. + baseOptions: compactionBaseOptionsForRoutedTurn ?? preRoutingOptions, reason: "on-send", }); + // The pricing gate above validated the ROUTED model, but the + // compaction request may inherit the pre-routing ambient model (the + // larger-window pick): for a budgeted goal that model must be priced + // too, or the compaction stream's cost cannot be enforced against + // the budget. + if (this.workspaceGoalService) { + const compactionPricingGate = + await this.workspaceGoalService.assertPricedModelForBudgetedGoal( + this.workspaceId, + autoCompactionRequest.sendOptions.model + ); + if (!compactionPricingGate.success) { + if ( + isManualUserMessage && + options?.editMessageId == null && + internal?.dequeued === true + ) { + const persisted = await this.preserveRejectedManualSend( + message, + options, + compactionPricingGate.error, + internal?.enqueuedAtMs + ); + if (persisted) { + await this.applyManualUserMessageGoalSafety({ + policy: "pause", + enqueuedAtMs: internal?.enqueuedAtMs, + }); + } + } + return Err(compactionPricingGate.error); + } + } + // RLM keep-recent floor: stamp on-send auto-compaction requests with // the durable tail-start sequence. No-op when RLM is off. if (autoCompactionRequest.metadata.type === "compaction-request") { @@ -3784,21 +4305,97 @@ export class AgentSession { // On on-send compaction paths, snapshots are deferred with the follow-up turn. const shouldPersistTurnSnapshots = autoCompactionMessage === null; + // On-send compaction DEFERS the routed skill: the class-routed + // invocation has not dispatched (its persisted follow-up re-enters + // sendMessage and re-resolves routing after compaction — mapping, trust, + // and availability may all differ by then), so reporting the + // pre-compaction class model would attribute a dispatch that never + // happened. Report the deferral like a queued send instead. + const sendAcceptedFinal: SendMessageAccepted | undefined = + autoCompactionMessage !== null && sendAccepted != null ? { queued: true } : sendAccepted; + + // Consent recheck immediately before snapshots are built and persisted: + // the queue wait and preflights sit between the routing gate and this + // point. Runs the same dequeued-send preservation as the + // routing/pricing/PDF gates — the materialization-internal throw below + // surfaces as a bare Err, which would silently drop a queued prompt + // whose composer already cleared. Edit turns are exempt: their consent + // was checked (and their snapshots materialized) BEFORE the destructive + // truncation, and a rejection here would land after it. + if ( + options?.editMessageId == null && + skillModelOverride?.kind === "override" && + routedTurnCarriesProjectContent && + !(await this.isRoutedProjectSkillTurnStillTrusted()) + ) { + const trustError = createUnknownSendMessageError(ROUTED_SKILL_TRUST_REVOKED_MESSAGE); + if (isManualUserMessage && options?.editMessageId == null && internal?.dequeued === true) { + const persisted = await this.preserveRejectedManualSend( + message, + options, + trustError, + internal?.enqueuedAtMs + ); + if (persisted) { + await this.applyManualUserMessageGoalSafety({ + policy: "pause", + enqueuedAtMs: internal?.enqueuedAtMs, + }); + } + } + return Err(trustError); + } + let skillSnapshotMessages: MuxMessage[] = []; let mcpPromptSnapshotMessages: MuxMessage[] = []; if (shouldPersistTurnSnapshots) { try { - skillSnapshotMessages = await this.materializeAgentSkillSnapshots( - typedMuxMetadata, - options?.disableWorkspaceAgents - ); + const skillMaterialization = + preTruncationSkillSnapshots ?? + (await this.materializeAgentSkillSnapshots( + typedMuxMetadata, + options?.disableWorkspaceAgents, + skillModelOverride?.kind === "override" && skillModelOverride.resolvedPackage != null + ? new Map([ + [ + skillModelOverride.resolvedPackage.package.directoryName, + skillModelOverride.resolvedPackage, + ], + ]) + : undefined, + skillModelOverride?.kind === "override" + )); + skillSnapshotMessages = skillMaterialization.messages; + if (skillMaterialization.carriesProjectSkillContent) { + routedTurnCarriesProjectContent = true; + } mcpPromptSnapshotMessages = await this.materializeMcpPromptSnapshots( typedMuxMetadata, userMessage.id, cancelSignal ); } catch (error) { - return Err(createUnknownSendMessageError(getErrorMessage(error))); + const materializationError = createUnknownSendMessageError(getErrorMessage(error)); + // A queued prompt's composer already cleared: like the other + // pre-stream gates, a materialization failure (including the + // mid-materialization trust revocation throw) must leave a durable + // transcript row + visible error instead of silently dropping the + // send while sendQueuedMessages moves on. + if (isManualUserMessage && options?.editMessageId == null && internal?.dequeued === true) { + const persisted = await this.preserveRejectedManualSend( + message, + options, + materializationError, + internal?.enqueuedAtMs + ); + if (persisted) { + await this.applyManualUserMessageGoalSafety({ + policy: "pause", + enqueuedAtMs: internal?.enqueuedAtMs, + }); + } + } + return Err(materializationError); } if (await cancelBeforeAcceptance()) { return Ok(undefined); @@ -4029,7 +4626,16 @@ export class AgentSession { // Same-session retry should resume the exact accepted request we just finalized // in history, even if runtime warmup fails before streamWithHistory() starts. - this.setAutoRetryResumeState(optionsForStream, agentInitiated, goalKind, internal?.goalId); + this.setAutoRetryResumeState( + optionsForStream, + agentInitiated, + goalKind, + internal?.goalId, + compactionBaseOptionsForRoutedTurn, + // FINAL consent flag (post-materialization, inline refs included) — + // retries of this accepted request re-verify Project Trust. + skillModelOverride?.kind === "override" && routedTurnCarriesProjectContent + ); try { await internal?.onAccepted?.(); } catch (error) { @@ -4042,6 +4648,78 @@ export class AgentSession { } let acceptedPreStreamFailureNotified = false; + // Shared rejection for the late consent gates (pre-stream below and the + // provider-dispatch boundary inside streamWithHistory): performs durable + // bookkeeping and returns the error to surface. The turn's rows are + // durable by now, so rejection is non-destructive — but the accepted row + // must not be startup-resumable onto its persisted routed retry options + // (recovery honors this abandon reason without rerunning the gates), and + // the failure must be VISIBLE (these gates bypass streamWithHistory's + // own error emission). + const routedConsentRejection = async ( + // Set by the provider-boundary caller when the assembled REQUEST + // carries project-scope snapshot rows from EARLIER turns: an untrusted + // workspace's history can hold a project snapshot even when the + // current routed invocation is global with no project refs. + requestCarriesProjectContent?: boolean + ): Promise => { + if ( + skillModelOverride?.kind !== "override" || + !(routedTurnCarriesProjectContent || requestCarriesProjectContent === true) + ) { + return null; + } + if (await this.isRoutedProjectSkillTurnStillTrusted()) { + return null; + } + const trustError = createUnknownSendMessageError(ROUTED_SKILL_TRUST_REVOKED_MESSAGE); + // Durable, request-visible rejection: stamp the accepted turn's rows + // (filterPreStreamRejectedRows keys on ROW metadata — the sidecar + // abandon marker alone would leave this turn provider-eligible for the + // NEXT ordinary send), then belt with the abandon marker for startup + // recovery, then surface the visible error. + const rejectedRowIds = [ + userMessage.id, + ...skillSnapshotMessages.map((msg) => msg.id), + ...mcpPromptSnapshotMessages.map((msg) => msg.id), + // The @file-mention snapshot persisted with this turn carries + // repository contents too — an unstamped copy would stay + // provider-eligible after the turn's rejection. + ...(snapshotResult?.snapshotMessage != null ? [snapshotResult.snapshotMessage.id] : []), + ]; + let stampResult = await this.historyService.markMessagesPreStreamRejected( + this.workspaceId, + rejectedRowIds + ); + if (!stampResult.success) { + // Fail CLOSED: without the row marker the rejected turn stays + // provider-eligible for the next ordinary send (the sidecar abandon + // is invisible to request construction, and a later manual send + // clears it). Retry once; if the rewrite still fails, quarantine the + // ids in memory — request assembly filters them for the rest of the + // session, and startup recovery re-attempts the durable stamp when + // it sees the abandon reason. + stampResult = await this.historyService.markMessagesPreStreamRejected( + this.workspaceId, + rejectedRowIds + ); + } + if (!stampResult.success) { + for (const id of rejectedRowIds) { + this.unstampedRejectedRowIds.add(id); + } + log.warn("Failed to stamp rejected rows after consent revocation; quarantined in memory", { + workspaceId: this.workspaceId, + error: stampResult.error, + }); + } + await this.persistStartupAutoRetryAbandon("pre_stream_rejected", userMessage.id); + if (!this.disposed) { + this.emitChatEvent(createStreamErrorMessage(buildStreamErrorEventData(trustError))); + } + return trustError; + }; + const notifyAcceptedPreStreamFailure = async (error: SendMessageError): Promise => { if (acceptedPreStreamFailureNotified) { return; @@ -4082,7 +4760,9 @@ export class AgentSession { // service-side preflight reservation (see onTurnAdmissionCommitted doc). internal?.onTurnAdmissionCommitted?.(); - const startPreparedStream = async (): Promise> => { + const startPreparedStream = async (): Promise< + AgentSessionResult + > => { try { if (preparedTurnAbortController.signal.aborted) { await notifyAcceptedPreStreamFailure( @@ -4103,6 +4783,21 @@ export class AgentSession { return Ok(undefined); } + // Consent check before streamWithHistory's startup work: every await + // since the last gate (branch summary, file snapshot, MCP snapshots, + // history writes) is a revocation window — on edits those all run + // AFTER truncation. Post-acceptance, so the rejection surfaces as an + // accepted pre-stream failure (the emitted stream error is the + // visible record); an Err here would make the renderer restore the + // draft of a prompt that is already a durable transcript row. + { + const consentError = await routedConsentRejection(); + if (consentError) { + await notifyAcceptedPreStreamFailure(consentError); + return Ok(undefined); + } + } + // Turn-phase transitions for success are driven by stream events. const streamResult = await this.streamWithHistory( modelForStream, @@ -4113,8 +4808,20 @@ export class AgentSession { preparedTurnAbortController.signal, goalKind, internal?.goalId, - turnThinkingOverride + turnThinkingOverride, + compactionBaseOptionsForRoutedTurn, + routedConsentRejection ); + // The provider-boundary consent gate inside streamWithHistory + // surfaces here: same accepted-pre-stream conversion as above. + if ( + !streamResult.success && + streamResult.error.type === "unknown" && + streamResult.error.raw === ROUTED_SKILL_TRUST_REVOKED_MESSAGE + ) { + await notifyAcceptedPreStreamFailure(streamResult.error); + return Ok(undefined); + } if (streamResult.success && preparedTurnAbortController.signal.aborted) { await notifyAcceptedPreStreamFailure( createUnknownSendMessageError( @@ -4122,7 +4829,7 @@ export class AgentSession { ) ); } - return streamResult; + return streamResult.success ? Ok(sendAcceptedFinal) : streamResult; } finally { // Success should advance via stream events; if startup never emitted any, don't leave the // session stuck in PREPARING. Guard by controller identity so an aborted startup cannot @@ -4172,7 +4879,7 @@ export class AgentSession { } drainQueuedMessagesAfterFailedStartup(); }); - return Ok(undefined); + return Ok(sendAcceptedFinal); } // Non-edit sends preserve the old behavior so pre-stream startup failures still propagate to @@ -4182,7 +4889,15 @@ export class AgentSession { async resumeStream( options: SendMessageOptions, - internal?: { agentInitiated?: boolean; goalKind?: GoalSyntheticMessageKind; goalId?: string } + internal?: { + agentInitiated?: boolean; + goalKind?: GoalSyntheticMessageKind; + goalId?: string; + /** Routed-turn compaction context carried across same-session retries. */ + compactionBaseOptions?: SendMessageOptions; + /** Routed project-skill turn: re-verify Project Trust before dispatch. */ + routedProjectConsent?: boolean; + } ): Promise> { this.assertNotDisposed("resumeStream"); @@ -4200,6 +4915,39 @@ export class AgentSession { return Ok({ started: false }); } + // Routing consent is re-verified on EVERY resumed dispatch: trust can be + // revoked between the original acceptance and a same-session retry or a + // startup recovery, and this path bypasses the send gates. The Err is + // bounded by the retry machinery's attempt caps — and re-granting trust + // lets a later attempt proceed legitimately. + if ( + internal?.routedProjectConsent === true && + !(await this.isRoutedProjectSkillTurnStillTrusted()) + ) { + return Err(createUnknownSendMessageError(ROUTED_SKILL_TRUST_REVOKED_MESSAGE)); + } + // The same verdict rides to the provider-dispatch boundary (pricing, + // history reconstruction, request building, and stream startup below are + // all revocation windows). Fires on the persisted acceptance-time seed + // OR on the request scan — the replayed request carries the original + // turn's persisted snapshot rows, so history-carried and + // materialization-discovered project content is covered even when the + // pre-crash seed missed it. No row stamping here: the retry failure + // accounting owns bookkeeping for resumed attempts. + const isRoutedResume = + internal?.routedProjectConsent === true || internal?.compactionBaseOptions != null; + const resumedConsentRejection = isRoutedResume + ? async (requestCarriesProjectContent?: boolean): Promise => { + if (internal?.routedProjectConsent !== true && requestCarriesProjectContent !== true) { + return null; + } + if (await this.isRoutedProjectSkillTurnStillTrusted()) { + return null; + } + return createUnknownSendMessageError(ROUTED_SKILL_TRUST_REVOKED_MESSAGE); + } + : undefined; + if (this.workspaceGoalService) { const pricingGate = await this.workspaceGoalService.assertPricedModelForBudgetedGoal( this.workspaceId, @@ -4227,7 +4975,9 @@ export class AgentSession { optionsForStream, internal?.agentInitiated, internal?.goalKind, - internal?.goalId + internal?.goalId, + internal?.compactionBaseOptions, + internal?.routedProjectConsent ); this.preparingWorkspaceTurnMetadata = getWorkspaceTurnMuxMetadata(optionsForStream.muxMetadata); this.setTurnPhase(TurnPhase.PREPARING); @@ -4247,7 +4997,9 @@ export class AgentSession { undefined, internal?.goalKind, internal?.goalId, - turnThinkingOverride + turnThinkingOverride, + internal?.compactionBaseOptions, + resumedConsentRejection ); if (!result.success) { return result; @@ -4291,6 +5043,52 @@ export class AgentSession { return this.lastUsageState; } + /** + * Per-model thinking floor: the configured minThinkingLevelByModel override + * resolved against the model's policy. Tests may provide partial config + * mocks, so read overrides only when available. providersConfig lets mapped + * aliases (mappedToModel) resolve against the target model's policy. + */ + private resolveThinkingFloorForModel( + modelString: string, + providersConfig: ProvidersConfigMap | null + ): ThinkingLevel { + const maybeConfig = this.config as Config & { + loadConfigOrDefault?: () => { + minThinkingLevelByModel?: Record; + } | null; + }; + // Gateway-preserving key first (an explicit coder:/ + // floor stays distinct from a direct model with the same ID), with a + // legacy name-canonical fallback for floors persisted by older versions. + const minThinkingOverride = + typeof maybeConfig.loadConfigOrDefault === "function" + ? lookupMinThinkingLevelOverride( + maybeConfig.loadConfigOrDefault()?.minThinkingLevelByModel, + modelString + ) + : undefined; + return resolveMinimumThinkingLevel(modelString, minThinkingOverride, providersConfig); + } + + /** + * Apply per-model thinking floors + policy clamping — the single definition + * used by streamWithHistory's request build AND the accepted-send payload, + * so telemetry can never report a level the stream doesn't run at. + */ + private enforceThinkingFloorsForModel( + modelString: string, + thinkingLevel: ThinkingLevel, + providersConfig: ProvidersConfigMap | null + ): ThinkingLevel { + return enforceThinkingPolicy( + modelString, + thinkingLevel, + this.resolveThinkingFloorForModel(modelString, providersConfig), + providersConfig + ); + } + private getProvidersConfigSafe(): ProvidersConfigMap | null { try { // Prefer ProviderService's safe config view: it includes env/file API-key source @@ -4414,7 +5212,14 @@ export class AgentSession { // still need to stay defensive. return false; } + // True only once the row is durably in history: callers gate marker + // cleanup (dispatchPendingFollowUp's pendingFollowUp is the ONLY other + // durable copy of the prompt) and goal-safety pauses on it, so a failed + // append must report false — logging alone would let the caller delete + // the prompt's last copy. + let persisted = false; try { + const typedMuxMetadata = options?.muxMetadata as MuxMessageMetadata | undefined; const userMessage = createMuxMessage( createUserMessageId(), "user", @@ -4426,6 +5231,17 @@ export class AgentSession { // Without it a rejected queued send would pause a never-driven goal // on the next getGoal. timestamp: Date.now(), + // A rejected skill invocation keeps its invocation metadata: this + // preserved row is the ONLY transcript record of the send, and for + // queued skills `message` is the rewritten model-facing prompt — + // without rawCommand the user's typed "/skill args" and its badge + // are lost. Other metadata types stay off (a compaction-request + // stamp on a plain rejected row would confuse compaction detection). + ...(typedMuxMetadata?.type === "agent-skill" ? { muxMetadata: typedMuxMetadata } : {}), + // Atomic with the row (unlike the preference-file abandon marker + // below, which a crash between the two writes can lose): startup + // recovery must never replay a send its gate rejected. + preStreamRejected: true, ...(enqueuedAtMs != null ? { enqueuedAtMs } : {}), }, additionalParts.length > 0 ? additionalParts : undefined @@ -4436,8 +5252,19 @@ export class AgentSession { workspaceId: this.workspaceId, error: appendResult.error, }); - } else if (!this.disposed) { - this.emitChatEvent({ ...userMessage, type: "message" }); + } else { + // Durable from this point even if the marker write below throws: the + // row-atomic preStreamRejected stamp already gates startup recovery. + persisted = true; + // The preserved row is a REJECTED send, not an interrupted one: + // without a durable abandon marker, startup recovery would treat this + // tail user row as an interrupted request and resumeStream() it on + // the ambient model — bypassing the very gate (class routing, + // pricing, PDF) that rejected it. + await this.persistStartupAutoRetryAbandon("pre_stream_rejected", userMessage.id); + if (!this.disposed) { + this.emitChatEvent({ ...userMessage, type: "message" }); + } } } catch (error) { log.warn("Unexpected error persisting user message after pre-stream gate rejection", { @@ -4449,7 +5276,7 @@ export class AgentSession { const streamError = buildStreamErrorEventData(rejection); this.emitChatEvent(createStreamErrorMessage(streamError)); } - return true; + return persisted; } /** @@ -4785,7 +5612,10 @@ export class AgentSession { const autoCompactionRequest = this.buildAutoCompactionRequest({ followUpContent, - baseOptions: streamContext.options, + // Pre-routing options when the stream was skill-routed: the compaction + // request must never inherit a routed small model (it has to read the + // full uncompacted history) — mirrors the on-send compaction site. + baseOptions: streamContext.compactionBaseOptions ?? streamContext.options, reason: "mid-stream", }); @@ -4813,7 +5643,8 @@ export class AgentSession { }); await this.updateStartupAutoRetryAbandonFromFailure( failureType, - interruptedUserMessageId + interruptedUserMessageId, + this.extractRetryFailureMessage(sendResult.error) ); } @@ -4925,7 +5756,11 @@ export class AgentSession { type: failureType, message: this.extractRetryFailureMessage(error), }); - await this.updateStartupAutoRetryAbandonFromFailure(failureType, failedUserMessageId); + await this.updateStartupAutoRetryAbandonFromFailure( + failureType, + failedUserMessageId, + this.extractRetryFailureMessage(error) + ); } else { await this.handleStreamError(buildStreamErrorEventData(error, { acpPromptId })); } @@ -4965,7 +5800,21 @@ export class AgentSession { // Session-owned per-turn holder for mid-turn thinking changes. Passed // explicitly (not read from the field) so a preempted turn can never pick // up its replacement's holder. Absent for internal retry paths. - activeTurnThinkingOverride?: ActiveTurnThinkingOverride + activeTurnThinkingOverride?: ActiveTurnThinkingOverride, + // Pre-skill-routing options for compaction requests spawned off this + // stream (see activeStreamContext.compactionBaseOptions). Passed + // explicitly like the thinking holder so retry paths stay unaffected. + compactionBaseOptions?: SendMessageOptions, + // Late consent gate for routed project-skill turns, threaded to the + // provider-dispatch boundary inside AIService (invoked immediately + // before the stream manager starts the provider operation). Receives + // whether the assembled request carries historical project snapshots. + // Performs rejection bookkeeping and returns the error to surface; + // absent on retry/internal paths (resumeStream re-verifies consent + // itself via routedProjectConsent). + routedConsentRejection?: ( + requestCarriesProjectContent?: boolean + ) => Promise ): Promise> { const isStartupAbortRequested = (): boolean => abortSignal?.aborted === true; @@ -4988,6 +5837,7 @@ export class AgentSession { ...(goalKind != null ? { goalKind } : {}), ...(goalId != null ? { goalId } : {}), providersConfig, + ...(compactionBaseOptions != null ? { compactionBaseOptions } : {}), }; this.activeStreamUserMessageId = undefined; @@ -5027,6 +5877,15 @@ export class AgentSession { fileChangeDetection.commit(); } + // Request-time quarantine repair: a send can race the asynchronous + // startup recovery (getOrCreateSession exposes the session without + // awaiting it, and a PREPARING turn makes the recovery defer), so the + // rejected turn's row stamps and surviving partial must be repaired + // BEFORE this request reads history or commits partials. Marker-gated — + // a no-op in the common case. + await this.loadAutoRetryEnabledPreference(); + await this.repairUnstampedRejectedTurn(); + const historyResult = await this.historyService.getHistoryFromLatestBoundary(this.workspaceId); if (isStartupAbortRequested()) { return Ok(undefined); @@ -5039,8 +5898,13 @@ export class AgentSession { } // A crash between snapshot and user-row appends can leave orphaned prompt - // expansions on disk; exclude them from every provider request. - let requestMessages = filterOrphanedMcpPromptSnapshots(historyResult.data); + // expansions on disk; exclude them from every provider request. Rows + // preserved by pre-stream gate rejections stay visible in the transcript + // but never reach the provider — replaying them would duplicate the + // prompt after a retry (or re-fail on an incompatible PDF forever). + let requestMessages = filterPreStreamRejectedRows( + filterOrphanedMcpPromptSnapshots(historyResult.data) + ).filter((msg) => !this.unstampedRejectedRowIds.has(msg.id)); if (requestMessages.length === 0) { return await this.handleStreamWithHistoryFailure( @@ -5068,7 +5932,9 @@ export class AgentSession { await this.historyService.appendToHistory(this.workspaceId, sentinelMessage); const refreshed = await this.historyService.getHistoryFromLatestBoundary(this.workspaceId); if (refreshed.success) { - requestMessages = filterOrphanedMcpPromptSnapshots(refreshed.data); + requestMessages = filterPreStreamRejectedRows( + filterOrphanedMcpPromptSnapshots(refreshed.data) + ).filter((msg) => !this.unstampedRejectedRowIds.has(msg.id)); } } @@ -5105,32 +5971,9 @@ export class AgentSession { this.activeStreamHadPostCompactionInjection = postCompactionAttachments !== null && postCompactionAttachments.length > 0; - // Apply per-model thinking floors once so desktop, mobile, and ACP requests match. - // Tests may provide partial config mocks, so read overrides only when available. - const maybeConfig = this.config as Config & { - loadConfigOrDefault?: () => { - minThinkingLevelByModel?: Record; - } | null; - }; - // Gateway-preserving key first (an explicit coder:/ - // floor stays distinct from a direct model with the same ID), with a - // legacy name-canonical fallback for floors persisted by older versions. - const minThinkingOverride = - typeof maybeConfig.loadConfigOrDefault === "function" - ? lookupMinThinkingLevelOverride( - maybeConfig.loadConfigOrDefault()?.minThinkingLevelByModel, - modelString - ) - : undefined; - // Pass providersConfig so mapped aliases (mappedToModel -> e.g. GPT-5.6) - // clamp against the target model's policy — otherwise a capability level - // like native max would be stripped here before buildProviderOptions can - // resolve the alias. - const minThinkingLevel = resolveMinimumThinkingLevel( - modelString, - minThinkingOverride, - providersConfig - ); + // Mid-turn thinking overrides clamp against the same floor as the + // send-time level (single source of truth for the floor). + const minThinkingLevel = this.resolveThinkingFloorForModel(modelString, providersConfig); const effectiveThinkingLevel = options?.thinkingLevel ? enforceThinkingPolicy(modelString, options.thinkingLevel, minThinkingLevel, providersConfig) : undefined; @@ -5163,12 +6006,47 @@ export class AgentSession { normalizeDelegatedToolNames(options?.delegatedToolNames) ?? extractAcpDelegatedTools(optionsMuxMetadata); + // Provider-boundary consent gate, deferred INTO AIService (invoked + // immediately before streamManager.startStream — runtime init, model + // creation, and request building are all revocation windows). Bound here + // because only this scope can scan the assembled request for + // project-scope snapshots persisted by EARLIER turns: an untrusted + // workspace's history can carry one even when the current routed + // invocation is global. The gate performs the rejection bookkeeping; the + // caller converts the Err into an accepted pre-stream failure. + let preDispatchConsentGate: (() => Promise) | undefined; + if (routedConsentRejection) { + let requestCarriesProjectContent = requestMessages.some( + (msg) => msg.metadata?.agentSkillSnapshot?.scope === "project" + ); + if (requestCarriesProjectContent && !(await this.isRoutedProjectSkillTurnStillTrusted())) { + // Historical project snapshots in an UNTRUSTED workspace: exclude + // them from the routed request (least privilege, mirroring the + // fresh-snapshot omission) instead of rejecting the turn — global + // and built-in skills are allowed to route in untrusted projects, + // and rejecting on rows the rejection cannot remove would fail every + // later routed send deterministically. + requestMessages = requestMessages.filter( + (msg) => msg.metadata?.agentSkillSnapshot?.scope !== "project" + ); + log.warn("Excluding historical project skill snapshots from routed request", { + workspaceId: this.workspaceId, + }); + requestCarriesProjectContent = false; + } + // Bound once: rows kept under trust arm the gate so a revocation + // BETWEEN this assembly and any step's provider call still rejects. + const carriesForGate = requestCarriesProjectContent; + preDispatchConsentGate = () => routedConsentRejection(carriesForGate); + } + // Fatal pre-start failures (runtime readiness, strict agent resolution) // emit an error event for fire-and-forget senders and then return Err; // collect them so the Err path resolves each exactly once. const preStartErrors: StreamErrorPayload[] = []; const streamResult = await this.aiService.streamMessage({ messages: requestMessages, + preDispatchConsentGate, workspaceId: this.workspaceId, modelString, abortSignal, @@ -5568,7 +6446,10 @@ export class AgentSession { context.agentInitiated, undefined, context.goalKind, - context.goalId + context.goalId, + undefined, + // A routed turn's retry keeps its routed compaction policy. + context.compactionBaseOptions ); } finally { if (this.turnPhase === TurnPhase.PREPARING) { @@ -5741,11 +6622,37 @@ export class AgentSession { this.clearQueue(); } + // A mid-turn consent rejection (per-step gate) leaves the turn's + // in-flight assistant parts in partial.json — persisted by the stream + // error path BEFORE this handler runs. The next send would commit them + // as an orphaned assistant row that the user-row rejection filter cannot + // remove (and that can break tool/message ordering). Delete them with + // the rejected turn; if the delete fails, quarantine the would-be + // committed row id. + if (typeof data.error === "string" && data.error.includes(ROUTED_SKILL_TRUST_REVOKED_MESSAGE)) { + try { + const rejectedPartial = await this.historyService.readPartial(this.workspaceId); + const deleteResult = await this.historyService.deletePartial(this.workspaceId); + if (!deleteResult.success && rejectedPartial?.id != null) { + this.unstampedRejectedRowIds.add(rejectedPartial.id); + } + } catch (error) { + log.warn("Failed to remove in-flight assistant after consent rejection", { + workspaceId: this.workspaceId, + error: getErrorMessage(error), + }); + } + } + await this.handleStreamFailureForAutoRetry({ type: failureType, message: data.error, }); - await this.updateStartupAutoRetryAbandonFromFailure(failureType, failedUserMessageId); + await this.updateStartupAutoRetryAbandonFromFailure( + failureType, + failedUserMessageId, + data.error + ); this.resolveStreamErrorRecoveryDecision(data.messageId, "terminal"); this.emitChatEvent(streamErrorMessage); @@ -5905,6 +6812,13 @@ export class AgentSession { streamContext?.providersConfig ?? null ), providersConfig: streamContext?.providersConfig ?? null, + // A routed turn (compactionBaseOptions set) uses the routed-send + // policy mid-stream too: the ordinary threshold+buffer against the + // (usually smaller) routed window would immediately force the exact + // workspace-wide compaction the pre-send band declined to run. + ...(streamContext?.compactionBaseOptions != null + ? { forceThresholdPercentOverride: 100 - ROUTED_SEND_COMPACTION_HEADROOM_PERCENT } + : {}), }); if (shouldInterruptForCompaction) { @@ -6943,7 +7857,7 @@ export class AgentSession { this.preparingWorkspaceTurnMetadata = getWorkspaceTurnMuxMetadata(options?.muxMetadata); this.setTurnPhase(TurnPhase.PREPARING); - void this.sendMessage(message, options, { ...internal, enqueuedAtMs }) + void this.sendMessage(message, options, { ...internal, enqueuedAtMs, dequeued: true }) .then(async (result) => { // Keep the dispatch marker through the dequeue-to-stream-start window. A background // send can resolve before startup emits stream-start, and later reports must not claim @@ -6968,6 +7882,38 @@ export class AgentSession { this.setTurnPhase(TurnPhase.IDLE); } this.sendQueuedMessages(); + return; + } + if (this.turnPhase === TurnPhase.IDLE) { + // Accepted pre-stream failure (e.g. a late consent rejection): + // sendMessage resolved success but the turn is already back at + // IDLE with no stream-start ever fired — no stream-end drain + // will run for the remaining entries, so continue them here. + // Nothing streamed, so no dispatch attribution either. + if (!this.messageQueue.isEmpty()) { + this.sendQueuedMessages(); + } + return; + } + // Busy-queued SKILL sends suppressed the renderer's messageSent + // (routing unknown at queue time): attribute at dispatch, routed + // or unbound. A compaction-DEFERRED dispatch ({ queued: true }) + // has not streamed the skill — dispatchPendingFollowUp owns its + // attribution, and capturing here would double-count with a false + // model. + if ( + result.data?.queued !== true && + (options?.muxMetadata as MuxMessageMetadata | undefined)?.type === "agent-skill" + ) { + const dispatchModel = result.data?.routedModel ?? options?.model; + if (dispatchModel != null) { + await this.captureBackendMessageSent({ + model: dispatchModel, + agentId: options?.agentId, + messageLength: message.length, + thinkingLevel: result.data?.routedThinkingLevel ?? options?.thinkingLevel, + }); + } } }) .catch(async (error: unknown) => { @@ -7333,6 +8279,11 @@ export class AgentSession { // may have been removed/hidden/disabled while compaction ran. strictAgentResolution: followUp.strictAgentResolution, skipAiSettingsPersistence: followUp.skipAiSettingsPersistence, + // An explicit one-shot carried through compaction keeps bypassing class routing. + skipSkillModelRouting: followUp.skipSkillModelRouting, + // A raw numeric thinking index re-resolves against the routed model if + // this re-dispatched send gets class-routed. + oneShotThinkingIndex: followUp.oneShotThinkingIndex, }; if (effectiveFileParts && effectiveFileParts.length > 0) { @@ -7358,8 +8309,15 @@ export class AgentSession { // before sendQueuedMessages() runs, preventing race conditions. // Mark as synthetic so recovery/background dispatches do not implicitly // re-enable auto-retry after a user explicitly opted out. + // Acceptance boundary marker for the failure branch below: once the + // session accepted the send, the user row is durable and emitted — any + // later startup failure must not re-preserve (duplicate) it. + let followUpAccepted = false; const sendResult = await this.sendMessage(finalText, options, { synthetic: true, + onAccepted: () => { + followUpAccepted = true; + }, agentInitiated: followUp.agentInitiated, goalKind: persistedGoalKind, // Keep the re-dispatched continuation row goal-scoped so a replaced @@ -7388,6 +8346,51 @@ export class AgentSession { return false; } const message = this.extractRetryFailureMessage(sendResult.error) ?? sendResult.error.type; + if (followUpAccepted) { + // The user row is already durable and emitted (post-acceptance + // startup failure): re-preserving would duplicate it. The pre-armed + // resume state above owns recovery; clear the marker so a later + // stream-end cannot dispatch the same follow-up again on top of the + // durable row. + await this.clearPendingFollowUpFromSummary(lastMessage); + throw new Error(`Failed to dispatch pending follow-up: ${message}`); + } + // Only genuinely user-authored follow-ups get manual rejection + // recovery: heartbeat prompts and mid-stream compaction's "Continue" + // sentinel ride the same pendingFollowUp field, but no user typed them + // — persisting them as manual rows (and pausing a goal) would fabricate + // an intervention. + const userAuthoredFollowUp = + persistedGoalKind == null && + (options.muxMetadata as MuxMessageMetadata | undefined)?.type !== "heartbeat-request" && + // Persisted provenance, not content matching: a user who literally + // typed "Continue" must keep manual recovery, while the generated + // mid-stream resume sentinel carries dispatchOptions.source. + followUp.dispatchOptions?.source !== "internal-resume"; + if (userAuthoredFollowUp) { + // The follow-up text is the USER's prompt (their composer cleared + // when compaction started), redispatched synthetically — so the + // routing/pricing rejection branches inside sendMessage skip their + // manual-send preservation for it, and a bare throw reaches only the + // logs while the summary keeps re-arming the same failing dispatch. + const persisted = await this.preserveRejectedManualSend( + finalText, + options, + sendResult.error + ); + if (persisted) { + await this.applyManualUserMessageGoalSafety({ policy: "pause" }); + // The preserved row is the durable copy now; only then may the + // summary's pending marker go. A failed preservation KEEPS the + // marker — it is the only durable copy of the prompt, and the next + // stream-end/idle pass re-attempts the dispatch. + await this.clearPendingFollowUpFromSummary(lastMessage); + } + } else { + // Synthetic content: nothing user-visible to preserve; drop the + // marker so the same failing dispatch cannot loop. + await this.clearPendingFollowUpFromSummary(lastMessage); + } throw new Error(`Failed to dispatch pending follow-up: ${message}`); } @@ -7411,6 +8414,19 @@ export class AgentSession { } } + // Dispatch-time attribution for a compaction-DEFERRED routed skill: the + // original send reported { queued: true } (the renderer's messageSent + // deliberately skipped), so the routed turn is attributed here, when it + // actually streams. + if (sendResult.data?.routedModel != null) { + await this.captureBackendMessageSent({ + model: sendResult.data.routedModel, + agentId: options.agentId, + messageLength: finalText.length, + thinkingLevel: sendResult.data.routedThinkingLevel, + }); + } + return true; } @@ -7865,18 +8881,479 @@ export class AgentSession { return snapshots.filter((snapshot): snapshot is MuxMessage => snapshot !== null); } + /** + * Build a reader that resolves a skill package with the same roots and + * precedence as skill discovery for this workspace. Shared by snapshot + * materialization and per-skill model routing so both resolve identically. + */ + private buildSkillReader(args: { + metadata: WorkspaceMetadata; + runtime: Runtime; + workspacePath: string; + disableWorkspaceAgents: boolean | undefined; + }): (skillName: string) => Promise>> { + // When workspace agents are disabled, resolve skills from the project path instead of + // the worktree so skill invocation uses the same precedence/discovery root as the UI. + const skillDiscoveryPath = args.disableWorkspaceAgents + ? args.metadata.projectPath + : args.workspacePath; + + // claude-skills-compat experiment: resolve slash-invoked skills with the same + // roots as discovery. Guard for test mocks that may not implement the gate. + const includeClaudeSkills = + typeof this.aiService.isClaudeSkillsCompatEnabled === "function" && + this.aiService.isClaudeSkillsCompatEnabled(); + // agent-plugins experiment: same treatment for plugin-provided skills. + const includeAgentPlugins = + typeof this.aiService.isAgentPluginsEnabled === "function" && + this.aiService.isAgentPluginsEnabled(); + // Resolve project workspaces through the same storage context as the + // skill tools so subprojects inherit checkout-level skills and plugins + // across host-local and runtime-backed workspaces. disableWorkspaceAgents + // keeps default projectPath discovery. + const xumScope = + !args.disableWorkspaceAgents && + typeof this.aiService.resolveXumToolScopeForWorkspace === "function" + ? this.aiService.resolveXumToolScopeForWorkspace( + args.metadata, + args.runtime, + args.workspacePath + ) + : null; + const skillCtx = + xumScope?.type === "project" + ? resolveSkillStorageContext({ + runtime: args.runtime, + workspacePath: skillDiscoveryPath, + xumScope, + includeClaudeSkills, + includeAgentPlugins, + }) + : null; + return (skillName: string) => + readAgentSkill( + skillCtx?.runtime ?? args.runtime, + skillCtx?.workspacePath ?? skillDiscoveryPath, + skillName, + { + ...(skillCtx != null ? { roots: skillCtx.roots, containment: skillCtx.containment } : {}), + includeClaudeSkills, + includeAgentPlugins, + } + ); + } + + /** + * Per-skill model routing: a slash-invoked skill bound to a model class + * (config `skillModelClasses` table, else skill frontmatter metadata + * "model-class") streams on the class's model for this send only. + * + * Explicit overrides win: sends carrying skipAiSettingsPersistence (one-shot + * /model commands, compaction requests) are never re-routed. Workspace AI + * settings are untouched: persistence happens in WorkspaceService (with the + * user's model) before this runs. + * + * Error posture: a *bound* skill whose routing cannot be delivered — unknown + * class, malformed class value, or a class model no configured route can + * serve — returns a config-error so the send fails with an actionable + * message instead of silently streaming on an unintended (often expensive) + * model. Unbound skills route nothing, and infrastructure failures (config + * or skill unreadable, providers state unavailable) still fail open: those + * are not user mapping mistakes, and a skill send must survive them. + */ + private async resolveSkillModelClassOverride( + muxMetadata: MuxMessageMetadata | undefined, + options: SendMessageOptions + ): Promise< + | { + kind: "override"; + className: string; + model: string; + thinkingLevel?: ThinkingLevel; + /** + * The scope-checked package this routing consent was granted against + * (present whenever the resolver read one — always in untrusted + * projects). Materialization reuses it so a project shadow appearing + * between routing and the snapshot read cannot swap repo-controlled + * content into a class-provider turn. + */ + resolvedPackage?: ResolvedAgentSkill; + } + | { kind: "config-error"; message: string } + | null + > { + // Only an explicit model override suppresses routing. This must NOT key + // off skipAiSettingsPersistence: thinking-only one-shots (/+2 /skill) and + // several internal senders set that flag purely to protect persisted + // preferences and still want class routing to apply. + if (options.skipSkillModelRouting === true) { + return null; + } + if (muxMetadata?.type !== "agent-skill") { + return null; + } + + try { + // Defensive config access mirroring getPreferredCompactionSettings: test + // harnesses may provide a partial Config. + const maybeConfig = this.config as Config & { + loadConfigOrDefault?: () => { + modelClasses?: Record; + skillModelClasses?: Record; + routePriority?: string[]; + routeOverrides?: Record; + } | null; + }; + if (typeof maybeConfig.loadConfigOrDefault !== "function") { + return null; + } + const cfg = maybeConfig.loadConfigOrDefault(); + const modelClasses = cfg?.modelClasses; + const skillModelClasses = cfg?.skillModelClasses; + + const skillName = muxMetadata.skillName; + if (!SkillNameSchema.safeParse(skillName).success) { + return null; + } + + // Fast path: with no classes configured and no table binding for this + // skill, routing can never apply — skip the (possibly remote) SKILL.md + // frontmatter read entirely. The non-empty-after-trim requirement must + // match resolveSkillModelClassBinding's boundViaTable exactly: a blank + // hand-edited table entry ({done: ""}) must not suppress the frontmatter + // read and then fail the table lookup, silently unrouting the skill. + const hasModelClasses = modelClasses != null && Object.keys(modelClasses).length > 0; + const tableClassRaw = skillModelClasses?.[skillName]; + const hasTableBinding = typeof tableClassRaw === "string" && tableClassRaw.trim().length > 0; + if (!hasModelClasses && !hasTableBinding) { + return null; + } + + // Security: repo-controlled content must not silently reroute the + // transcript to a different configured provider — an attacker's + // repository could bind its skill to a class the user pointed at any + // provider. Project Trust is the existing consent boundary for + // repo-controlled configuration, so in an UNTRUSTED project a + // project-scope skill gets no class routing at all: neither its own + // frontmatter nor a name-keyed skillModelClasses entry — the table + // consent belongs to the (global/built-in) skill the user knew by that + // name, and project skills win name collisions, so a repo shadow would + // otherwise inherit it. Global/built-in skills are user-authored and + // route normally. Fail closed when trust cannot be determined. + if (typeof this.aiService.getWorkspaceMetadata !== "function") { + return null; + } + const metadataResult = await this.aiService.getWorkspaceMetadata(this.workspaceId); + if (!metadataResult.success) { + return null; + } + const projectTrusted = (() => { + try { + // Scratch workspaces are app-trusted for capability purposes + // (isWorkspaceProjectTrusted returns true by design), but their + // workdirs routinely hold CLONED third-party repositories whose + // .xum/skills ARE discovered — for provider-selection consent a + // scratch checkout is exactly the untrusted-repository case, so + // scratch project skills never route. Global/built-in skills still + // route normally there. + if (metadataResult.data.kind === "scratch") { + return false; + } + return isWorkspaceProjectTrusted(this.config, metadataResult.data); + } catch { + return false; + } + })(); + + // Package resolution (a possibly remote SKILL.md read) is ALWAYS + // performed: the resolved package is the consent anchor. Its + // AUTHORITATIVE scope gates the trust decision here AND the mid-send + // revocation rechecks (client-supplied invocation scope must never + // gate a security decision), and materialization reuses the exact + // package via preResolvedSkills so no shadow can swap content in + // between. A trusted table binding previously skipped this read, which + // left the routed invocation unidentifiable at recheck time. + const { runtime, workspacePath } = createRuntimeContextForWorkspace(metadataResult.data); + const resolved = await this.buildSkillReader({ + metadata: metadataResult.data, + runtime, + workspacePath, + disableWorkspaceAgents: options.disableWorkspaceAgents, + })(skillName); + if (resolved.package.scope === "project" && !projectTrusted) { + return null; + } + const consentCheckedPackage = resolved; + // Table bindings take precedence; frontmatter feeds the binding + // resolver only when no table entry names this skill. + const frontmatterMetadata = hasTableBinding + ? undefined + : resolved.package.frontmatter.metadata; + + const providersConfig = this.getProvidersConfigSafe(); + const binding = resolveSkillModelClassBinding({ + skillName, + frontmatterMetadata, + modelClasses, + skillModelClasses, + providersConfig, + }); + + switch (binding.status) { + case "unbound": + return null; + case "unknown-class": + return { + kind: "config-error", + message: describeSkillModelClassRoutingProblem({ + kind: "unknown-class", + skillName, + className: binding.className, + }), + }; + case "invalid-value": + return { + kind: "config-error", + message: describeSkillModelClassRoutingProblem({ + kind: "invalid-value", + skillName, + className: binding.className, + value: binding.value, + }), + }; + case "resolved": { + // Availability is a routing-state question (gateways count: a model + // can be servable via OpenRouter without a direct provider key). + // Null providersConfig means "cannot determine", never "unavailable". + if ( + providersConfig != null && + !isModelServableWithProvidersConfig({ + canonicalModel: binding.model, + routePriority: cfg?.routePriority, + routeOverrides: cfg?.routeOverrides, + providersConfig, + }) + ) { + return { + kind: "config-error", + message: describeSkillModelClassRoutingProblem({ + kind: "model-unavailable", + skillName, + className: binding.className, + model: binding.model, + }), + }; + } + + log.debug( + `skill model routing: /${skillName} → class "${binding.className}" → ${binding.model}` + + (binding.thinkingLevel != null ? `+${binding.thinkingLevel}` : "") + ); + return { + kind: "override", + className: binding.className, + model: binding.model, + ...(binding.thinkingLevel != null ? { thinkingLevel: binding.thinkingLevel } : {}), + resolvedPackage: consentCheckedPackage, + }; + } + } + } catch (error) { + log.debug(`skill model routing: fail-open for skill send: ${getErrorMessage(error)}`); + return null; + } + } + + /** + * Fresh provider-selection consent verdict for a routed project-skill + * turn. Read IMMEDIATELY before each irreversible step (edit truncation, + * snapshot materialization, snapshot persistence): consent granted at the + * routing gate can be revoked mid-send. Fails closed — an unreadable + * verdict must not ship repo-controlled content to the class provider. + * Same scratch rule as resolveSkillModelClassOverride: scratch workdirs + * hold cloned third-party repositories and never carry this consent. + */ + private async isRoutedProjectSkillTurnStillTrusted(): Promise { + try { + if (typeof this.aiService.getWorkspaceMetadata !== "function") { + return false; + } + const metadataResult = await this.aiService.getWorkspaceMetadata(this.workspaceId); + if (!metadataResult.success) { + return false; + } + if (metadataResult.data.kind === "scratch") { + return false; + } + return isWorkspaceProjectTrusted(this.config, metadataResult.data); + } catch { + return false; + } + } + + /** + * Backend message_sent attribution for dispatches whose renderer telemetry + * was deliberately suppressed (busy-queued and compaction-deferred skill + * sends report { queued: true } before routing is known). Telemetry must + * never fail the dispatch. + */ + private async captureBackendMessageSent(args: { + model: string; + agentId?: string; + messageLength: number; + thinkingLevel?: ThinkingLevel; + }): Promise { + if (this.telemetryService == null) { + return; + } + try { + const metadataResult = + typeof this.aiService.getWorkspaceMetadata === "function" + ? await this.aiService.getWorkspaceMetadata(this.workspaceId) + : null; + const runtimeType = + metadataResult?.success === true && metadataResult.data.runtimeConfig?.type != null + ? metadataResult.data.runtimeConfig.type + : "local"; + this.telemetryService.capture({ + event: "message_sent", + properties: { + workspaceId: this.workspaceId, + model: args.model, + agentId: args.agentId, + message_length_b2: roundToBase2(args.messageLength), + runtimeType, + // Backend-originated event: there is no renderer to describe. + frontendPlatform: { userAgent: "backend", platform: process.platform }, + thinkingLevel: args.thinkingLevel ?? "off", + }, + }); + } catch (error) { + log.debug("Failed to capture backend message_sent telemetry", { + workspaceId: this.workspaceId, + error: getErrorMessage(error), + }); + } + } + + /** + * Rows quarantined after a failed durable rejection stamp: side-channel + * model calls (refine) exclude them like request assembly does. + */ + getQuarantinedRejectedRowIds(): ReadonlySet { + return this.unstampedRejectedRowIds; + } + + /** + * Self-healing for a late-gate rejection whose durable row stamp FAILED + * (transient rewrite error; the in-memory quarantine died with the + * process): the abandon marker still names the rejected user row — + * re-attempt the stamp for the row AND its turn's snapshot rows (skill, + * MCP prompt, @file — persisted immediately before the user row) so the + * whole rejected turn goes provider-ineligible together. Runs regardless + * of the auto-retry preference: the hazard is the next MANUAL send. + */ + private async repairUnstampedRejectedTurn(): Promise { + const abandon = this.startupAutoRetryAbandon; + if (abandon?.reason !== "pre_stream_rejected") { + return; + } + try { + // The rejected turn's in-flight assistant may still sit in + // partial.json: its delete can fail at rejection time, and the + // in-memory quarantine died with the process. A marker still present + // at startup means no send succeeded since the rejection (any accepted + // manual send clears it), so a surviving partial belongs to the + // rejected turn — remove it before any request-build path commits it + // as an unmarked assistant row. Runs even when the row stamp itself + // succeeded (the two failures are independent). + const rejectedPartial = await this.historyService.readPartial(this.workspaceId); + if (rejectedPartial != null) { + const deletePartialResult = await this.historyService.deletePartial(this.workspaceId); + if (!deletePartialResult.success) { + this.unstampedRejectedRowIds.add(rejectedPartial.id); + } + } + if (abandon.userMessageId == null) { + return; + } + // Full active epoch, not a bounded tail: a turn's synthetic snapshot + // prefix (one row per distinct skill/MCP/@file ref) has no count limit, + // and a truncated read would stamp only the newest subset. + const historyResult = await this.historyService.getHistoryFromLatestBoundary( + this.workspaceId + ); + if (!historyResult.success) { + return; + } + const rows = historyResult.data; + const userIdx = rows.findIndex((msg) => msg.id === abandon.userMessageId); + if (userIdx === -1 || rows[userIdx].metadata?.preStreamRejected === true) { + return; + } + const restampIds = [rows[userIdx].id]; + // Snapshot rows persist immediately before their user row; walk the + // contiguous synthetic snapshot prefix. + for (let i = userIdx - 1; i >= 0; i--) { + const metadata = rows[i].metadata; + if ( + metadata?.synthetic === true && + (metadata.agentSkillSnapshot != null || + metadata.mcpPromptSnapshot != null || + metadata.fileAtMentionSnapshot != null) + ) { + restampIds.push(rows[i].id); + continue; + } + break; + } + const restamp = await this.historyService.markMessagesPreStreamRejected( + this.workspaceId, + restampIds + ); + if (!restamp.success) { + for (const id of restampIds) { + this.unstampedRejectedRowIds.add(id); + } + } + } catch (error) { + log.warn("Failed to repair unstamped rejected turn on startup", { + workspaceId: this.workspaceId, + error: getErrorMessage(error), + }); + } + } + private async materializeAgentSkillSnapshots( muxMetadata: MuxMessageMetadata | undefined, - disableWorkspaceAgents: boolean | undefined - ): Promise { + disableWorkspaceAgents: boolean | undefined, + // Routing consent binds to a specific resolved package: reuse it here so + // a project shadow appearing between routing and this snapshot read + // cannot swap repo-controlled content into a class-provider turn. + preResolvedSkills?: Map, + // True when this turn streams on a routed class model: EVERY + // repository-controlled snapshot in it needs the provider-selection + // consent gate, not just the slash-invoked package (an inline + // $project-skill ref would otherwise ride the routed request). + routedTurn?: boolean, + // Edit turns materialize BEFORE truncation: recent-snapshot dedupe would + // compare against rows the truncation is about to delete and wrongly + // suppress a snapshot the rewritten history needs. + skipRecentSnapshotDedupe?: boolean + // carriesProjectSkillContent: whether any project-scope skill content + // (fresh or deduped-into-history) rides this routed turn — the later + // consent gates must fire even when the routed invocation itself is + // global/built-in but an inline $project-skill ref travels with it. + ): Promise<{ messages: MuxMessage[]; carriesProjectSkillContent: boolean }> { + const none = { messages: [], carriesProjectSkillContent: false }; const refs = extractAgentSkillRefs(muxMetadata); if (refs.length === 0) { - return []; + return none; } // Guard for test mocks that may not implement getWorkspaceMetadata. if (typeof this.aiService.getWorkspaceMetadata !== "function") { - return []; + return none; } const metadataResult = await this.aiService.getWorkspaceMetadata(this.workspaceId); @@ -7885,21 +9362,21 @@ export class AgentSession { if (hasSlash) { throw new Error("Cannot materialize agent skill: workspace metadata not found"); } - return []; + return none; } const metadata = metadataResult.data; const { runtime, workspacePath } = createRuntimeContextForWorkspace(metadata); - - // When workspace agents are disabled, resolve skills from the project path instead of - // the worktree so skill invocation uses the same precedence/discovery root as the UI. - const skillDiscoveryPath = disableWorkspaceAgents ? metadata.projectPath : workspacePath; + const trustedForRoutedSnapshots = + routedTurn === true ? await this.isRoutedProjectSkillTurnStillTrusted() : true; // Dedupe per skill against recent persisted snapshots. A wider window keeps multi-skill // turns from reloading snapshots that were persisted together on the previous turn. const recentSnapshots: Array<{ skillName: string; sha256: string }> = []; - const historyResult = await this.historyService.getLastMessages(this.workspaceId, 10); - if (historyResult.success) { + const historyResult = skipRecentSnapshotDedupe + ? null + : await this.historyService.getLastMessages(this.workspaceId, 10); + if (historyResult?.success) { for (const msg of historyResult.data) { const metadata = msg.metadata; if (metadata?.synthetic && metadata.agentSkillSnapshot) { @@ -7912,6 +9389,14 @@ export class AgentSession { } const snapshotMessages: MuxMessage[] = []; + // Tracked for the post-loop trust revalidation below. Every resolved + // project-scope ref counts (slash or inline), recorded BEFORE dedupe: a + // deduped snapshot still means repo-controlled content rides the routed + // request via history. + const projectScopeSnapshotIds = new Set(); + let projectScopeRefSeen = false; + let routedSlashProjectSkillSeen = false; + let dedupedProjectScopeRefSeen = false; for (const ref of refs) { const parsedName = SkillNameSchema.safeParse(ref.skillName); if (!parsedName.success) { @@ -7922,56 +9407,58 @@ export class AgentSession { } let resolved: Awaited>; - try { - // claude-skills-compat experiment: resolve slash-invoked skills with the same - // roots as discovery. Guard for test mocks that may not implement the gate. - const includeClaudeSkills = - typeof this.aiService.isClaudeSkillsCompatEnabled === "function" && - this.aiService.isClaudeSkillsCompatEnabled(); - // agent-plugins experiment: same treatment for plugin-provided skills. - const includeAgentPlugins = - typeof this.aiService.isAgentPluginsEnabled === "function" && - this.aiService.isAgentPluginsEnabled(); - // Resolve project workspaces through the same storage context as the - // skill tools so subprojects inherit checkout-level skills and plugins - // across host-local and runtime-backed workspaces. disableWorkspaceAgents - // keeps default projectPath discovery. - const xumScope = - !disableWorkspaceAgents && - typeof this.aiService.resolveXumToolScopeForWorkspace === "function" - ? this.aiService.resolveXumToolScopeForWorkspace(metadata, runtime, workspacePath) - : null; - const skillCtx = - xumScope?.type === "project" - ? resolveSkillStorageContext({ - runtime, - workspacePath: skillDiscoveryPath, - xumScope, - includeClaudeSkills, - includeAgentPlugins, - }) - : null; - resolved = await readAgentSkill( - skillCtx?.runtime ?? runtime, - skillCtx?.workspacePath ?? skillDiscoveryPath, - parsedName.data, - { - ...(skillCtx != null - ? { roots: skillCtx.roots, containment: skillCtx.containment } - : {}), - includeClaudeSkills, - includeAgentPlugins, + const preResolved = preResolvedSkills?.get(parsedName.data); + if (preResolved != null) { + resolved = preResolved; + } else { + try { + resolved = await this.buildSkillReader({ + metadata, + runtime, + workspacePath, + disableWorkspaceAgents, + })(parsedName.data); + } catch (error) { + if (ref.source === "slash") { + throw error; } - ); - } catch (error) { - if (ref.source === "slash") { - throw error; + continue; } - continue; } const skill = resolved.package; + if (routedTurn === true && skill.scope === "project") { + projectScopeRefSeen = true; + if (ref.source === "slash") { + routedSlashProjectSkillSeen = true; + } + } + + // Routed turns stream to the class provider: an untrusted project + // skill's snapshot must not ride along. + if (!trustedForRoutedSnapshots && skill.scope === "project") { + if (ref.source === "slash") { + // The slash ref IS the routed invocation (identified by source, + // not by preResolved presence — a trusted table binding resolves + // its package right here): revocation between the routing gate and + // materialization means the class route itself is no longer + // authorized, and the route stays on the turn regardless of + // snapshot omission. Reject before any row persists; a re-send + // resolves routing against the revoked trust and proceeds + // unrouted. + throw new Error(ROUTED_SKILL_TRUST_REVOKED_MESSAGE); + } + // Inline refs are subject to the same rule — the snapshot is omitted + // rather than failing the turn (least privilege, and the invoked + // skill's own content still dispatches). + log.warn("Omitting untrusted project skill snapshot from routed turn", { + workspaceId: this.workspaceId, + skillName: skill.directoryName, + }); + continue; + } + // Slash invocations can carry trailing argument text (e.g. "/fix-issue 123 high"). // Substitute $ARGUMENTS/$1..$9 placeholders in the snapshot body so the model sees // the resolved instructions; bodies without placeholders stay byte-identical and the @@ -8022,6 +9509,12 @@ export class AgentSession { (recent) => recent.skillName === skill.frontmatter.name && recent.sha256 === sha256 ) ) { + if (routedTurn === true && skill.scope === "project") { + // The recent snapshot this dedupes against rides the routed + // request via history — omission cannot exclude it, so the + // post-loop revalidation must treat it like the invocation. + dedupedProjectScopeRefSeen = true; + } continue; } @@ -8039,13 +9532,39 @@ export class AgentSession { }, }) ); + if (skill.scope === "project") { + projectScopeSnapshotIds.add(snapshotId); + } // Defense-in-depth: avoid double-loading this skill within the same turn even if // future metadata shapes bypass extractAgentSkillRefs dedupe. recentSnapshots.push({ skillName: skill.frontmatter.name, sha256 }); } - return snapshotMessages; + // The loop above awaits (remote SKILL.md reads, dynamic context + // injection): trust can be revoked WHILE those ran, after the pre-loop + // verdict was taken. Revalidate after the last await, immediately before + // these snapshots are returned for persistence. The routed invocation — + // and any deduped project snapshot, which already rides history and + // cannot be omitted — rejects the turn; fresh incidental inline + // snapshots are dropped. + if (routedTurn === true && projectScopeRefSeen) { + const stillTrusted = await this.isRoutedProjectSkillTurnStillTrusted(); + if (!stillTrusted) { + if (routedSlashProjectSkillSeen || dedupedProjectScopeRefSeen) { + throw new Error(ROUTED_SKILL_TRUST_REVOKED_MESSAGE); + } + log.warn("Dropping project skill snapshots after mid-materialization trust revocation", { + workspaceId: this.workspaceId, + }); + return { + messages: snapshotMessages.filter((msg) => !projectScopeSnapshotIds.has(msg.id)), + carriesProjectSkillContent: false, + }; + } + } + + return { messages: snapshotMessages, carriesProjectSkillContent: projectScopeRefSeen }; } /** diff --git a/src/node/services/agentSkills/builtInSkillContent.generated.ts b/src/node/services/agentSkills/builtInSkillContent.generated.ts index 6c87cd129be..53df420f87f 100644 --- a/src/node/services/agentSkills/builtInSkillContent.generated.ts +++ b/src/node/services/agentSkills/builtInSkillContent.generated.ts @@ -2129,6 +2129,47 @@ export const BUILTIN_SKILL_FILES: Record> = { "", "Use the `argument-hint` frontmatter field to document the expected arguments in invocation UIs.", "", + "## Per-skill model routing", + "", + "Mechanical skills (session wrap-up, worktree helpers, PR chores) rarely need your frontier model. Skill invocations can be routed to a **model class** — an indirection that survives model churn, since bindings name a class and only the class map names concrete models.", + "", + "Configure the three canonical classes — `large`, `medium`, `small` — in **Settings → Models → Model Classes** (a model plus an optional thinking level per class). Canonical names keep skill bindings portable across machines. The classes are stored in `~/.xum/config.json`, where values use the [one-shot override syntax](/config/models#one-shot-overrides): a model alias or full `provider:model` id, with an optional `+thinking` suffix (named level or model-relative numeric index). Hand-edited custom class names in config.json also work and are preserved by the Settings editor:", + "", + "```json", + "{", + ' "modelClasses": {', + ' "large": "fable+max",', + ' "medium": "sonnet+high",', + ' "small": "haiku+0"', + " }", + "}", + "```", + "", + "Bind skills to classes in either of two places:", + "", + "- **Skill frontmatter** — the spec-standard `metadata` map, so the binding travels with the skill and other agent tools ignore it:", + "", + " ```yaml", + " metadata:", + " model-class: small", + " ```", + "", + " For project skills, class routing only applies in **trusted projects** — repo-controlled content must not silently reroute your conversation to a different configured provider. In an untrusted project a project skill is never routed: not by its frontmatter, and not by a `skillModelClasses` entry either, since project skills win name collisions and a repo-shipped shadow would otherwise inherit a binding you meant for your own skill. Global and built-in skills route normally.", + "", + "- **Config routing table** — for skills you don't own, `skillModelClasses` in `~/.xum/config.json` maps skill names to classes and **wins over frontmatter**:", + "", + " ```json", + " {", + ' "skillModelClasses": { "done": "small", "wt": "small" }', + " }", + " ```", + "", + "Routing applies to the slash invocation's send only: the workspace's selected model is untouched, and your next message streams on it again. If auto-compaction triggers, the threshold is computed against the routed model's context window, and the compaction request starts from whichever of the two models — the workspace's or the routed one — has the larger context window (it must read the full uncompacted history). As with any compaction, settings configured for the Compact agent take precedence over that base.", + "", + 'Broken bindings fail loudly: when a bound class exists but its value is malformed, or no configured provider route can serve its model (a retired model, a removed provider or key), the send fails with an error naming the mapping to fix — and the Model Classes editor shows the same "no configured route" warning inline. A dangling `skillModelClasses` table entry (naming a class you deleted) also errors, since the table is your own explicit routing intent. Frontmatter bindings to a class you never defined are simply ignored, so skills you don\'t own can ship `model-class` metadata without ever breaking your sends; infrastructure hiccups (an unreadable skill or config) likewise fall back to the workspace model instead of failing the send.', + "", + "To override routing for one invocation, compose a one-shot prefix with the skill: `/sonnet+high /done` runs the skill on Sonnet regardless of its class. A model-carrying one-shot always wins over class routing; a thinking-only one-shot (`/+2 /done`) layers on top of it — the skill still routes to its class model, at the overridden thinking level. Numeric thinking indices are model-relative and resolve against the model that actually streams: in `/+0 /done`, the `0` means the class model's lowest allowed level, not the workspace model's. Both overrides survive compact-and-retry: the rebuilt send keeps the one-shot's model and thinking instead of falling back to routing or ambient settings.", + "", "## Dynamic context injection (experiment)", "", "Enable the **Skill dynamic context injection** experiment (Settings → Experiments) to let skills pull live command output into their instructions. When you invoke a skill, any line whose entire content is `` !`command` `` runs in the workspace, and the line is replaced with a fenced block containing the command’s output before the model sees the skill:", @@ -3903,12 +3944,15 @@ export const BUILTIN_SKILL_FILES: Record> = { "", "### Syntax", "", - "| Command | Effect |", - "| --------------------------- | ---------------------------------------- |", - "| `/sonnet explain this code` | Use Sonnet for one message |", - "| `/opus+high deep review` | Use Opus with high thinking |", - "| `/haiku+0 quick answer` | Use Haiku at its lowest thinking level |", - "| `/+2 analyze this` | Keep current model, set thinking level 2 |", + "| Command | Effect |", + "| --------------------------- | ------------------------------------------- |", + "| `/sonnet explain this code` | Use Sonnet for one message |", + "| `/opus+high deep review` | Use Opus with high thinking |", + "| `/haiku+0 quick answer` | Use Haiku at its lowest thinking level |", + "| `/+2 analyze this` | Keep current model, set thinking level 2 |", + "| `/haiku+0 /done` | Run the `done` skill on Haiku for this send |", + "", + "One-shot prefixes compose with [skill invocations](/agents/agent-skills): `/haiku+0 /done cleanup` invokes the skill normally (arguments, snapshots) while overriding the model for that send. An explicit one-shot also wins over the skill's own [model-class routing](/agents/agent-skills#per-skill-model-routing).", "", "### Thinking levels", "", diff --git a/src/node/services/aiService.ts b/src/node/services/aiService.ts index 5ce958eca2c..16eca5a0307 100644 --- a/src/node/services/aiService.ts +++ b/src/node/services/aiService.ts @@ -898,6 +898,12 @@ export class AIService extends EventEmitter { return buildOutcome.result; } + // Routed project-skill turns: the consent gate rides + // turnExecutionOptions into StreamManager.startStream, which invokes + // it inside its critical section (mutex held, safety and temp-dir + // setup done) immediately before the provider stream is constructed — + // checking here would leave that section as a revocation window. Its + // rejection surfaces below as a failed stream start. const startStreamStartedAt = Date.now(); const streamResult = await this.streamManager.startStream(buildOutcome.turnExecutionOptions); recordStartupPhaseTiming("startStreamMs", startStreamStartedAt); diff --git a/src/node/services/branchSummary.ts b/src/node/services/branchSummary.ts index 13c82364393..109252e8dfd 100644 --- a/src/node/services/branchSummary.ts +++ b/src/node/services/branchSummary.ts @@ -20,7 +20,11 @@ import type { LanguageModelV2Usage } from "@ai-sdk/provider"; import { EXPERIMENT_IDS, type ExperimentId } from "@/common/constants/experiments"; import { buildCompactionPrompt } from "@/common/constants/ui"; -import { createMuxMessage, type MuxMessage } from "@/common/types/message"; +import { + filterPreStreamRejectedRows, + createMuxMessage, + type MuxMessage, +} from "@/common/types/message"; import type { WorkspaceMetadata } from "@/common/types/workspace"; import * as fs from "node:fs/promises"; import * as path from "node:path"; @@ -147,7 +151,13 @@ function formatMessageForBranchTranscript(message: MuxMessage): string { */ export function buildAbandonedBranchTranscript(messages: MuxMessage[]): string { assert(Array.isArray(messages), "buildAbandonedBranchTranscript requires a message array"); - const formatted = messages.map(formatMessageForBranchTranscript).filter((s) => s.length > 0); + // Same exclusion as main request assembly: rows preserved by pre-stream + // gate rejections are transcript-only — every model call (refine, RLM + // branch summaries) must skip them or the rejected prompt gets distilled + // into durable context anyway. + const formatted = filterPreStreamRejectedRows(messages) + .map(formatMessageForBranchTranscript) + .filter((s) => s.length > 0); let totalChars = formatted.reduce((sum, s) => sum + s.length, 0); let drop = 0; diff --git a/src/node/services/compactionHandler.test.ts b/src/node/services/compactionHandler.test.ts index aceaea6e861..538eb25e78a 100644 --- a/src/node/services/compactionHandler.test.ts +++ b/src/node/services/compactionHandler.test.ts @@ -1860,7 +1860,10 @@ describe("CompactionHandler", () => { await seedHistory( createMuxMessage("u0", "user", "old head question"), createMuxMessage("a0", "assistant", "old head answer"), - createMuxMessage("u1", "user", "tail question"), + // preStreamRejected rides the copy: the boundary hides the original + // row, so a marker-less copy would become provider-eligible and + // re-send the very prompt whose gate rejection preserved it. + createMuxMessage("u1", "user", "tail question", { preStreamRejected: true }), tailAssistant, // seedHistory assigns sequences 0..4; the tail starts at u1 (seq 2). createStampedCompactionRequest("compact-req", 2) @@ -1881,6 +1884,8 @@ describe("CompactionHandler", () => { // History round-trips normalize parts (adds state markers), so compare content. expect(epoch[1].parts).toMatchObject([{ type: "text", text: "tail question" }]); expect(epoch[2].parts).toMatchObject([{ type: "text", text: "tail answer" }]); + expect(epoch[1].metadata?.preStreamRejected).toBe(true); + expect(epoch[2].metadata?.preStreamRejected).toBeUndefined(); for (const copy of epoch.slice(1)) { // Fresh IDs + durable marker, UI-hidden synthetic. diff --git a/src/node/services/compactionHandler.ts b/src/node/services/compactionHandler.ts index 21fe8e7438d..08398735cee 100644 --- a/src/node/services/compactionHandler.ts +++ b/src/node/services/compactionHandler.ts @@ -1426,6 +1426,10 @@ export class CompactionHandler { ...(source?.agentSkillSnapshot !== undefined ? { agentSkillSnapshot: source.agentSkillSnapshot } : {}), + // A rejected row's copy must stay provider-ineligible: the boundary + // hides the original row, so a marker-less copy would re-send the + // very prompt whose pre-stream gate rejection preserved it. + ...(source?.preStreamRejected === true ? { preStreamRejected: true } : {}), ...(mcpPromptSnapshot !== undefined ? { mcpPromptSnapshot } : {}), }, }; diff --git a/src/node/services/compactionMonitor.test.ts b/src/node/services/compactionMonitor.test.ts index 0f84fb6a33b..0e7c520a1e1 100644 --- a/src/node/services/compactionMonitor.test.ts +++ b/src/node/services/compactionMonitor.test.ts @@ -111,6 +111,40 @@ describe("CompactionMonitor", () => { expect(statusEvents).toHaveLength(1); }); + test("checkMidStream honors the routed-send force threshold override", () => { + const { monitor, statusEvents } = createMonitor(); + + // 75% would force-compact under the workspace threshold+buffer, but a + // routed turn's override defers until the routed window is nearly full. + expect( + monitor.checkMidStream({ + model: BETA_SONNET_MODEL, + usage: createMidStreamUsage(150_000), + use1MContext: false, + providersConfig: null, + forceThresholdPercentOverride: 90, + }) + ).toBe(false); + expect(statusEvents).toHaveLength(0); + + expect( + monitor.checkMidStream({ + model: BETA_SONNET_MODEL, + usage: createMidStreamUsage(184_000), + use1MContext: false, + providersConfig: null, + forceThresholdPercentOverride: 90, + }) + ).toBe(true); + expect(statusEvents).toEqual([ + { + type: "auto-compaction-triggered", + reason: "mid-stream", + usagePercent: 92, + }, + ]); + }); + test("checkMidStream stays disabled when threshold is set to 1.0", () => { const { monitor, statusEvents } = createMonitor(); monitor.setThreshold(1); diff --git a/src/node/services/compactionMonitor.ts b/src/node/services/compactionMonitor.ts index b4f6c10b77e..5773cee76b6 100644 --- a/src/node/services/compactionMonitor.ts +++ b/src/node/services/compactionMonitor.ts @@ -35,6 +35,13 @@ interface CheckMidStreamParams { usage: LanguageModelV2Usage; use1MContext: boolean; providersConfig: ProvidersConfigMap | null; + /** + * Replaces the workspace threshold+buffer force bar for this check. Skill + * class routing passes the routed-send policy (compact only near the routed + * window's limit) so a mid-stream usage update can't force the workspace-wide + * compaction the routed pre-send band deliberately avoided. + */ + forceThresholdPercentOverride?: number; } /** @@ -119,7 +126,9 @@ export class CompactionMonitor { ); const usagePercent = (usageTokens / contextLimit) * 100; - const forceThresholdPercent = this.threshold * 100 + FORCE_COMPACTION_BUFFER_PERCENT; + const forceThresholdPercent = + params.forceThresholdPercentOverride ?? + this.threshold * 100 + FORCE_COMPACTION_BUFFER_PERCENT; if (usagePercent < forceThresholdPercent) { return false; diff --git a/src/node/services/historyService.ts b/src/node/services/historyService.ts index bdc9a09973f..b65d9a4b1f1 100644 --- a/src/node/services/historyService.ts +++ b/src/node/services/historyService.ts @@ -2787,6 +2787,44 @@ export class HistoryService { * edit-resend) can summarize the abandoned segment; computed under the * history lock so it exactly matches what was cut. */ + /** + * Stamp preStreamRejected onto already-persisted rows (the accepted user + * row and its snapshot rows) when a late consent gate rejects the turn + * post-acceptance: request construction filters on the ROW metadata, so a + * sidecar marker alone would leave the rejected turn provider-eligible. + * Missing ids are skipped (a concurrent truncation is not an error). + */ + async markMessagesPreStreamRejected( + workspaceId: string, + messageIds: string[] + ): Promise> { + if (messageIds.length === 0) { + return Ok(undefined); + } + return this.withRecoveredHistoryWriteResultLock( + workspaceId, + "Failed to mark rejected messages", + async () => { + const ids = new Set(messageIds); + const messages = await this.readChatHistory(workspaceId); + let changed = false; + const updated = messages.map((msg) => { + if (!ids.has(msg.id) || msg.metadata?.preStreamRejected === true) { + return msg; + } + changed = true; + return { ...msg, metadata: { ...msg.metadata, preStreamRejected: true as const } }; + }); + if (!changed) { + return Ok(undefined); + } + const historyPath = this.getChatHistoryPath(workspaceId); + await writeFileAtomic(historyPath, this.serializeHistoryEntries(updated, workspaceId)); + return Ok(undefined); + } + ); + } + async truncateAfterMessage( workspaceId: string, messageId: string, diff --git a/src/node/services/memoryConsolidationService.ts b/src/node/services/memoryConsolidationService.ts index b536e2b8e56..772abb6e8bd 100644 --- a/src/node/services/memoryConsolidationService.ts +++ b/src/node/services/memoryConsolidationService.ts @@ -56,6 +56,7 @@ import { parseAgentDefinitionMarkdown } from "@/node/services/agentDefinitions/p import { log } from "@/node/services/log"; import type { HistoryService } from "@/node/services/historyService"; import { runMemoryHarvest } from "@/node/services/memoryHarvest"; +import { filterPreStreamRejectedRows } from "@/common/types/message"; import { runMemoryConsolidation } from "@/node/services/memoryConsolidation"; import type { MemoryScopeContext, MemoryService } from "@/node/services/memoryService"; import { memoryLogicalKey, type MemoryMetaService } from "@/node/services/memoryMeta"; @@ -342,6 +343,17 @@ export class MemoryConsolidationService extends EventEmitter { this.sidecarPath = path.join(config.rootDir, "memory-consolidation.json"); } + /** + * Session-local quarantine of rejected rows whose durable stamp failed: + * the harvest boundary must exclude them like request assembly does. + * Late-bound — WorkspaceService is constructed after core services. + */ + private getQuarantinedRowIds?: (workspaceId: string) => ReadonlySet; + + setQuarantinedRowIdsLookup(lookup: (workspaceId: string) => ReadonlySet): void { + this.getQuarantinedRowIds = lookup; + } + private enabled(): boolean { return ( this.experiments.isExperimentEnabled(EXPERIMENT_IDS.MEMORY) && @@ -807,6 +819,15 @@ export class MemoryConsolidationService extends EventEmitter { throw new Error(`could not create model ${modelString}: ${modelResult.error.type}`); } + // Rejected turns are transcript-only: the dream model (which may use + // an explicit alternate provider) must not harvest their prompt or + // repository snapshots — filter both stamped rows and the session's + // in-memory quarantine, exactly like provider request assembly. + const quarantinedRowIds = this.getQuarantinedRowIds?.(metadata.workspaceId); + const harvestMessages = filterPreStreamRejectedRows(epoch.data.messages).filter( + (msg) => !quarantinedRowIds?.has(msg.id) + ); + const harvest = await runMemoryHarvest({ model: modelResult.data.model, agentBody: @@ -814,7 +835,7 @@ export class MemoryConsolidationService extends EventEmitter { memoryService: this.memoryService, ctx, completionMetadata: metadata, - messages: epoch.data.messages, + messages: harvestMessages, summary: epoch.data.summary, // Timeout + removal (r60); see the runLocked signal for rationale. abortSignal: AbortSignal.any([ diff --git a/src/node/services/providerModelFactory.ts b/src/node/services/providerModelFactory.ts index c7161b17cb7..22283a92457 100644 --- a/src/node/services/providerModelFactory.ts +++ b/src/node/services/providerModelFactory.ts @@ -1122,7 +1122,8 @@ export class ProviderModelFactory { private isProviderAvailableForRouting( provider: ProviderName, providersConfig: ProvidersConfig, - config: ReturnType + config: ReturnType, + canonicalModel?: string ): boolean { const rawProviderConfig = providersConfig[provider] ?? {}; const providerConfig = @@ -1141,6 +1142,34 @@ export class ProviderModelFactory { return false; } + // Model-aware OpenAI gate, mirroring createModel's credential outcome: a + // Codex-OAuth-only credential serves only the OAuth-allowed model set, so + // direct OpenAI must not win the route for a model it would then reject + // with api_key_not_found — a usable gateway later in routePriority (or the + // caller's availability error) should win instead. Matches the shared + // canDirectOpenAIServeModel predicate used by availability preflights. + if ( + provider === "openai" && + canonicalModel != null && + !credentials.isConfigured && + !isCodexOauthAllowedModel(canonicalModel, providersConfig) + ) { + return false; + } + + // Same mirror for the wire format: Codex OAuth speaks only the Responses + // endpoint, so an OAuth-only credential cannot serve a provider pinned to + // chatCompletions — createModel rejects it with api_key_not_found, and a + // usable gateway later in routePriority should win instead (matches + // canDirectOpenAIServeModel). + if ( + provider === "openai" && + !credentials.isConfigured && + (providerConfig as { wireFormat?: unknown }).wireFormat === "chatCompletions" + ) { + return false; + } + // Route resolution must honor the shared provider-level enabled=false switch // before considering legacy gateway-specific config gates. if (isProviderDisabledInConfig(providerConfig as { enabled?: unknown })) { @@ -2790,7 +2819,8 @@ export class ProviderModelFactory { return this.isProviderAvailableForRouting( provider as ProviderName, providersConfig, - config + config, + canonicalModel ); }, isGatewayModelAccessible @@ -2894,7 +2924,10 @@ export class ProviderModelFactory { return this.isProviderAvailableForRouting( provider as ProviderName, providersConfig, - config + config, + typeof modelKeyOrRouteContext === "string" + ? normalizeToCanonical(modelKeyOrRouteContext) + : canonicalModelString ); }, isGatewayModelAccessible diff --git a/src/node/services/refinement/refineService.ts b/src/node/services/refinement/refineService.ts index 85403777b8e..43972a25fbe 100644 --- a/src/node/services/refinement/refineService.ts +++ b/src/node/services/refinement/refineService.ts @@ -129,6 +129,12 @@ interface RefineServiceOptions { sessionUsageService?: Pick; /** Live-session emission hook so the appended summary row renders immediately. */ emitChatMessage?: (workspaceId: string, message: MuxMessage) => void; + /** + * Session-local quarantine of rejected rows whose durable preStreamRejected + * stamp failed: refine's side-channel model call must exclude them exactly + * like provider request assembly does. + */ + getQuarantinedRowIds?: (workspaceId: string) => ReadonlySet; /** * Serialize refine row publication (and apply mutations) with the * workspace's turn lifecycle (r40): returns a disposable holding the @@ -935,7 +941,10 @@ export class RefineService { if (!messagesResult.success) { return Err(`could not read workspace history: ${messagesResult.error}`); } - const activeSegment = sliceMessagesForProviderFromLatestContextBoundary(messagesResult.data); + const quarantinedRowIds = this.options.getQuarantinedRowIds?.(workspaceId); + const activeSegment = sliceMessagesForProviderFromLatestContextBoundary( + messagesResult.data + ).filter((msg) => !quarantinedRowIds?.has(msg.id)); // r47: fingerprint the snapshot rows for the pre-publication recheck. // Row IDs alone cannot detect same-ID rewrites: StreamManager finalizes // a streaming assistant row through updateHistory() PRESERVING its ID @@ -1172,7 +1181,12 @@ export class RefineService { const recheckBoundaryIndex = findLatestContextBoundaryIndex(recheckResult.data); const recheckBoundaryId = recheckBoundaryIndex >= 0 ? recheckResult.data[recheckBoundaryIndex].id : null; - const recheckSegment = sliceMessagesForProviderFromLatestContextBoundary(recheckResult.data); + const recheckSegment = sliceMessagesForProviderFromLatestContextBoundary( + recheckResult.data + // Same quarantine filter as the segment above: an unfiltered recheck + // mismatches at the quarantined row and deterministically refuses to + // publish after the model call was already spent. + ).filter((msg) => !quarantinedRowIds?.has(msg.id)); const snapshotIsUnchangedPrefix = activeSegment.length <= recheckSegment.length && snapshotRowFingerprints.every( diff --git a/src/node/services/serviceContainer.ts b/src/node/services/serviceContainer.ts index 77cd9b14453..d1cb0acf934 100644 --- a/src/node/services/serviceContainer.ts +++ b/src/node/services/serviceContainer.ts @@ -317,6 +317,8 @@ export class ServiceContainer { sessionUsageService: this.sessionUsageService, emitChatMessage: (workspaceId, message) => this.workspaceService.emitChatEvent(workspaceId, { ...message, type: "message" }), + getQuarantinedRowIds: (workspaceId) => + this.workspaceService.getQuarantinedRejectedRowIds(workspaceId), // r40: refine row publication and apply mutations must not interleave // with a concurrent turn's PREPARING snapshot or split its // user/assistant pair — hold the session's turn-admission block while @@ -515,6 +517,11 @@ export class ServiceContainer { // without any chat activity; ingest promptly so background sweeps reach // dashboard totals instead of stranding until an unrelated stream-end // or app restart. + // Rejected-row quarantine reaches the dream-harvest boundary (r-consent): + // WorkspaceService owns the per-session sets, constructed above. + this.memoryConsolidationService.setQuarantinedRowIdsLookup((workspaceId) => + this.workspaceService.getQuarantinedRejectedRowIds(workspaceId) + ); this.memoryConsolidationService.on("analyticsIngest", (event: { workspaceId: string }) => { ingestWorkspaceAnalytics(event.workspaceId); }); diff --git a/src/node/services/streamManager.ts b/src/node/services/streamManager.ts index 864b093ceb3..950073e3788 100644 --- a/src/node/services/streamManager.ts +++ b/src/node/services/streamManager.ts @@ -57,6 +57,7 @@ import type { ProvidersConfigMap } from "@/common/orpc/types"; import { coerceStreamErrorTypeForMessage, createErrorEvent, + formatSendMessageError, stripNoisyErrorPrefix, type StreamErrorPayload, } from "@/node/services/utils/sendMessageError"; @@ -233,6 +234,15 @@ export function createTurnCompletionController(): TurnCompletionController { // Request-construction options shared by the primary turn and model-fallback // hops (fallbacks rebuild these from the prepared fallback request). interface StreamRequestOptions { + /** + * Routed project-skill turns: final consent verdict for EVERY provider + * request of this turn. Invoked in the stream-start critical section and + * again in prepareStep immediately before each step's provider call + * (fallback and retry recreations inherit it via the request config). + * Returns the error to surface (null = proceed); rejection bookkeeping + * happens inside the callback. + */ + preDispatchConsentGate?: () => Promise; model: LanguageModel; modelString: string; messages: ModelMessage[]; @@ -279,6 +289,8 @@ interface StepMessageTracker { interface StreamRequestConfig { model: LanguageModel; messages: ModelMessage[]; + /** Per-step consent verdict for routed project-skill turns (see TurnExecutionOptions). */ + preDispatchConsentGate?: () => Promise; /** Provider-ready system instructions from TurnContextAssembler. */ system?: string | SystemModelMessage; tools?: Record; @@ -2018,6 +2030,7 @@ export class StreamManager { forcedFirstStepToolNames, providersConfigSnapshot, rebuildFirstStepForThinkingLevel, + preDispatchConsentGate, } = input; // The request's pinned providers-config snapshot keeps type-derived output limits // aligned with the config that created the SDK model. @@ -2046,6 +2059,7 @@ export class StreamManager { maxOutputTokens ?? configMaxOutputTokens ?? resolvedModelStats?.max_output_tokens; return { + preDispatchConsentGate, model, messages, system, @@ -2254,6 +2268,19 @@ export class StreamManager { }); } } + // Routed project-skill turns: per-attempt consent verdict at the + // provider-call boundary — prepareStep is the last awaited hook + // before EVERY step's provider request (fallback and retry + // recreations included), so mid-turn revocation stops the next + // request instead of riding the stream. The callback already + // performed its rejection bookkeeping; throwing surfaces through + // the stream's standard error path. + if (request.preDispatchConsentGate) { + const consentError = await request.preDispatchConsentGate(); + if (consentError) { + throw new Error(formatSendMessageError(consentError).message); + } + } if ( rewritten === stepMessages && activeTools === undefined && @@ -3096,6 +3123,10 @@ export class StreamManager { headers: prepared.data.headers, onChunk: streamInfo.request.onChunk, onStepMessages: streamInfo.request.onStepMessages, + // The fallback attempt ships the same routed project-skill turn: the + // per-step consent gate must ride along or the fallback provider gets + // the content with no verdict. + preDispatchConsentGate: streamInfo.request.preDispatchConsentGate, // Same state object: aiService's fallback prepare() rebuilt it in place // against the fallback toolset, so prepareStep keeps reading live state. toolSearchState: streamInfo.request.toolSearchState, @@ -4725,6 +4756,20 @@ export class StreamManager { return settleStartupAbort(); } + // Routed project-skill turns: final consent verdict inside the + // critical section — the mutex wait, ensureStreamSafety, and + // temp-dir creation above were the last revocation windows. Nothing + // awaitable remains between this check and provider dispatch. + if (options.preDispatchConsentGate) { + const consentError = await options.preDispatchConsentGate(); + if (consentError) { + return Err(consentError); + } + if (streamAbortController.signal.aborted) { + return settleStartupAbort(); + } + } + // Step 4: Atomic stream creation and registration const streamInfo = this.createStreamAtomically(options, { streamToken, diff --git a/src/node/services/taskWorkspaceSeam.ts b/src/node/services/taskWorkspaceSeam.ts index 61ba8f67b77..03c49344967 100644 --- a/src/node/services/taskWorkspaceSeam.ts +++ b/src/node/services/taskWorkspaceSeam.ts @@ -4,7 +4,7 @@ import type { ExperimentId } from "@/common/constants/experiments"; import type { GoalSyntheticMessageKind } from "@/constants/goals"; import type { ArchivePreflightResult, ArchiveWorkspaceResult } from "@/common/orpc/schemas/api"; import type { FilePart, SendMessageOptions, WorkspaceChatMessage } from "@/common/orpc/types"; -import type { SendMessageError } from "@/common/types/errors"; +import type { SendMessageAccepted, SendMessageError } from "@/common/types/errors"; import type { MuxMessage, MuxMessageMetadata, @@ -88,6 +88,14 @@ export interface WorkspaceLiveActivity { } export interface SendMessageInternalOptions { + /** + * True when this send is a QUEUE-DISPATCHED entry (sendQueuedMessages): + * pre-stream gate rejections preserve the user row only then — the + * composer already cleared on queue accept, so the row is the only + * record. A direct renderer send gets its Err back and restores the + * draft; preserving would double-record the prompt. + */ + dequeued?: boolean; allowQueuedAgentTask?: boolean; skipAutoResumeReset?: boolean; synthetic?: boolean; @@ -148,7 +156,7 @@ export interface WorkspaceTurnHost { message: string, options: SendMessageOptions & { fileParts?: FilePart[] }, internal?: SendMessageInternalOptions - ): Promise>; + ): Promise>; resumeStream( workspaceId: string, options: SendMessageOptions, diff --git a/src/node/services/turnRequestBuilder.ts b/src/node/services/turnRequestBuilder.ts index 1af75a84e54..28613526294 100644 --- a/src/node/services/turnRequestBuilder.ts +++ b/src/node/services/turnRequestBuilder.ts @@ -231,6 +231,14 @@ export interface StreamMessageOptions { messages: MuxMessage[]; workspaceId: string; modelString: string; + /** + * Routed project-skill turns: last consent check before the provider + * operation starts. Invoked by AIService immediately before + * streamManager.startStream — request building is the final revocation + * window. Performs its own rejection bookkeeping and returns the error to + * surface (null = proceed). Never sourced from IPC schemas. + */ + preDispatchConsentGate?: () => Promise; thinkingLevel?: ThinkingLevel; /** OpenAI pro reasoning mode; delivered via provider options (inert for unsupported models). */ reasoningMode?: OpenAIReasoningMode; @@ -423,7 +431,9 @@ interface WorkflowResultContinuationSender { requireIdle?: boolean; startStreamInBackground?: boolean; } - ): Promise>; + // The continuation sender ignores the accepted-send payload; unknown keeps + // this structural type compatible with WorkspaceService.sendMessage. + ): Promise>; } interface TurnRequestBuildStartupState { @@ -2829,6 +2839,11 @@ export class TurnRequestBuilder { emitStartupBreadcrumb("starting_stream"); const turnExecutionOptions: TurnExecutionOptions = { workspaceId, + // Threaded to the stream-start critical section (see + // TurnExecutionOptions.preDispatchConsentGate). + ...(opts.preDispatchConsentGate != null + ? { preDispatchConsentGate: opts.preDispatchConsentGate } + : {}), messages: streamFinalMessages, model: modelResult.data.model, modelString, diff --git a/src/node/services/utils/sendMessageError.ts b/src/node/services/utils/sendMessageError.ts index b8148b2a5ca..a9a95beec53 100644 --- a/src/node/services/utils/sendMessageError.ts +++ b/src/node/services/utils/sendMessageError.ts @@ -28,6 +28,19 @@ export const stripNoisyErrorPrefix = (message: string): string => { * Helper to wrap arbitrary errors into SendMessageError structures. * Enforces that the raw string is non-empty for defensive debugging. */ +/** + * Rejection surfaced when Project Trust is revoked between the routing + * gate's consent check and one of the turn's irreversible steps (edit + * truncation, snapshot persistence, provider dispatch). Lives here — not in + * agentSession — so StreamManager's per-step consent gate can reference it + * without an import cycle. Shared by every recheck site so the user sees one + * consistent, actionable message. + */ +export const ROUTED_SKILL_TRUST_REVOKED_MESSAGE = + "Project trust was revoked while this send was being prepared; the skill " + + "was not dispatched to its routed model. Re-send to run it on the current " + + "model without routing."; + export const createUnknownSendMessageError = (raw: string): SendMessageError => { assert(typeof raw === "string", "Expected raw error to be a string"); const trimmed = stripNoisyErrorPrefix(raw.trim()); diff --git a/src/node/services/workspaceOperations.ts b/src/node/services/workspaceOperations.ts index 19ad2e51ee8..0435a02d53a 100644 --- a/src/node/services/workspaceOperations.ts +++ b/src/node/services/workspaceOperations.ts @@ -237,7 +237,21 @@ export async function sendWorkspaceMessage( input.options ); return result.success - ? { success: true as const, data: {} } + ? { + success: true as const, + // Routed skill sends report the class model (and effective thinking) + // so the frontend can attribute send telemetry to what actually + // streams instead of the workspace's selected model. `queued` marks + // acknowledgements taken BEFORE routing resolved (busy session), so + // absence of routedModel there means "unknown", not "unrouted". + data: { + ...(result.data?.routedModel != null ? { routedModel: result.data.routedModel } : {}), + ...(result.data?.routedThinkingLevel != null + ? { routedThinkingLevel: result.data.routedThinkingLevel } + : {}), + ...(result.data?.queued === true ? { queued: true } : {}), + }, + } : { success: false as const, error: result.error }; } diff --git a/src/node/services/workspaceService.test.ts b/src/node/services/workspaceService.test.ts index 838cc828bac..b27afd60d97 100644 --- a/src/node/services/workspaceService.test.ts +++ b/src/node/services/workspaceService.test.ts @@ -13293,6 +13293,55 @@ describe("WorkspaceService sendMessage status clearing", () => { ); }); + test("skill sends defer the service pricing preflight to the routing-aware session gate", async () => { + fakeSession.isBusy.mockReturnValue(false); + const pricingError: SendMessageError = { type: "unknown", raw: "unpriced model" }; + const assertPriced = mock(() => Promise.resolve(Err(pricingError))); + workspaceService.setWorkspaceGoalService({ + assertPricedModelForBudgetedGoal: assertPriced, + } as unknown as WorkspaceGoalService); + const persistMock = ( + workspaceService as unknown as { + maybePersistAISettingsFromOptions: ReturnType; + } + ).maybePersistAISettingsFromOptions; + fakeSession.sendMessage.mockImplementation( + async ( + _message: unknown, + _options: unknown, + internalArg?: { onAccepted?: () => unknown } + ) => { + // AI settings must not persist before the session's dispatch-time + // gates — an unbound skill would reject the ambient model AFTER a + // premature persist had already stored it. + expect(persistMock).not.toHaveBeenCalled(); + await internalArg?.onAccepted?.(); + return Ok(undefined); + } + ); + + const result = await workspaceService.sendMessage("test-workspace", "/lint", { + model: "custom:unpriced-model", + agentId: "exec", + muxMetadata: { + type: "agent-skill", + rawCommand: "/lint", + skillName: "lint", + scope: "project", + }, + }); + + // Class routing resolves inside AgentSession (it needs the workspace's + // skill definitions), so the service must not reject on the ambient model + // before the route is known — the session's dispatch-time gate re-asserts + // pricing against the model that actually streams. + expect(result.success).toBe(true); + expect(assertPriced).not.toHaveBeenCalled(); + expect(fakeSession.sendMessage).toHaveBeenCalledTimes(1); + // Acceptance (all gates passed) is what triggers the deferred persist. + expect(persistMock).toHaveBeenCalledTimes(1); + }); + test("the follow-up idle probe excludes the originating send after its session handoff", async () => { // Codex P1 (PRRT_kwDOPxxmWM6cRi_J): preflightSendCounts stays positive // until the outer service call returns, so a probe reading it would let a diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index bda429c1ae9..81bf994961d 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -162,7 +162,7 @@ import type { } from "@/common/orpc/types"; import type { z } from "zod"; -import type { SendMessageError, StreamErrorType } from "@/common/types/errors"; +import type { SendMessageAccepted, SendMessageError, StreamErrorType } from "@/common/types/errors"; // Aliased to avoid clashing with the private `formatSendMessageError` string formatter below. import { formatSendMessageError as classifySendMessageError } from "@/node/services/utils/sendMessageError"; import type { IdleCompactionOutcome } from "@/node/services/idleCompactionService"; @@ -4896,6 +4896,11 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { this.sessions.get(trimmed)?.emitChatEvent(message); } + /** Session-local quarantine of rejected rows for refine's side-channel exclusion. */ + getQuarantinedRejectedRowIds(workspaceId: string): ReadonlySet { + return this.sessions.get(workspaceId)?.getQuarantinedRejectedRowIds() ?? new Set(); + } + /** Queued agent peer messages behind a busy workspace; sessions are lazy, so no session ⇒ 0. */ public countQueuedAgentPeerMessages(workspaceId: string): number { const trimmed = workspaceId.trim(); @@ -11310,7 +11315,7 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { fileParts?: FilePart[]; }, internal?: SendMessageInternalOptions - ): Promise> { + ): Promise> { log.debug("sendMessage handler: Received", { workspaceId, messagePreview: message.substring(0, 50), @@ -11514,10 +11519,78 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { // that bypass the client-side guard. Manual sends still delegate into // AgentSession on rejection so it can preserve the user's interruption // message and apply goal auto-pause safety. - const pricingGate = await this.assertPricedModelForBudgetedGoal( - workspaceId, - normalizedOptions - ); + // + // Skill sends defer this preflight: class routing resolves inside + // AgentSession (it needs the workspace's skill definitions), and the + // dispatch-time gate there re-asserts pricing against the model that + // will actually stream — the routed class model when routing applies. + // Gating here on the ambient model would reject a skill bound to a + // priced class just because the workspace model is unpriced. Deferral + // cannot corrupt stored settings: a composer skill send re-persists the + // already-selected workspace model, and one-shot prefixed sends skip + // persistence entirely. + const mayRouteViaSkill = normalizedMuxMetadata?.type === "agent-skill"; + if (mayRouteViaSkill) { + // AI-settings persistence is deferred with the gate (see below): a + // skill send whose routing does NOT apply at dispatch (unbound skill, + // skipSkillModelRouting) still carries the ambient model into + // AgentSession's own pricing gate, and persisting before that gate + // could store an unpriced model for a budgeted goal — the exact + // corruption assertPricedModelForBudgetedGoal exists to prevent. + // Acceptance fires only after every dispatch-time gate passed, and + // MessageQueue carries the callback through queued dispatch. + const callerOnAccepted = internal?.onAccepted; + // MessageQueue carries the acceptance callback until dispatch — an + // unbounded window in which the user may store a newer selection. + // Snapshot the persisted settings NOW and persist at acceptance only + // when they are unchanged, or the enqueue-time options would + // overwrite the newer choice. + const snapshotPersistedAiSettings = (): string => { + try { + for (const project of this.config.loadConfigOrDefault().projects.values()) { + const entry = project.workspaces.find((candidate) => candidate.id === workspaceId); + if (entry) { + return JSON.stringify({ + agentId: entry.agentId, + aiSettingsByAgent: entry.aiSettingsByAgent, + }); + } + } + } catch { + // Unreadable either time compares equal below → old behavior. + } + return "unavailable"; + }; + const settingsAtEnqueue = snapshotPersistedAiSettings(); + internal = { + ...internal, + onAccepted: async () => { + try { + if (snapshotPersistedAiSettings() === settingsAtEnqueue) { + await this.maybePersistAISettingsFromOptions( + workspaceId, + normalizedOptions, + "send" + ); + } + } catch (error) { + // Best-effort by contract: the send is already accepted (its + // user row is durable), so a persistence failure must not + // propagate through onAccepted and turn the accepted send into + // a partial failure (draft restored over a visible row; queued + // entry dropped without streaming). + log.debug("Failed to persist AI settings from accepted skill send", { + workspaceId, + error: getErrorMessage(error), + }); + } + await callerOnAccepted?.(); + }, + }; + } + const pricingGate = mayRouteViaSkill + ? Ok(undefined) + : await this.assertPricedModelForBudgetedGoal(workspaceId, normalizedOptions); if (!pricingGate.success) { if (internal?.synthetic !== true) { // Codex P1 (PRRT_kwDOPxxmWM6cSCjs): unlike the accepted handoffs @@ -11551,7 +11624,11 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { } // Persist last-used model + thinking level for cross-device consistency. - await this.maybePersistAISettingsFromOptions(workspaceId, normalizedOptions, "send"); + // Skill sends persist on session acceptance instead (see mayRouteViaSkill + // above), after the routing-aware gates validated the send. + if (!mayRouteViaSkill) { + await this.maybePersistAISettingsFromOptions(workspaceId, normalizedOptions, "send"); + } const shouldQueue = !normalizedOptions?.editMessageId && session.isBusy(); @@ -11709,7 +11786,10 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { this.agentTaskIntegration?.backgroundForegroundWaitsForWorkspace(workspaceId); } - return Ok(undefined); + // Queued: class routing has not resolved yet — the acknowledgement + // must say so, or the frontend would attribute the ambient model to a + // send that may dispatch on a routed class model. + return Ok({ queued: true }); } if (!internal?.skipAutoResumeReset) {