diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8889a0ec8..cf3e6eb4e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,6 +43,13 @@ jobs: - name: Install workspace run: bun install --frozen-lockfile + # Exercise the actual HTTP-400 -> second-model retitle path in addition + # to the cheaper pure candidate-ranking cases. + - name: Title recovery regressions + run: | + bun test ./packages/opencode/test/session/title-recovery-models.test.ts + bun test ./packages/opencode/test/server/httpapi-session.test.ts --test-name-pattern "regenerateTitle recovers on a second model" + - name: Lint run: bun run lint diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 46aa47c16..033b2e58b 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -27,6 +27,7 @@ import { TurnCheckpoint } from "./checkpoint" import { Session } from "./session" import { Agent } from "../agent/agent" import { Provider } from "@/provider/provider" +import { rankTitleRecoveryModels } from "./title-recovery-models" import { type ModelMessage, type Tool as AITool, tool, jsonSchema } from "ai" import type { JSONSchema7 } from "@ai-sdk/provider" @@ -697,7 +698,7 @@ const layer = Layer.effect( ...(input.route ? { route: input.route } : {}), }) const small = yield* provider.getSmallModel(input.providerID) - const candidates = [ + const configuredCandidates = [ input.model ? yield* resolve( input.model.providerID, @@ -719,19 +720,66 @@ const layer = Layer.effect( small ? yield* resolve(small.providerID, small.id) : undefined, yield* resolve(input.providerID, input.modelID), ].filter((item): item is MaintenanceModelResolution => item !== undefined) + const seen = new Set() - const uniqueCandidates = candidates.filter((item) => { - const key = [ + const candidateKey = (item: MaintenanceModelResolution) => + [ item.model.providerID, item.model.id, item.route?.routeKind ?? "legacy", item.route?.accountID ?? "", ].join("/") + const uniqueCandidates = configuredCandidates.filter((item) => { + const key = candidateKey(item) if (seen.has(key)) return false seen.add(key) return true }) const toolCandidates = uniqueCandidates.filter((item) => item.model.capabilities.toolcall) + + // Normal title generation must not perform extra catalog reads or route + // resolution. Only an exhausted candidate queue activates recovery. + // Stay within the original provider, inheriting the committed route when + // one exists; never silently choose an unrelated provider/account. + let recoveryLoaded = false + const loadRecovery = Effect.fn("SessionPrompt.loadTitleRecovery")(function* () { + if (recoveryLoaded) return + recoveryLoaded = true + const excluded = new Set( + configuredCandidates + .filter((candidate) => candidate.model.providerID === input.providerID) + .map((candidate) => String(candidate.model.id)), + ) + const recoveryModels = yield* provider.getProvider(input.providerID).pipe( + Effect.map((info) => + rankTitleRecoveryModels(Object.values(info.models).filter(Provider.isLanguageModel), excluded, 8), + ), + Effect.catchCause((cause) => + Effect.logWarning("title recovery catalog unavailable", { + sessionID: input.sessionID, + providerID: input.providerID, + error: String(Cause.squash(cause)), + }).pipe(Effect.as([] as Provider.Model[])), + ), + ) + const added: string[] = [] + for (const model of recoveryModels) { + const candidate = yield* resolve(input.providerID, model.id) + if (!candidate || !candidate.model.capabilities.toolcall) continue + const key = candidateKey(candidate) + if (seen.has(key)) continue + seen.add(key) + toolCandidates.push(candidate) + added.push(String(candidate.model.id)) + if (added.length === 2) break + } + yield* Effect.logInfo("title recovery candidates", { + sessionID: input.sessionID, + providerID: input.providerID, + models: added, + }) + }) + if (toolCandidates.length === 0) yield* loadRecovery() yield* Effect.logInfo("title generation candidates", { sessionID: input.sessionID, candidates: toolCandidates.map((item) => `${item.model.providerID}/${item.model.id}`), @@ -773,7 +821,8 @@ const layer = Layer.effect( text: titleRequestText, }) } - for (const candidate of toolCandidates) { + for (let index = 0; index < toolCandidates.length; index++) { + const candidate = toolCandidates[index]! const mdl = candidate.model const candidateRoute = candidate.route const candidateUsageRoute = usageRoute(candidateRoute) @@ -1050,6 +1099,7 @@ const layer = Layer.effect( ), ) if (title) return title + if (index === toolCandidates.length - 1) yield* loadRecovery() } yield* Effect.logWarning("all title model candidates failed", { sessionID: input.sessionID, diff --git a/packages/opencode/src/session/title-recovery-models.ts b/packages/opencode/src/session/title-recovery-models.ts new file mode 100644 index 000000000..ce86cdfcb --- /dev/null +++ b/packages/opencode/src/session/title-recovery-models.ts @@ -0,0 +1,41 @@ +/** + * Order bounded title-generation recovery models without changing provider/account + * ownership. The caller supplies only models from the original provider and + * resolves every choice against its committed provider route before execution. + * + * This is a last resort after explicitly configured models and the normal + * small-model selection. It must never expand into all configured providers. + */ +export function rankTitleRecoveryModels< + T extends { + readonly id: string + readonly status: string + readonly capabilities: { readonly toolcall: boolean } + readonly cost: { readonly input: number; readonly output: number } + }, +>( + models: readonly T[], + excluded: ReadonlySet, + limit = 2, +): T[] { + if (limit <= 0) return [] + return models + .filter( + (model) => + model.status === "active" && + model.capabilities.toolcall === true && + !excluded.has(model.id) && + Number.isFinite(model.cost.input) && + Number.isFinite(model.cost.output) && + model.cost.input >= 0 && + model.cost.output >= 0, + ) + .sort((left, right) => { + // A typical title request has more input than output tokens. Compare + // prices within the *same provider*; never select another billing domain. + const a = left.cost.input * 4 + left.cost.output + const b = right.cost.input * 4 + right.cost.output + return a - b || left.id.localeCompare(right.id) + }) + .slice(0, limit) +} diff --git a/packages/opencode/test/server/httpapi-session.test.ts b/packages/opencode/test/server/httpapi-session.test.ts index 7eb1d985b..d4d32fafd 100644 --- a/packages/opencode/test/server/httpapi-session.test.ts +++ b/packages/opencode/test/server/httpapi-session.test.ts @@ -52,7 +52,7 @@ import * as DateTime from "effect/DateTime" import { eq } from "drizzle-orm" import { resetDatabase } from "../fixture/db" import { disposeAllInstances, provideInstanceEffect, TestInstance, tmpdirScoped } from "../fixture/fixture" -import { TestLLMServer, reply } from "../lib/llm-server" +import { TestLLMServer, reply, httpError } from "../lib/llm-server" import { testProviderConfig } from "../lib/test-provider" import { pollWithTimeout, awaitWithTimeout, testEffect } from "../lib/effect" @@ -2475,6 +2475,73 @@ describe("session HttpApi", () => { 20_000, ) + it.live("regenerateTitle recovers on a second model after a provider HTTP 400", () => + Effect.gen(function* () { + const llm = yield* TestLLMServer + yield* llm.push( + httpError(400, { error: { type: "invalid_request_error", message: "Bad Request" } }), + reply().tool(SessionTitle.GENERATED_TITLE_TOOL, { title: "Fallback Title" }).item(), + ) + + const base = testProviderConfig(llm.url) + const config = { + ...base, + provider: { + test: { + ...base.provider.test, + models: { + ...base.provider.test.models, + "recovery-model": { + ...base.provider.test.models["test-model"], + id: "recovery-model", + name: "Recovery Model", + }, + }, + }, + }, + } + const directory = yield* tmpdirScoped({ git: true, config }) + const session = yield* createSession({ title: "Original Title" }).pipe(provideInstanceEffect(directory)) + const svc = yield* Session.Service + const message = yield* svc.updateMessage({ + id: MessageID.ascending(), + role: "user", + sessionID: session.id, + agent: "build", + model: { providerID: ProviderV2.ID.make("test"), modelID: ModelV2.ID.make("test-model") }, + time: { created: Date.now() }, + }) + yield* svc.updatePart({ + id: PartID.ascending(), + sessionID: session.id, + messageID: message.id, + type: "text", + text: "Recover the title without changing provider", + }) + + const response = yield* request(pathFor(SessionPaths.regenerateTitle, { sessionID: session.id }), { + method: "POST", + headers: { "x-opencode-directory": directory, "content-type": "application/json" }, + body: JSON.stringify({}), + }) + expect(response.status).toBe(204) + + const title = yield* pollWithTimeout( + Effect.gen(function* () { + const current = yield* Session.use.get(session.id).pipe(provideInstanceEffect(directory), Effect.orDie) + return current.title === "Original Title" ? undefined : current.title + }), + "title recovery failed after provider 400", + "10 seconds", + ) + expect(title).toBe("Fallback Title") + const inputs = yield* llm.inputs + expect(inputs).toHaveLength(2) + expect(inputs.map((body) => body.model)).toEqual(["test-model", "recovery-model"]) + }).pipe(Effect.provide(TestLLMServer.layer), Effect.provide(AppNodeBuilder.build(CrossSpawnSpawner.node))), + 20_000, + ) + it.live("regenerateTitle repairs prose in the same title-agent conversation", () => Effect.gen(function* () { const llm = yield* TestLLMServer diff --git a/packages/opencode/test/session/title-recovery-models.test.ts b/packages/opencode/test/session/title-recovery-models.test.ts new file mode 100644 index 000000000..a395f0c9d --- /dev/null +++ b/packages/opencode/test/session/title-recovery-models.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, test } from "bun:test" +import { rankTitleRecoveryModels } from "../../src/session/title-recovery-models" + +const model = ( + id: string, + input: number, + output: number, + status = "active", + toolcall = true, +) => ({ id, status, capabilities: { toolcall }, cost: { input, output } }) + +describe("title model recovery ranking", () => { + test("recovers from a deduplicated sole model with cheaper same-provider candidates", () => { + const models = [ + model("qwen3.8-flash", 0, 0), + model("other-paid", 0.1, 0.5), + model("other-free", 0, 0), + ] + const result = rankTitleRecoveryModels(models, new Set(["qwen3.8-flash"])) + expect(result.map((item) => item.id)).toEqual(["other-free", "other-paid"]) + }) + + test("rejects models that cannot provide the required terminal tool", () => { + const models = [ + model("unavailable", 0, 0, "deprecated"), + model("no-tools", 0, 0, "active", false), + model("eligible", 2, 3), + model("experimental", 0, 0, "beta"), + ] + expect(rankTitleRecoveryModels(models, new Set()).map((item) => item.id)).toEqual(["eligible"]) + }) + + test("has stable ordering on equal cost and strictly bounds fallback work", () => { + const models = [model("z", 1, 1), model("a", 1, 1), model("b", 1, 1)] + expect(rankTitleRecoveryModels(models, new Set(), 2).map((item) => item.id)).toEqual(["a", "b"]) + expect(rankTitleRecoveryModels(models, new Set(), 0)).toEqual([]) + }) + + test("does not mutate the provider catalog and excludes invalid prices", () => { + const models = [ + model("valid", 0.2, 1), + model("negative", -1, 0), + model("nan", Number.NaN, 0), + ] + const before = models.slice() + expect(rankTitleRecoveryModels(models, new Set()).map((item) => item.id)).toEqual(["valid"]) + expect(models).toEqual(before) + }) +})