diff --git a/src/features/background-agent/manager.test.ts b/src/features/background-agent/manager.test.ts index 11025c730..71d62f509 100644 --- a/src/features/background-agent/manager.test.ts +++ b/src/features/background-agent/manager.test.ts @@ -1668,7 +1668,7 @@ describe("BackgroundManager.resume model persistence", () => { // then - model should be passed in prompt body expect(promptCalls).toHaveLength(1) expect(promptCalls[0].body.model).toEqual({ providerID: "anthropic", modelID: "claude-sonnet-4-20250514" }) - expect("agent" in promptCalls[0].body).toBe(false) + expect(promptCalls[0].body.agent).toBe("explore") }) test("should NOT pass model when task has no model (backward compatibility)", async () => { @@ -1832,7 +1832,7 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { expect(task2.status).toBe("pending") }) - test("should omit agent when launch has model and keep agent without model", async () => { + test("should keep agent when launch has model and keep agent without model", async () => { // given const promptBodies: Array> = [] let resolveFirstPromptStarted: (() => void) | undefined @@ -1894,7 +1894,7 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { expect(taskWithoutModel.status).toBe("pending") expect(promptBodies).toHaveLength(2) expect(promptBodies[0].model).toEqual({ providerID: "anthropic", modelID: "claude-opus-4-6" }) - expect("agent" in promptBodies[0]).toBe(false) + expect(promptBodies[0].agent).toBe("test-agent") expect(promptBodies[1].agent).toBe("test-agent") expect("model" in promptBodies[1]).toBe(false) }) @@ -4752,6 +4752,53 @@ describe("BackgroundManager - tool permission spread order", () => { manager.shutdown() }) + test("startTask keeps agent when explicit model is configured", async () => { + //#given + const promptCalls: Array<{ path: { id: string }; body: Record }> = [] + const client = { + session: { + get: async () => ({ data: { directory: "/test/dir" } }), + create: async () => ({ data: { id: "session-1" } }), + promptAsync: async (args: { path: { id: string }; body: Record }) => { + promptCalls.push(args) + return {} + }, + }, + } + const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput) + const task: BackgroundTask = { + id: "task-explicit-model", + status: "pending", + queuedAt: new Date(), + description: "test task", + prompt: "test prompt", + agent: "sisyphus-junior", + parentSessionID: "parent-session", + parentMessageID: "parent-message", + model: { providerID: "openai", modelID: "gpt-5.4", variant: "medium" }, + } + const input: import("./types").LaunchInput = { + description: task.description, + prompt: task.prompt, + agent: task.agent, + parentSessionID: task.parentSessionID, + parentMessageID: task.parentMessageID, + model: task.model, + } + + //#when + await (manager as unknown as { startTask: (item: { task: BackgroundTask; input: import("./types").LaunchInput }) => Promise }) + .startTask({ task, input }) + + //#then + expect(promptCalls).toHaveLength(1) + expect(promptCalls[0].body.agent).toBe("sisyphus-junior") + expect(promptCalls[0].body.model).toEqual({ providerID: "openai", modelID: "gpt-5.4" }) + expect(promptCalls[0].body.variant).toBe("medium") + + manager.shutdown() + }) + test("resume respects explore agent restrictions", async () => { //#given let capturedTools: Record | undefined @@ -4796,4 +4843,48 @@ describe("BackgroundManager - tool permission spread order", () => { manager.shutdown() }) + + test("resume keeps agent when explicit model is configured", async () => { + //#given + let promptCall: { path: { id: string }; body: Record } | undefined + const client = { + session: { + promptAsync: async (args: { path: { id: string }; body: Record }) => { + promptCall = args + return {} + }, + abort: async () => ({}), + }, + } + const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput) + const task: BackgroundTask = { + id: "task-explicit-model-resume", + sessionID: "session-3", + parentSessionID: "parent-session", + parentMessageID: "parent-message", + description: "resume task", + prompt: "resume prompt", + agent: "explore", + status: "completed", + startedAt: new Date(), + completedAt: new Date(), + model: { providerID: "anthropic", modelID: "claude-sonnet-4-20250514" }, + } + getTaskMap(manager).set(task.id, task) + + //#when + await manager.resume({ + sessionId: "session-3", + prompt: "continue", + parentSessionID: "parent-session", + parentMessageID: "parent-message", + }) + + //#then + expect(promptCall).toBeDefined() + expect(promptCall?.body.agent).toBe("explore") + expect(promptCall?.body.model).toEqual({ providerID: "anthropic", modelID: "claude-sonnet-4-20250514" }) + + manager.shutdown() + }) }) diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index c858667a8..c4ea7528b 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -515,9 +515,7 @@ export class BackgroundManager { promptWithModelSuggestionRetry(this.client, { path: { id: sessionID }, body: { - // When a model is explicitly provided, omit the agent name so opencode's - // built-in agent fallback chain does not override the user-specified model. - ...(launchModel ? {} : { agent: input.agent }), + agent: input.agent, ...(launchModel ? { model: launchModel } : {}), ...(launchVariant ? { variant: launchVariant } : {}), system: input.skillContent, @@ -794,9 +792,7 @@ export class BackgroundManager { this.client.session.promptAsync({ path: { id: existingTask.sessionID }, body: { - // When a model is explicitly provided, omit the agent name so opencode's - // built-in agent fallback chain does not override the user-specified model. - ...(resumeModel ? {} : { agent: existingTask.agent }), + agent: existingTask.agent, ...(resumeModel ? { model: resumeModel } : {}), ...(resumeVariant ? { variant: resumeVariant } : {}), tools: (() => { diff --git a/src/features/background-agent/spawner.test.ts b/src/features/background-agent/spawner.test.ts index 27d26a519..0ff9b3f78 100644 --- a/src/features/background-agent/spawner.test.ts +++ b/src/features/background-agent/spawner.test.ts @@ -64,4 +64,63 @@ describe("background-agent spawner.startTask", () => { { permission: "question", action: "deny", pattern: "*" }, ]) }) + + test("keeps agent when explicit model is configured", async () => { + //#given + const promptCalls: any[] = [] + + const client = { + session: { + get: async () => ({ data: { directory: "/parent/dir" } }), + create: async () => ({ data: { id: "ses_child" } }), + promptAsync: async (args?: any) => { + promptCalls.push(args) + return {} + }, + }, + } + + const task = createTask({ + description: "Test task", + prompt: "Do work", + agent: "sisyphus-junior", + parentSessionID: "ses_parent", + parentMessageID: "msg_parent", + model: { providerID: "openai", modelID: "gpt-5.4", variant: "medium" }, + }) + + const item = { + task, + input: { + description: task.description, + prompt: task.prompt, + agent: task.agent, + parentSessionID: task.parentSessionID, + parentMessageID: task.parentMessageID, + parentModel: task.parentModel, + parentAgent: task.parentAgent, + model: task.model, + }, + } + + const ctx = { + client, + directory: "/fallback", + concurrencyManager: { release: () => {} }, + tmuxEnabled: false, + onTaskError: () => {}, + } + + //#when + await startTask(item as any, ctx as any) + + //#then + expect(promptCalls).toHaveLength(1) + expect(promptCalls[0]?.body?.agent).toBe("sisyphus-junior") + expect(promptCalls[0]?.body?.model).toEqual({ + providerID: "openai", + modelID: "gpt-5.4", + }) + expect(promptCalls[0]?.body?.variant).toBe("medium") + }) }) diff --git a/src/features/background-agent/spawner.ts b/src/features/background-agent/spawner.ts index a66720966..c4f435720 100644 --- a/src/features/background-agent/spawner.ts +++ b/src/features/background-agent/spawner.ts @@ -135,9 +135,7 @@ export async function startTask( promptWithModelSuggestionRetry(client, { path: { id: sessionID }, body: { - // When a model is explicitly provided, omit the agent name so opencode's - // built-in agent fallback chain does not override the user-specified model. - ...(launchModel ? {} : { agent: input.agent }), + agent: input.agent, ...(launchModel ? { model: launchModel } : {}), ...(launchVariant ? { variant: launchVariant } : {}), system: input.skillContent, @@ -222,9 +220,7 @@ export async function resumeTask( client.session.promptAsync({ path: { id: task.sessionID }, body: { - // When a model is explicitly provided, omit the agent name so opencode's - // built-in agent fallback chain does not override the user-specified model. - ...(resumeModel ? {} : { agent: task.agent }), + agent: task.agent, ...(resumeModel ? { model: resumeModel } : {}), ...(resumeVariant ? { variant: resumeVariant } : {}), tools: { diff --git a/src/features/mcp-oauth/callback-server.test.ts b/src/features/mcp-oauth/callback-server.test.ts index 060cdb580..063f2bdea 100644 --- a/src/features/mcp-oauth/callback-server.test.ts +++ b/src/features/mcp-oauth/callback-server.test.ts @@ -1,44 +1,112 @@ -import { afterEach, describe, expect, it } from "bun:test" +import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test" import { startCallbackServer, type CallbackServer } from "./callback-server" +const HOSTNAME = "127.0.0.1" const nativeFetch = Bun.fetch.bind(Bun) +function supportsRealSocketBinding(): boolean { + try { + const server = Bun.serve({ + port: 0, + hostname: HOSTNAME, + fetch: () => new Response("probe"), + }) + server.stop(true) + return true + } catch { + return false + } +} + +const canBindRealSockets = supportsRealSocketBinding() + +type MockServerState = { + port: number + stopped: boolean + fetch: (request: Request) => Response | Promise +} + describe("startCallbackServer", () => { let server: CallbackServer | null = null + let serveSpy: ReturnType | null = null + let activeServer: MockServerState | null = null + + async function request(url: string): Promise { + if (canBindRealSockets) { + return nativeFetch(url) + } + + if (!activeServer || activeServer.stopped) { + throw new Error("Connection refused") + } + + return await activeServer.fetch(new Request(url)) + } + + beforeEach(() => { + if (canBindRealSockets) { + return + } + + activeServer = null + serveSpy = spyOn(Bun, "serve").mockImplementation((options: { + port: number + hostname?: string + fetch: (request: Request) => Response | Promise + }) => { + const state: MockServerState = { + port: options.port === 0 ? 19877 : options.port, + stopped: false, + fetch: options.fetch, + } + + const handle = { + port: state.port, + stop: (_force?: boolean) => { + state.stopped = true + if (activeServer === state) { + activeServer = null + } + }, + } + + activeServer = state + return handle as ReturnType + }) + }) afterEach(async () => { server?.close() server = null - // Allow time for port to be released before next test - await Bun.sleep(10) + + if (serveSpy) { + serveSpy.mockRestore() + serveSpy = null + } + activeServer = null + + if (canBindRealSockets) { + await Bun.sleep(10) + } }) it("starts server and returns port", async () => { - // given - no preconditions - - // when server = await startCallbackServer() - // then expect(server.port).toBeGreaterThanOrEqual(19877) expect(typeof server.waitForCallback).toBe("function") expect(typeof server.close).toBe("function") }) it("resolves callback with code and state from query params", async () => { - // given server = await startCallbackServer() - const callbackUrl = `http://127.0.0.1:${server.port}/oauth/callback?code=test-code&state=test-state` + const callbackUrl = `http://${HOSTNAME}:${server.port}/oauth/callback?code=test-code&state=test-state` - // when - // Use Promise.all to ensure fetch and waitForCallback run concurrently - // This prevents race condition where waitForCallback blocks before fetch starts const [result, response] = await Promise.all([ server.waitForCallback(), - nativeFetch(callbackUrl) + request(callbackUrl), ]) - // then expect(result).toEqual({ code: "test-code", state: "test-state" }) expect(response.status).toBe(200) const html = await response.text() @@ -46,25 +114,19 @@ describe("startCallbackServer", () => { }) it("returns 404 for non-callback routes", async () => { - // given server = await startCallbackServer() - // when - const response = await nativeFetch(`http://127.0.0.1:${server.port}/other`) + const response = await request(`http://${HOSTNAME}:${server.port}/other`) - // then expect(response.status).toBe(404) }) it("returns 400 and rejects when code is missing", async () => { - // given server = await startCallbackServer() - const callbackRejection = server.waitForCallback().catch((e: Error) => e) + const callbackRejection = server.waitForCallback().catch((error: Error) => error) - // when - const response = await nativeFetch(`http://127.0.0.1:${server.port}/oauth/callback?state=s`) + const response = await request(`http://${HOSTNAME}:${server.port}/oauth/callback?state=s`) - // then expect(response.status).toBe(400) const error = await callbackRejection expect(error).toBeInstanceOf(Error) @@ -72,14 +134,11 @@ describe("startCallbackServer", () => { }) it("returns 400 and rejects when state is missing", async () => { - // given server = await startCallbackServer() - const callbackRejection = server.waitForCallback().catch((e: Error) => e) + const callbackRejection = server.waitForCallback().catch((error: Error) => error) - // when - const response = await nativeFetch(`http://127.0.0.1:${server.port}/oauth/callback?code=c`) + const response = await request(`http://${HOSTNAME}:${server.port}/oauth/callback?code=c`) - // then expect(response.status).toBe(400) const error = await callbackRejection expect(error).toBeInstanceOf(Error) @@ -87,18 +146,15 @@ describe("startCallbackServer", () => { }) it("close stops the server immediately", async () => { - // given server = await startCallbackServer() const port = server.port - // when server.close() server = null - // then try { - await nativeFetch(`http://127.0.0.1:${port}/oauth/callback?code=c&state=s`) - expect(true).toBe(false) + await request(`http://${HOSTNAME}:${port}/oauth/callback?code=c&state=s`) + expect.unreachable("request should fail after close") } catch (error) { expect(error).toBeDefined() } diff --git a/src/features/mcp-oauth/callback-server.ts b/src/features/mcp-oauth/callback-server.ts index c8d856fa8..48dcb1729 100644 --- a/src/features/mcp-oauth/callback-server.ts +++ b/src/features/mcp-oauth/callback-server.ts @@ -39,7 +39,7 @@ export async function findAvailablePort(startPort: number = DEFAULT_PORT): Promi } export async function startCallbackServer(startPort: number = DEFAULT_PORT): Promise { - const port = await findAvailablePort(startPort) + const requestedPort = await findAvailablePort(startPort).catch(() => 0) let resolveCallback: ((result: OAuthCallbackResult) => void) | null = null let rejectCallback: ((error: Error) => void) | null = null @@ -55,7 +55,7 @@ export async function startCallbackServer(startPort: number = DEFAULT_PORT): Pro }, TIMEOUT_MS) const server = Bun.serve({ - port, + port: requestedPort, hostname: "127.0.0.1", fetch(request: Request): Response { const url = new URL(request.url) @@ -93,9 +93,10 @@ export async function startCallbackServer(startPort: number = DEFAULT_PORT): Pro }) }, }) + const activePort = server.port ?? requestedPort return { - port, + port: activePort, waitForCallback: () => callbackPromise, close: () => { clearTimeout(timeoutId) diff --git a/src/shared/data-path.ts b/src/shared/data-path.ts index 28aa9a072..d46e6b1c6 100644 --- a/src/shared/data-path.ts +++ b/src/shared/data-path.ts @@ -1,5 +1,18 @@ import * as path from "node:path" import * as os from "node:os" +import { accessSync, constants, mkdirSync } from "node:fs" + +function resolveWritableDirectory(preferredDir: string, fallbackSuffix: string): string { + try { + mkdirSync(preferredDir, { recursive: true }) + accessSync(preferredDir, constants.W_OK) + return preferredDir + } catch { + const fallbackDir = path.join(os.tmpdir(), fallbackSuffix) + mkdirSync(fallbackDir, { recursive: true }) + return fallbackDir + } +} /** * Returns the user-level data directory. @@ -10,7 +23,8 @@ import * as os from "node:os" * including Windows, so we match that behavior exactly. */ export function getDataDir(): string { - return process.env.XDG_DATA_HOME ?? path.join(os.homedir(), ".local", "share") + const preferredDir = process.env.XDG_DATA_HOME ?? path.join(os.homedir(), ".local", "share") + return resolveWritableDirectory(preferredDir, "opencode-data") } /** @@ -27,7 +41,8 @@ export function getOpenCodeStorageDir(): string { * - All platforms: XDG_CACHE_HOME or ~/.cache */ export function getCacheDir(): string { - return process.env.XDG_CACHE_HOME ?? path.join(os.homedir(), ".cache") + const preferredDir = process.env.XDG_CACHE_HOME ?? path.join(os.homedir(), ".cache") + return resolveWritableDirectory(preferredDir, "opencode-cache") } /** diff --git a/src/tools/delegate-task/sync-prompt-sender.test.ts b/src/tools/delegate-task/sync-prompt-sender.test.ts index d7e0eb0e3..39bf6bd6f 100644 --- a/src/tools/delegate-task/sync-prompt-sender.test.ts +++ b/src/tools/delegate-task/sync-prompt-sender.test.ts @@ -165,6 +165,55 @@ bunDescribe("sendSyncPrompt", () => { bunExpect(promptArgs.body.tools.call_omo_agent).toBe(true) }) + bunTest("includes agent alongside explicit category model", async () => { + //#given + const { sendSyncPrompt } = require("./sync-prompt-sender") + + let promptArgs: any + const promptAsync = bunMock(async (input: any) => { + promptArgs = input + return { data: {} } + }) + + const mockClient = { + session: { + promptAsync, + }, + } + + const input = { + sessionID: "test-session", + agentToUse: "sisyphus-junior", + args: { + description: "test task", + prompt: "test prompt", + category: "quick", + run_in_background: false, + load_skills: [], + }, + systemContent: undefined, + categoryModel: { + providerID: "openai", + modelID: "gpt-5.4", + variant: "medium", + }, + toastManager: null, + taskId: undefined, + } + + //#when + await sendSyncPrompt(mockClient, input) + + //#then + bunExpect(promptAsync).toHaveBeenCalled() + bunExpect(promptArgs.body.agent).toBe("sisyphus-junior") + bunExpect(promptArgs.body.model).toEqual({ + providerID: "openai", + modelID: "gpt-5.4", + }) + bunExpect(promptArgs.body.variant).toBe("medium") + }) + bunTest("retries with promptSync for oracle when promptAsync fails with unexpected EOF", async () => { //#given const { sendSyncPrompt } = require("./sync-prompt-sender") diff --git a/src/tools/delegate-task/sync-prompt-sender.ts b/src/tools/delegate-task/sync-prompt-sender.ts index f34009744..fe4f8a693 100644 --- a/src/tools/delegate-task/sync-prompt-sender.ts +++ b/src/tools/delegate-task/sync-prompt-sender.ts @@ -56,9 +56,7 @@ export async function sendSyncPrompt( const promptArgs = { path: { id: input.sessionID }, body: { - // When a custom model is configured, omit the agent name so opencode's - // built-in agent fallback chain does not override the user-specified model. - ...(input.categoryModel ? {} : { agent: input.agentToUse }), + agent: input.agentToUse, system: input.systemContent, tools, parts: [createInternalAgentTextPart(effectivePrompt)],