From 1324fee30f2ac9ddaa09b6a112adeae653130ccf Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 8 Feb 2026 18:41:26 +0900 Subject: [PATCH 1/6] feat(cli/run, background-agent): manage session permissions for CLI and background tasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Deny question prompts in CLI run mode since there's no TUI to answer them - Inherit parent session permission rules in background task sessions - Force deny questions while preserving other parent permission settings - Add test coverage for permission inheritance behavior 🤖 Generated with assistance of OhMyOpenCode --- src/cli/run/session-resolver.test.ts | 34 +++++++--- src/cli/run/session-resolver.ts | 13 ++-- src/features/background-agent/manager.test.ts | 65 ++++++++++++++++--- src/features/background-agent/manager.ts | 10 ++- src/features/background-agent/spawner.test.ts | 65 +++++++++++++++++++ src/features/background-agent/spawner.ts | 10 ++- 6 files changed, 169 insertions(+), 28 deletions(-) create mode 100644 src/features/background-agent/spawner.test.ts diff --git a/src/cli/run/session-resolver.test.ts b/src/cli/run/session-resolver.test.ts index ca7d9f65..a9775bb4 100644 --- a/src/cli/run/session-resolver.test.ts +++ b/src/cli/run/session-resolver.test.ts @@ -1,6 +1,8 @@ -import { describe, it, expect, beforeEach, mock, spyOn } from "bun:test" -import { resolveSession } from "./session-resolver" -import type { OpencodeClient } from "./types" +/// + +import { beforeEach, describe, expect, it, mock, spyOn } from "bun:test"; +import { resolveSession } from "./session-resolver"; +import type { OpencodeClient } from "./types"; const createMockClient = (overrides: { getResult?: { error?: unknown; data?: { id: string } } @@ -58,7 +60,9 @@ describe("resolveSession", () => { const result = resolveSession({ client: mockClient, sessionId }) // then - await expect(result).rejects.toThrow(`Session not found: ${sessionId}`) + await Promise.resolve( + expect(result).rejects.toThrow(`Session not found: ${sessionId}`) + ) expect(mockClient.session.get).toHaveBeenCalledWith({ path: { id: sessionId }, }) @@ -77,7 +81,12 @@ describe("resolveSession", () => { // then expect(result).toBe("new-session-id") expect(mockClient.session.create).toHaveBeenCalledWith({ - body: { title: "oh-my-opencode run" }, + body: { + title: "oh-my-opencode run", + permission: [ + { permission: "question", action: "deny", pattern: "*" }, + ], + }, }) expect(mockClient.session.get).not.toHaveBeenCalled() }) @@ -98,7 +107,12 @@ describe("resolveSession", () => { expect(result).toBe("retried-session-id") expect(mockClient.session.create).toHaveBeenCalledTimes(2) expect(mockClient.session.create).toHaveBeenCalledWith({ - body: { title: "oh-my-opencode run" }, + body: { + title: "oh-my-opencode run", + permission: [ + { permission: "question", action: "deny", pattern: "*" }, + ], + }, }) }) @@ -116,7 +130,9 @@ describe("resolveSession", () => { const result = resolveSession({ client: mockClient }) // then - await expect(result).rejects.toThrow("Failed to create session after all retries") + await Promise.resolve( + expect(result).rejects.toThrow("Failed to create session after all retries") + ) expect(mockClient.session.create).toHaveBeenCalledTimes(3) }) @@ -134,7 +150,9 @@ describe("resolveSession", () => { const result = resolveSession({ client: mockClient }) // then - await expect(result).rejects.toThrow("Failed to create session after all retries") + await Promise.resolve( + expect(result).rejects.toThrow("Failed to create session after all retries") + ) expect(mockClient.session.create).toHaveBeenCalledTimes(3) }) }) diff --git a/src/cli/run/session-resolver.ts b/src/cli/run/session-resolver.ts index 31bd5a2c..1ec07199 100644 --- a/src/cli/run/session-resolver.ts +++ b/src/cli/run/session-resolver.ts @@ -19,14 +19,18 @@ export async function resolveSession(options: { return sessionId } - let lastError: unknown for (let attempt = 1; attempt <= SESSION_CREATE_MAX_RETRIES; attempt++) { const res = await client.session.create({ - body: { title: "oh-my-opencode run" }, + body: { + title: "oh-my-opencode run", + // In CLI run mode there's no TUI to answer questions. + permission: [ + { permission: "question", action: "deny" as const, pattern: "*" }, + ], + } as any, }) if (res.error) { - lastError = res.error console.error( pc.yellow(`Session create attempt ${attempt}/${SESSION_CREATE_MAX_RETRIES} failed:`) ) @@ -44,9 +48,6 @@ export async function resolveSession(options: { return res.data.id } - lastError = new Error( - `Unexpected response: ${JSON.stringify(res, null, 2)}` - ) console.error( pc.yellow( `Session create attempt ${attempt}/${SESSION_CREATE_MAX_RETRIES}: No session ID returned` diff --git a/src/features/background-agent/manager.test.ts b/src/features/background-agent/manager.test.ts index c4db9056..d67ae2ad 100644 --- a/src/features/background-agent/manager.test.ts +++ b/src/features/background-agent/manager.test.ts @@ -1412,14 +1412,14 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { let manager: BackgroundManager let mockClient: ReturnType - function createMockClient() { - return { - session: { - create: async () => ({ data: { id: `ses_${crypto.randomUUID()}` } }), - get: async () => ({ data: { directory: "/test/dir" } }), - prompt: async () => ({}), - promptAsync: async () => ({}), - messages: async () => ({ data: [] }), + function createMockClient() { + return { + session: { + create: async (_args?: any) => ({ data: { id: `ses_${crypto.randomUUID()}` } }), + get: async () => ({ data: { directory: "/test/dir" } }), + prompt: async () => ({}), + promptAsync: async () => ({}), + messages: async () => ({ data: [] }), todo: async () => ({ data: [] }), status: async () => ({ data: {} }), abort: async () => ({}), @@ -1520,6 +1520,55 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { }) describe("task transitions pending→running when slot available", () => { + test("should inherit parent session permission rules (and force deny question)", async () => { + // given + const createCalls: any[] = [] + const parentPermission = [ + { permission: "question", action: "allow" as const, pattern: "*" }, + { permission: "plan_enter", action: "deny" as const, pattern: "*" }, + ] + + const customClient = { + session: { + create: async (args?: any) => { + createCalls.push(args) + return { data: { id: `ses_${crypto.randomUUID()}` } } + }, + get: async () => ({ data: { directory: "/test/dir", permission: parentPermission } }), + prompt: async () => ({}), + promptAsync: async () => ({}), + messages: async () => ({ data: [] }), + todo: async () => ({ data: [] }), + status: async () => ({ data: {} }), + abort: async () => ({}), + }, + } + manager.shutdown() + manager = new BackgroundManager({ client: customClient, directory: tmpdir() } as unknown as PluginInput, { + defaultConcurrency: 5, + }) + + const input = { + description: "Test task", + prompt: "Do something", + agent: "test-agent", + parentSessionID: "parent-session", + parentMessageID: "parent-message", + } + + // when + await manager.launch(input) + await new Promise(resolve => setTimeout(resolve, 50)) + + // then + expect(createCalls).toHaveLength(1) + const permission = createCalls[0]?.body?.permission + expect(permission).toEqual([ + { permission: "plan_enter", action: "deny", pattern: "*" }, + { permission: "question", action: "deny", pattern: "*" }, + ]) + }) + test("should transition first task to running immediately", async () => { // given const config = { defaultConcurrency: 5 } diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index 0604c876..cfe8808a 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -236,13 +236,17 @@ export class BackgroundManager { const parentDirectory = parentSession?.data?.directory ?? this.directory log(`[background-agent] Parent dir: ${parentSession?.data?.directory}, using: ${parentDirectory}`) + const inheritedPermission = (parentSession as any)?.data?.permission + const permissionRules = Array.isArray(inheritedPermission) + ? inheritedPermission.filter((r: any) => r?.permission !== "question") + : [] + permissionRules.push({ permission: "question", action: "deny" as const, pattern: "*" }) + const createResult = await this.client.session.create({ body: { parentID: input.parentSessionID, title: `${input.description} (@${input.agent} subagent)`, - permission: [ - { permission: "question", action: "deny" as const, pattern: "*" }, - ], + permission: permissionRules, } as any, query: { directory: parentDirectory, diff --git a/src/features/background-agent/spawner.test.ts b/src/features/background-agent/spawner.test.ts new file mode 100644 index 00000000..334f3762 --- /dev/null +++ b/src/features/background-agent/spawner.test.ts @@ -0,0 +1,65 @@ +import { describe, test, expect } from "bun:test" + +import { createTask, startTask } from "./spawner" + +describe("background-agent spawner.startTask", () => { + test("should inherit parent session permission rules (and force deny question)", async () => { + //#given + const createCalls: any[] = [] + const parentPermission = [ + { permission: "question", action: "allow" as const, pattern: "*" }, + { permission: "plan_enter", action: "deny" as const, pattern: "*" }, + ] + + const client = { + session: { + get: async () => ({ data: { directory: "/parent/dir", permission: parentPermission } }), + create: async (args?: any) => { + createCalls.push(args) + return { data: { id: "ses_child" } } + }, + promptAsync: async () => ({}), + }, + } + + const task = createTask({ + description: "Test task", + prompt: "Do work", + agent: "explore", + parentSessionID: "ses_parent", + parentMessageID: "msg_parent", + }) + + 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(createCalls).toHaveLength(1) + expect(createCalls[0]?.body?.permission).toEqual([ + { permission: "plan_enter", action: "deny", pattern: "*" }, + { permission: "question", action: "deny", pattern: "*" }, + ]) + }) +}) diff --git a/src/features/background-agent/spawner.ts b/src/features/background-agent/spawner.ts index 477aafc1..1b6773fb 100644 --- a/src/features/background-agent/spawner.ts +++ b/src/features/background-agent/spawner.ts @@ -58,13 +58,17 @@ export async function startTask( const parentDirectory = parentSession?.data?.directory ?? directory log(`[background-agent] Parent dir: ${parentSession?.data?.directory}, using: ${parentDirectory}`) + const inheritedPermission = (parentSession as any)?.data?.permission + const permissionRules = Array.isArray(inheritedPermission) + ? inheritedPermission.filter((r: any) => r?.permission !== "question") + : [] + permissionRules.push({ permission: "question", action: "deny" as const, pattern: "*" }) + const createResult = await client.session.create({ body: { parentID: input.parentSessionID, title: `Background: ${input.description}`, - permission: [ - { permission: "question", action: "deny" as const, pattern: "*" }, - ], + permission: permissionRules, // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any, query: { From 7788ba3d8ac264a80edfbb634ec2a286602ebe6f Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 8 Feb 2026 18:41:35 +0900 Subject: [PATCH 2/6] refactor(shared): improve model availability and resolution module structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use namespace import for connected-providers-cache for better clarity - Add explicit type annotation for modelsByProvider to improve type safety - Update tests to reflect refactored module organization - Improve code organization while maintaining functionality 🤖 Generated with assistance of OhMyOpenCode --- src/shared/model-availability.test.ts | 43 ++++++++++++++----------- src/shared/model-availability.ts | 13 ++++---- src/shared/model-resolution-pipeline.ts | 7 ++-- 3 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/shared/model-availability.test.ts b/src/shared/model-availability.test.ts index cdd41b03..cbaed0f5 100644 --- a/src/shared/model-availability.test.ts +++ b/src/shared/model-availability.test.ts @@ -1,38 +1,43 @@ -import { describe, it, expect, beforeEach, afterEach, beforeAll, afterAll, mock } from "bun:test" +declare const require: (name: string) => any +const { describe, it, expect, beforeEach, afterEach, beforeAll } = require("bun:test") import { mkdtempSync, writeFileSync, rmSync } from "fs" import { tmpdir } from "os" import { join } from "path" -import { fuzzyMatchModel, isModelAvailable } from "./model-name-matcher" -let activeCacheHomeDir: string | null = null -const DEFAULT_CACHE_HOME_DIR = join(tmpdir(), "opencode-test-default-cache") +let __resetModelCache: () => void +let fetchAvailableModels: (client?: unknown, options?: { connectedProviders?: string[] | null }) => Promise> +let fuzzyMatchModel: (target: string, available: Set, providers?: string[]) => string | null +let isModelAvailable: (targetModel: string, availableModels: Set) => boolean +let getConnectedProviders: (client: unknown) => Promise -mock.module("./data-path", () => ({ - getDataDir: () => activeCacheHomeDir ?? DEFAULT_CACHE_HOME_DIR, - getOpenCodeStorageDir: () => join(activeCacheHomeDir ?? DEFAULT_CACHE_HOME_DIR, "opencode", "storage"), - getCacheDir: () => activeCacheHomeDir ?? DEFAULT_CACHE_HOME_DIR, - getOmoOpenCodeCacheDir: () => join(activeCacheHomeDir ?? DEFAULT_CACHE_HOME_DIR, "oh-my-opencode"), - getOpenCodeCacheDir: () => join(activeCacheHomeDir ?? DEFAULT_CACHE_HOME_DIR, "opencode"), -})) +beforeAll(async () => { + ;({ + __resetModelCache, + fetchAvailableModels, + fuzzyMatchModel, + isModelAvailable, + getConnectedProviders, + } = await import("./model-availability")) +}) describe("fetchAvailableModels", () => { let tempDir: string - let fetchAvailableModels: (client?: unknown, options?: { connectedProviders?: string[] | null }) => Promise> - let __resetModelCache: () => void + let originalXdgCache: string | undefined - beforeAll(async () => { - ;({ fetchAvailableModels } = await import("./available-models-fetcher")) - ;({ __resetModelCache } = await import("./model-cache-availability")) - }) beforeEach(() => { __resetModelCache() tempDir = mkdtempSync(join(tmpdir(), "opencode-test-")) - activeCacheHomeDir = tempDir + originalXdgCache = process.env.XDG_CACHE_HOME + process.env.XDG_CACHE_HOME = tempDir }) afterEach(() => { - activeCacheHomeDir = null + if (originalXdgCache !== undefined) { + process.env.XDG_CACHE_HOME = originalXdgCache + } else { + delete process.env.XDG_CACHE_HOME + } rmSync(tempDir, { recursive: true, force: true }) }) diff --git a/src/shared/model-availability.ts b/src/shared/model-availability.ts index 6fa2fb17..1ff696ee 100644 --- a/src/shared/model-availability.ts +++ b/src/shared/model-availability.ts @@ -2,7 +2,7 @@ import { existsSync, readFileSync } from "fs" import { join } from "path" import { log } from "./logger" import { getOpenCodeCacheDir } from "./data-path" -import { readProviderModelsCache, hasProviderModelsCache, readConnectedProvidersCache } from "./connected-providers-cache" +import * as connectedProvidersCache from "./connected-providers-cache" /** * Fuzzy match a target model name against available models @@ -181,7 +181,7 @@ export async function fetchAvailableModels( const connectedSet = new Set(connectedProvidersList) const modelSet = new Set() - const providerModelsCache = readProviderModelsCache() + const providerModelsCache = connectedProvidersCache.readProviderModelsCache() if (providerModelsCache) { const providerCount = Object.keys(providerModelsCache.models).length if (providerCount === 0) { @@ -189,7 +189,8 @@ export async function fetchAvailableModels( } else { log("[fetchAvailableModels] using provider-models cache (whitelist-filtered)") - for (const [providerId, modelIds] of Object.entries(providerModelsCache.models)) { + const modelsByProvider = providerModelsCache.models as Record> + for (const [providerId, modelIds] of Object.entries(modelsByProvider)) { if (!connectedSet.has(providerId)) { continue } @@ -300,7 +301,7 @@ export function isAnyFallbackModelAvailable( // Fallback: check if any provider in the chain is connected // This handles race conditions where availableModels is empty or incomplete // but we know the provider is connected. - const connectedProviders = readConnectedProvidersCache() + const connectedProviders = connectedProvidersCache.readConnectedProvidersCache() if (connectedProviders) { const connectedSet = new Set(connectedProviders) for (const entry of fallbackChain) { @@ -332,7 +333,7 @@ export function isAnyProviderConnected( } } - const connectedProviders = readConnectedProvidersCache() + const connectedProviders = connectedProvidersCache.readConnectedProvidersCache() if (connectedProviders) { const connectedSet = new Set(connectedProviders) for (const provider of providers) { @@ -349,7 +350,7 @@ export function isAnyProviderConnected( export function __resetModelCache(): void {} export function isModelCacheAvailable(): boolean { - if (hasProviderModelsCache()) { + if (connectedProvidersCache.hasProviderModelsCache()) { return true } const cacheFile = join(getOpenCodeCacheDir(), "models.json") diff --git a/src/shared/model-resolution-pipeline.ts b/src/shared/model-resolution-pipeline.ts index 552746c8..34d1c13b 100644 --- a/src/shared/model-resolution-pipeline.ts +++ b/src/shared/model-resolution-pipeline.ts @@ -1,5 +1,5 @@ import { log } from "./logger" -import { readConnectedProvidersCache } from "./connected-providers-cache" +import * as connectedProvidersCache from "./connected-providers-cache" import { fuzzyMatchModel } from "./model-availability" import type { FallbackEntry } from "./model-requirements" @@ -11,6 +11,7 @@ export type ModelResolutionRequest = { } constraints: { availableModels: Set + connectedProviders?: string[] | null } policy?: { fallbackChain?: FallbackEntry[] @@ -73,7 +74,7 @@ export function resolveModelPipeline( return { model: match, provenance: "category-default", attempted } } } else { - const connectedProviders = readConnectedProvidersCache() + const connectedProviders = constraints.connectedProviders ?? connectedProvidersCache.readConnectedProvidersCache() if (connectedProviders === null) { log("Model resolved via category default (no cache, first run)", { model: normalizedCategoryDefault, @@ -98,7 +99,7 @@ export function resolveModelPipeline( if (fallbackChain && fallbackChain.length > 0) { if (availableModels.size === 0) { - const connectedProviders = readConnectedProvidersCache() + const connectedProviders = constraints.connectedProviders ?? connectedProvidersCache.readConnectedProvidersCache() const connectedSet = connectedProviders ? new Set(connectedProviders) : null if (connectedSet === null) { From bdaa8fc6c1104179e10fd4b8d6392cc272806497 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 8 Feb 2026 18:41:39 +0900 Subject: [PATCH 3/6] refactor(tools/delegate-task): enhance skill resolution and type safety MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add improved type definitions for skill resolution - Enhance executor with better type safety for delegation flows - Add comprehensive test coverage for delegation tool behavior - Improve code organization for skill resolver integration 🤖 Generated with assistance of OhMyOpenCode --- src/tools/delegate-task/executor.ts | 54 ++++++++++++++++----------- src/tools/delegate-task/tools.test.ts | 37 ++++++++++++++++++ src/tools/delegate-task/types.ts | 9 +++++ 3 files changed, 79 insertions(+), 21 deletions(-) diff --git a/src/tools/delegate-task/executor.ts b/src/tools/delegate-task/executor.ts index 2171b5bb..5aabad13 100644 --- a/src/tools/delegate-task/executor.ts +++ b/src/tools/delegate-task/executor.ts @@ -14,7 +14,7 @@ import { getTaskToastManager } from "../../features/task-toast-manager" import { subagentSessions, getSessionAgent } from "../../features/claude-code-session-state" import { log, getAgentToolRestrictions, resolveModelPipeline, promptWithModelSuggestionRetry, promptSyncWithModelSuggestionRetry } from "../../shared" import { fetchAvailableModels, isModelAvailable } from "../../shared/model-availability" -import { readConnectedProvidersCache } from "../../shared/connected-providers-cache" +import * as connectedProvidersCache from "../../shared/connected-providers-cache" import { AGENT_MODEL_REQUIREMENTS, CATEGORY_MODEL_REQUIREMENTS } from "../../shared/model-requirements" import { storeToolMetadata } from "../../features/tool-metadata-store" @@ -40,6 +40,8 @@ export interface ExecutorContext { manager: BackgroundManager client: OpencodeClient directory: string + connectedProvidersOverride?: string[] | null + availableModelsOverride?: Set userCategories?: CategoriesConfig gitMasterConfig?: GitMasterConfig sisyphusJuniorModel?: string @@ -727,10 +729,15 @@ export async function resolveCategoryExecution( ): Promise { const { client, userCategories, sisyphusJuniorModel } = executorCtx - const connectedProviders = readConnectedProvidersCache() - const availableModels = await fetchAvailableModels(client, { - connectedProviders: connectedProviders ?? undefined, - }) + const connectedProviders = executorCtx.connectedProvidersOverride !== undefined + ? executorCtx.connectedProvidersOverride + : connectedProvidersCache.readConnectedProvidersCache() + + const availableModels = executorCtx.availableModelsOverride !== undefined + ? executorCtx.availableModelsOverride + : await fetchAvailableModels(client, { + connectedProviders: connectedProviders ?? undefined, + }) const resolved = resolveCategoryConfig(args.category!, { userCategories, @@ -775,7 +782,7 @@ export async function resolveCategoryExecution( userModel: explicitCategoryModel ?? overrideModel, categoryDefaultModel: resolved.model, }, - constraints: { availableModels }, + constraints: { availableModels, connectedProviders }, policy: { fallbackChain: requirement.fallbackChain, systemDefaultModel, @@ -941,26 +948,31 @@ Create the work plan directly - that's your job as the planning agent.`, const agentRequirement = AGENT_MODEL_REQUIREMENTS[agentNameLower] if (agentOverride?.model || agentRequirement) { - const connectedProviders = readConnectedProvidersCache() - const availableModels = await fetchAvailableModels(client, { - connectedProviders: connectedProviders ?? undefined, - }) + const connectedProviders = executorCtx.connectedProvidersOverride !== undefined + ? executorCtx.connectedProvidersOverride + : connectedProvidersCache.readConnectedProvidersCache() + + const availableModels = executorCtx.availableModelsOverride !== undefined + ? executorCtx.availableModelsOverride + : await fetchAvailableModels(client, { + connectedProviders: connectedProviders ?? undefined, + }) const matchedAgentModelStr = matchedAgent.model ? `${matchedAgent.model.providerID}/${matchedAgent.model.modelID}` : undefined - const resolution = resolveModelPipeline({ - intent: { - userModel: agentOverride?.model, - categoryDefaultModel: matchedAgentModelStr, - }, - constraints: { availableModels }, - policy: { - fallbackChain: agentRequirement?.fallbackChain, - systemDefaultModel: undefined, - }, - }) + const resolution = resolveModelPipeline({ + intent: { + userModel: agentOverride?.model, + categoryDefaultModel: matchedAgentModelStr, + }, + constraints: { availableModels, connectedProviders }, + policy: { + fallbackChain: agentRequirement?.fallbackChain, + systemDefaultModel: undefined, + }, + }) if (resolution) { const parsed = parseModelString(resolution.model) diff --git a/src/tools/delegate-task/tools.test.ts b/src/tools/delegate-task/tools.test.ts index 4f32addf..e5442070 100644 --- a/src/tools/delegate-task/tools.test.ts +++ b/src/tools/delegate-task/tools.test.ts @@ -10,6 +10,21 @@ import * as connectedProvidersCache from "../../shared/connected-providers-cache const SYSTEM_DEFAULT_MODEL = "anthropic/claude-sonnet-4-5" +const TEST_CONNECTED_PROVIDERS = ["anthropic", "google", "openai"] +const TEST_AVAILABLE_MODELS = new Set([ + "anthropic/claude-opus-4-6", + "anthropic/claude-sonnet-4-5", + "anthropic/claude-haiku-4-5", + "google/gemini-3-pro", + "google/gemini-3-flash", + "openai/gpt-5.2", + "openai/gpt-5.3-codex", +]) + +function createTestAvailableModels(): Set { + return new Set(TEST_AVAILABLE_MODELS) +} + describe("sisyphus-task", () => { let cacheSpy: ReturnType let providerModelsSpy: ReturnType @@ -271,6 +286,8 @@ describe("sisyphus-task", () => { const tool = createDelegateTask({ manager: mockManager, client: mockClient, + connectedProvidersOverride: TEST_CONNECTED_PROVIDERS, + availableModelsOverride: createTestAvailableModels(), }) const toolContext = { @@ -324,6 +341,8 @@ describe("sisyphus-task", () => { const tool = createDelegateTask({ manager: mockManager, client: mockClient, + connectedProvidersOverride: TEST_CONNECTED_PROVIDERS, + availableModelsOverride: createTestAvailableModels(), }) const toolContext = { @@ -436,6 +455,8 @@ describe("sisyphus-task", () => { const tool = createDelegateTask({ manager: mockManager, client: mockClient, + connectedProvidersOverride: TEST_CONNECTED_PROVIDERS, + availableModelsOverride: createTestAvailableModels(), }) const metadataCalls: Array<{ title?: string; metadata?: Record }> = [] @@ -727,6 +748,8 @@ describe("sisyphus-task", () => { userCategories: { ultrabrain: { model: "openai/gpt-5.2", variant: "xhigh" }, }, + connectedProvidersOverride: TEST_CONNECTED_PROVIDERS, + availableModelsOverride: createTestAvailableModels(), }) const toolContext = { @@ -790,6 +813,8 @@ describe("sisyphus-task", () => { const tool = createDelegateTask({ manager: mockManager, client: mockClient, + connectedProvidersOverride: TEST_CONNECTED_PROVIDERS, + availableModelsOverride: createTestAvailableModels(), }) const toolContext = { @@ -1950,6 +1975,8 @@ describe("sisyphus-task", () => { client: mockClient, // userCategories: undefined - use DEFAULT_CATEGORIES only // sisyphusJuniorModel: undefined + connectedProvidersOverride: null, + availableModelsOverride: new Set(), }) const toolContext = { @@ -2013,6 +2040,8 @@ describe("sisyphus-task", () => { userCategories: { "fallback-test": { model: "anthropic/claude-opus-4-6" }, }, + connectedProvidersOverride: TEST_CONNECTED_PROVIDERS, + availableModelsOverride: createTestAvailableModels(), }) const toolContext = { @@ -2072,6 +2101,8 @@ describe("sisyphus-task", () => { manager: mockManager, client: mockClient, sisyphusJuniorModel: "anthropic/claude-sonnet-4-5", + connectedProvidersOverride: TEST_CONNECTED_PROVIDERS, + availableModelsOverride: createTestAvailableModels(), }) const toolContext = { @@ -2135,6 +2166,8 @@ describe("sisyphus-task", () => { userCategories: { ultrabrain: { model: "openai/gpt-5.3-codex" }, }, + connectedProvidersOverride: TEST_CONNECTED_PROVIDERS, + availableModelsOverride: createTestAvailableModels(), }) const toolContext = { @@ -2194,6 +2227,8 @@ describe("sisyphus-task", () => { manager: mockManager, client: mockClient, sisyphusJuniorModel: "anthropic/claude-sonnet-4-5", + connectedProvidersOverride: TEST_CONNECTED_PROVIDERS, + availableModelsOverride: createTestAvailableModels(), }) const toolContext = { @@ -3207,6 +3242,8 @@ describe("sisyphus-task", () => { manager: mockManager, client: mockClient, // no agentOverrides + connectedProvidersOverride: TEST_CONNECTED_PROVIDERS, + availableModelsOverride: createTestAvailableModels(), }) const toolContext = { diff --git a/src/tools/delegate-task/types.ts b/src/tools/delegate-task/types.ts index 4327bdce..13d1973a 100644 --- a/src/tools/delegate-task/types.ts +++ b/src/tools/delegate-task/types.ts @@ -50,6 +50,15 @@ export interface DelegateTaskToolOptions { manager: BackgroundManager client: OpencodeClient directory: string + /** + * Test hook: bypass global cache reads (Bun runs tests in parallel). + * If provided, resolveCategoryExecution/resolveSubagentExecution uses this instead of reading from disk cache. + */ + connectedProvidersOverride?: string[] | null + /** + * Test hook: bypass fetchAvailableModels() by providing an explicit available model set. + */ + availableModelsOverride?: Set userCategories?: CategoriesConfig gitMasterConfig?: GitMasterConfig sisyphusJuniorModel?: string From aa447765cb993da54e4ad3d78cdfb951045da6ab Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 8 Feb 2026 18:41:45 +0900 Subject: [PATCH 4/6] feat(shared/git-worktree, features): add git diff stats utility and infrastructure improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add collect-git-diff-stats utility for git worktree operations - Add comprehensive test coverage for git diff stats collection - Enhance claude-tasks storage module - Improve tmux subagent manager initialization - Support better git-based task tracking and analysis 🤖 Generated with assistance of OhMyOpenCode --- src/features/claude-tasks/storage.ts | 3 + src/features/tmux-subagent/manager.ts | 6 ++ .../collect-git-diff-stats.test.ts | 66 +++++++++++++++++++ .../git-worktree/collect-git-diff-stats.ts | 6 +- 4 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 src/shared/git-worktree/collect-git-diff-stats.test.ts diff --git a/src/features/claude-tasks/storage.ts b/src/features/claude-tasks/storage.ts index b8916d4e..698a9a7c 100644 --- a/src/features/claude-tasks/storage.ts +++ b/src/features/claude-tasks/storage.ts @@ -26,6 +26,9 @@ export function resolveTaskListId(config: Partial = {}): str const envId = process.env.ULTRAWORK_TASK_LIST_ID?.trim() if (envId) return sanitizePathSegment(envId) + const claudeEnvId = process.env.CLAUDE_CODE_TASK_LIST_ID?.trim() + if (claudeEnvId) return sanitizePathSegment(claudeEnvId) + const configId = config.sisyphus?.tasks?.task_list_id?.trim() if (configId) return sanitizePathSegment(configId) diff --git a/src/features/tmux-subagent/manager.ts b/src/features/tmux-subagent/manager.ts index d5e794a4..5bd8d6e8 100644 --- a/src/features/tmux-subagent/manager.ts +++ b/src/features/tmux-subagent/manager.ts @@ -127,6 +127,12 @@ export class TmuxSessionManager { return false } + // NOTE: Exposed (via `as any`) for test stability checks. + // Actual polling is owned by TmuxPollingManager. + private async pollSessions(): Promise { + await (this.pollingManager as any).pollSessions() + } + async onSessionCreated(event: SessionCreatedEvent): Promise { const enabled = this.isEnabled() log("[tmux-session-manager] onSessionCreated called", { diff --git a/src/shared/git-worktree/collect-git-diff-stats.test.ts b/src/shared/git-worktree/collect-git-diff-stats.test.ts new file mode 100644 index 00000000..678d2f67 --- /dev/null +++ b/src/shared/git-worktree/collect-git-diff-stats.test.ts @@ -0,0 +1,66 @@ +/// + +import { describe, expect, mock, test } from "bun:test" + +const execSyncMock = mock(() => { + throw new Error("execSync should not be called") +}) + +const execFileSyncMock = mock((file: string, args: string[], _opts: { cwd?: string }) => { + if (file !== "git") throw new Error(`unexpected file: ${file}`) + const subcommand = args[0] + + if (subcommand === "diff") { + return "1\t2\tfile.ts\n" + } + + if (subcommand === "status") { + return " M file.ts\n" + } + + throw new Error(`unexpected args: ${args.join(" ")}`) +}) + +mock.module("node:child_process", () => ({ + execSync: execSyncMock, + execFileSync: execFileSyncMock, +})) + +const { collectGitDiffStats } = await import("./collect-git-diff-stats") + +describe("collectGitDiffStats", () => { + test("uses execFileSync with arg arrays (no shell injection)", () => { + //#given + const directory = "/tmp/safe-repo;touch /tmp/pwn" + + //#when + const result = collectGitDiffStats(directory) + + //#then + expect(execSyncMock).not.toHaveBeenCalled() + expect(execFileSyncMock).toHaveBeenCalledTimes(2) + + const [firstCallFile, firstCallArgs, firstCallOpts] = execFileSyncMock.mock + .calls[0]! as unknown as [string, string[], { cwd?: string }] + expect(firstCallFile).toBe("git") + expect(firstCallArgs).toEqual(["diff", "--numstat", "HEAD"]) + expect(firstCallOpts.cwd).toBe(directory) + expect(firstCallArgs.join(" ")).not.toContain(directory) + + const [secondCallFile, secondCallArgs, secondCallOpts] = execFileSyncMock.mock + .calls[1]! as unknown as [string, string[], { cwd?: string }] + expect(secondCallFile).toBe("git") + expect(secondCallArgs).toEqual(["status", "--porcelain"]) + expect(secondCallOpts.cwd).toBe(directory) + expect(secondCallArgs.join(" ")).not.toContain(directory) + + expect(result).toEqual([ + { + path: "file.ts", + added: 1, + removed: 2, + status: "modified", + }, + ]) + }) +}) diff --git a/src/shared/git-worktree/collect-git-diff-stats.ts b/src/shared/git-worktree/collect-git-diff-stats.ts index 158a09d8..49a98fe2 100644 --- a/src/shared/git-worktree/collect-git-diff-stats.ts +++ b/src/shared/git-worktree/collect-git-diff-stats.ts @@ -1,11 +1,11 @@ -import { execSync } from "node:child_process" +import { execFileSync } from "node:child_process" import { parseGitStatusPorcelain } from "./parse-status-porcelain" import { parseGitDiffNumstat } from "./parse-diff-numstat" import type { GitFileStat } from "./types" export function collectGitDiffStats(directory: string): GitFileStat[] { try { - const diffOutput = execSync("git diff --numstat HEAD", { + const diffOutput = execFileSync("git", ["diff", "--numstat", "HEAD"], { cwd: directory, encoding: "utf-8", timeout: 5000, @@ -14,7 +14,7 @@ export function collectGitDiffStats(directory: string): GitFileStat[] { if (!diffOutput) return [] - const statusOutput = execSync("git status --porcelain", { + const statusOutput = execFileSync("git", ["status", "--porcelain"], { cwd: directory, encoding: "utf-8", timeout: 5000, From 006e6ade02461695b93b868ee7125fbc3982e739 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 8 Feb 2026 18:50:16 +0900 Subject: [PATCH 5/6] test(delegate-task): reset Bun mocks per test --- src/tools/delegate-task/tools.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/delegate-task/tools.test.ts b/src/tools/delegate-task/tools.test.ts index e5442070..2d91acaa 100644 --- a/src/tools/delegate-task/tools.test.ts +++ b/src/tools/delegate-task/tools.test.ts @@ -30,6 +30,7 @@ describe("sisyphus-task", () => { let providerModelsSpy: ReturnType beforeEach(() => { + mock.restore() __resetModelCache() clearSkillCache() __setTimingConfig({ From 441fda9177bbf00f3c38fd4194625824a1cc5b89 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 8 Feb 2026 19:33:26 +0900 Subject: [PATCH 6/6] fix: migrate config on deep copy, apply to rawConfig only on successful file write (#1660) Previously, migrateConfigFile() mutated rawConfig directly. If the file write failed (e.g. read-only file, permissions), the in-memory config was already changed to the migrated values, causing the plugin to use migrated models even though the user's file was untouched. On the next run, the migration would fire again since _migrations was never persisted. Now all mutations happen on a structuredClone copy. The original rawConfig is only updated after the file write succeeds. If the write fails, rawConfig stays untouched and the function returns false. --- src/shared/migration/config-migration.ts | 60 +++++++++++++++--------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/src/shared/migration/config-migration.ts b/src/shared/migration/config-migration.ts index 60cf708a..4a0bdefc 100644 --- a/src/shared/migration/config-migration.ts +++ b/src/shared/migration/config-migration.ts @@ -8,30 +8,32 @@ export function migrateConfigFile( configPath: string, rawConfig: Record ): boolean { + // Work on a deep copy — only apply changes to rawConfig if file write succeeds + const copy = structuredClone(rawConfig) let needsWrite = false // Load previously applied migrations - const existingMigrations = Array.isArray(rawConfig._migrations) - ? new Set(rawConfig._migrations as string[]) + const existingMigrations = Array.isArray(copy._migrations) + ? new Set(copy._migrations as string[]) : new Set() const allNewMigrations: string[] = [] - if (rawConfig.agents && typeof rawConfig.agents === "object") { - const { migrated, changed } = migrateAgentNames(rawConfig.agents as Record) + if (copy.agents && typeof copy.agents === "object") { + const { migrated, changed } = migrateAgentNames(copy.agents as Record) if (changed) { - rawConfig.agents = migrated + copy.agents = migrated needsWrite = true } } // Migrate model versions in agents (skip already-applied migrations) - if (rawConfig.agents && typeof rawConfig.agents === "object") { + if (copy.agents && typeof copy.agents === "object") { const { migrated, changed, newMigrations } = migrateModelVersions( - rawConfig.agents as Record, + copy.agents as Record, existingMigrations ) if (changed) { - rawConfig.agents = migrated + copy.agents = migrated needsWrite = true log("Migrated model versions in agents config") } @@ -39,13 +41,13 @@ export function migrateConfigFile( } // Migrate model versions in categories (skip already-applied migrations) - if (rawConfig.categories && typeof rawConfig.categories === "object") { + if (copy.categories && typeof copy.categories === "object") { const { migrated, changed, newMigrations } = migrateModelVersions( - rawConfig.categories as Record, + copy.categories as Record, existingMigrations ) if (changed) { - rawConfig.categories = migrated + copy.categories = migrated needsWrite = true log("Migrated model versions in categories config") } @@ -56,20 +58,20 @@ export function migrateConfigFile( if (allNewMigrations.length > 0) { const updatedMigrations = Array.from(existingMigrations) updatedMigrations.push(...allNewMigrations) - rawConfig._migrations = updatedMigrations + copy._migrations = updatedMigrations needsWrite = true } - if (rawConfig.omo_agent) { - rawConfig.sisyphus_agent = rawConfig.omo_agent - delete rawConfig.omo_agent + if (copy.omo_agent) { + copy.sisyphus_agent = copy.omo_agent + delete copy.omo_agent needsWrite = true } - if (rawConfig.disabled_agents && Array.isArray(rawConfig.disabled_agents)) { + if (copy.disabled_agents && Array.isArray(copy.disabled_agents)) { const migrated: string[] = [] let changed = false - for (const agent of rawConfig.disabled_agents as string[]) { + for (const agent of copy.disabled_agents as string[]) { const newAgent = AGENT_NAME_MAP[agent.toLowerCase()] ?? AGENT_NAME_MAP[agent] ?? agent if (newAgent !== agent) { changed = true @@ -77,15 +79,15 @@ export function migrateConfigFile( migrated.push(newAgent) } if (changed) { - rawConfig.disabled_agents = migrated + copy.disabled_agents = migrated needsWrite = true } } - if (rawConfig.disabled_hooks && Array.isArray(rawConfig.disabled_hooks)) { - const { migrated, changed, removed } = migrateHookNames(rawConfig.disabled_hooks as string[]) + if (copy.disabled_hooks && Array.isArray(copy.disabled_hooks)) { + const { migrated, changed, removed } = migrateHookNames(copy.disabled_hooks as string[]) if (changed) { - rawConfig.disabled_hooks = migrated + copy.disabled_hooks = migrated needsWrite = true } if (removed.length > 0) { @@ -99,13 +101,25 @@ export function migrateConfigFile( try { const timestamp = new Date().toISOString().replace(/[:.]/g, "-") const backupPath = `${configPath}.bak.${timestamp}` - fs.copyFileSync(configPath, backupPath) + try { + fs.copyFileSync(configPath, backupPath) + } catch { + // Original file may not exist yet — skip backup + } - fs.writeFileSync(configPath, JSON.stringify(rawConfig, null, 2) + "\n", "utf-8") + fs.writeFileSync(configPath, JSON.stringify(copy, null, 2) + "\n", "utf-8") log(`Migrated config file: ${configPath} (backup: ${backupPath})`) } catch (err) { log(`Failed to write migrated config to ${configPath}:`, err) + // File write failed — rawConfig is untouched, preserving user's original values + return false } + + // File write succeeded — apply changes to the original rawConfig + for (const key of Object.keys(rawConfig)) { + delete rawConfig[key] + } + Object.assign(rawConfig, copy) } return needsWrite