elias/oh-my-opencode
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #1709 from code-yeongyu/feature/comment-checker-apply-patch

Browse Source 
feat(comment-checker): support apply_patch
This commit is contained in:
YeonGyu-Kim 2026-02-10 14:17:28 +09:00 committed by GitHub
commit 4f9cec434b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 818 additions and 116 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
src/agents/sisyphus-junior/default.ts
View File

@ -12,6 +12,7 @@ export function buildDefaultSisyphusJuniorPrompt(
promptAppend?: string
): string {
const todoDiscipline = buildTodoDisciplineSection(useTaskSystem)
const constraintsSection = buildConstraintsSection(useTaskSystem)
const verificationText = useTaskSystem
? "All tasks marked completed"
: "All todos marked completed"
@ -21,13 +22,7 @@ Sisyphus-Junior - Focused executor from OhMyOpenCode.
Execute tasks directly. NEVER delegate or spawn other agents.
</Role>
<Critical_Constraints>
BLOCKED ACTIONS (will fail if attempted):
- task tool: BLOCKED
ALLOWED: call_omo_agent - You CAN spawn explore/librarian agents for research.
You work ALONE for implementation. No delegation of implementation tasks.
</Critical_Constraints>
${constraintsSection}
${todoDiscipline}
@ -48,6 +43,29 @@ Task NOT complete without:
return prompt + "\n\n" + promptAppend
}
function buildConstraintsSection(useTaskSystem: boolean): string {
if (useTaskSystem) {
return `<Critical_Constraints>
BLOCKED ACTIONS (will fail if attempted):
- task (agent delegation tool): BLOCKED — you cannot delegate work to other agents
ALLOWED tools:
- call_omo_agent: You CAN spawn explore/librarian agents for research
- task_create, task_update, task_list, task_get: ALLOWED — use these for tracking your work
You work ALONE for implementation. No delegation of implementation tasks.
</Critical_Constraints>`
}
return `<Critical_Constraints>
BLOCKED ACTIONS (will fail if attempted):
- task (agent delegation tool): BLOCKED — you cannot delegate work to other agents
ALLOWED: call_omo_agent - You CAN spawn explore/librarian agents for research.
You work ALONE for implementation. No delegation of implementation tasks.
</Critical_Constraints>`
}
function buildTodoDisciplineSection(useTaskSystem: boolean): string {
if (useTaskSystem) {
return `<Task_Discipline>

51
src/agents/sisyphus-junior/gpt.ts
View File

@ -21,6 +21,7 @@ export function buildGptSisyphusJuniorPrompt(
promptAppend?: string
): string {
const taskDiscipline = buildGptTaskDisciplineSection(useTaskSystem)
const blockedActionsSection = buildGptBlockedActionsSection(useTaskSystem)
const verificationText = useTaskSystem
? "All tasks marked completed"
: "All todos marked completed"
@ -45,19 +46,7 @@ Role: Execute tasks directly. You work ALONE.
- Do NOT expand task boundaries beyond what's written.
</scope_and_design_constraints>
<blocked_actions>
BLOCKED (will fail if attempted):
| Tool | Status |
|------|--------|
| task | BLOCKED |
ALLOWED:
| Tool | Usage |
|------|-------|
| call_omo_agent | Spawn explore/librarian for research ONLY |
You work ALONE for implementation. No delegation.
</blocked_actions>
${blockedActionsSection}
<uncertainty_and_ambiguity>
- If a task is ambiguous or underspecified:
@ -99,6 +88,42 @@ Task NOT complete without evidence:
return prompt + "\n\n" + promptAppend
}
function buildGptBlockedActionsSection(useTaskSystem: boolean): string {
if (useTaskSystem) {
return `<blocked_actions>
BLOCKED (will fail if attempted):
| Tool | Status | Description |
|------|--------|-------------|
| task | BLOCKED | Agent delegation tool — you cannot spawn other agents |
ALLOWED:
| Tool | Usage |
|------|-------|
| call_omo_agent | Spawn explore/librarian for research ONLY |
| task_create | Create tasks to track your work |
| task_update | Update task status (in_progress, completed) |
| task_list | List active tasks |
| task_get | Get task details by ID |
You work ALONE for implementation. No delegation.
</blocked_actions>`
}
return `<blocked_actions>
BLOCKED (will fail if attempted):
| Tool | Status | Description |
|------|--------|-------------|
| task | BLOCKED | Agent delegation tool — you cannot spawn other agents |
ALLOWED:
| Tool | Usage |
|------|-------|
| call_omo_agent | Spawn explore/librarian for research ONLY |
You work ALONE for implementation. No delegation.
</blocked_actions>`
}
function buildGptTaskDisciplineSection(useTaskSystem: boolean): string {
if (useTaskSystem) {
return `<task_discipline_spec>

42
src/agents/sisyphus-junior/index.test.ts
View File

@ -238,6 +238,48 @@ describe("createSisyphusJuniorAgentWithOverrides", () => {
expect(result.prompt).toContain("todowrite")
expect(result.prompt).not.toContain("TaskCreate")
})
test("useTaskSystem=true explicitly lists task management tools as ALLOWED for Claude", () => {
//#given
const override = { model: "anthropic/claude-sonnet-4-5" }
//#when
const result = createSisyphusJuniorAgentWithOverrides(override, undefined, true)
//#then - prompt must disambiguate: delegation tool blocked, management tools allowed
expect(result.prompt).toContain("task_create")
expect(result.prompt).toContain("task_update")
expect(result.prompt).toContain("task_list")
expect(result.prompt).toContain("task_get")
expect(result.prompt).toContain("agent delegation tool")
})
test("useTaskSystem=true explicitly lists task management tools as ALLOWED for GPT", () => {
//#given
const override = { model: "openai/gpt-5.2" }
//#when
const result = createSisyphusJuniorAgentWithOverrides(override, undefined, true)
//#then - prompt must disambiguate: delegation tool blocked, management tools allowed
expect(result.prompt).toContain("task_create")
expect(result.prompt).toContain("task_update")
expect(result.prompt).toContain("task_list")
expect(result.prompt).toContain("task_get")
expect(result.prompt).toContain("Agent delegation tool")
})
test("useTaskSystem=false does NOT list task management tools in constraints", () => {
//#given - Claude model without task system
const override = { model: "anthropic/claude-sonnet-4-5" }
//#when
const result = createSisyphusJuniorAgentWithOverrides(override, undefined, false)
//#then - no task management tool references in constraints section
expect(result.prompt).not.toContain("task_create")
expect(result.prompt).not.toContain("task_update")
})
})
describe("prompt composition", () => {

2
src/features/opencode-skill-loader/loader.ts
View File

@ -1,5 +1,5 @@
import { join } from "path"
import { getClaudeConfigDir } from "../../shared"
import { getClaudeConfigDir } from "../../shared/claude-config-dir"
import { getOpenCodeConfigDir } from "../../shared/opencode-config-dir"
import type { CommandDefinition } from "../claude-code-command-loader/types"
import type { LoadedSkill } from "./types"

18
src/hooks/anthropic-context-window-limit-recovery/executor.test.ts
View File

@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from "bun:
import { executeCompact } from "./executor"
import type { AutoCompactState } from "./types"
import * as storage from "./storage"
import * as messagesReader from "../session-recovery/storage/messages-reader"
type TimerCallback = (...args: any[]) => void
@ -168,7 +169,8 @@ describe("executeCompact lock management", () => {
})
test("clears lock when fixEmptyMessages path executes", async () => {
// given: Empty content error scenario
//#given - Empty content error scenario with no messages in storage
const readMessagesSpy = spyOn(messagesReader, "readMessages").mockReturnValue([])
autoCompactState.errorDataBySession.set(sessionID, {
errorType: "non-empty content required",
messageIndex: 0,
@ -176,16 +178,17 @@ describe("executeCompact lock management", () => {
maxTokens: 200000,
})
// when: Execute compaction (fixEmptyMessages will be called)
//#when - Execute compaction (fixEmptyMessages will be called)
await executeCompact(sessionID, msg, autoCompactState, mockClient, directory)
// then: Lock should be cleared
//#then - Lock should be cleared
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(false)
readMessagesSpy.mockRestore()
})
test("clears lock when truncation is sufficient", async () => {
// given: Aggressive truncation scenario with sufficient truncation
// This test verifies the early return path in aggressive truncation
//#given - Aggressive truncation scenario with no messages in storage
const readMessagesSpy = spyOn(messagesReader, "readMessages").mockReturnValue([])
autoCompactState.errorDataBySession.set(sessionID, {
errorType: "token_limit",
currentTokens: 250000,
@ -197,7 +200,7 @@ describe("executeCompact lock management", () => {
aggressive_truncation: true,
}
// when: Execute compaction with experimental flag
//#when - Execute compaction with experimental flag
await executeCompact(
sessionID,
msg,
@ -207,8 +210,9 @@ describe("executeCompact lock management", () => {
experimental,
)
// then: Lock should be cleared even on early return
//#then - Lock should be cleared even on early return
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(false)
readMessagesSpy.mockRestore()
})
test("prevents concurrent compaction attempts", async () => {

11
src/hooks/atlas/event-handler.ts
View File

@ -89,13 +89,20 @@ export function createAtlasEventHandler(input: {
return
}
const requiredAgent = (boulderState.agent ?? "atlas").toLowerCase()
const lastAgent = getLastAgentFromSession(sessionID)
if (!lastAgent || lastAgent !== requiredAgent) {
const requiredAgent = (boulderState.agent ?? "atlas").toLowerCase()
const lastAgentMatchesRequired = lastAgent === requiredAgent
const boulderAgentWasNotExplicitlySet = boulderState.agent === undefined
const boulderAgentDefaultsToAtlas = requiredAgent === "atlas"
const lastAgentIsSisyphus = lastAgent === "sisyphus"
const allowSisyphusWhenDefaultAtlas = boulderAgentWasNotExplicitlySet && boulderAgentDefaultsToAtlas && lastAgentIsSisyphus
const agentMatches = lastAgentMatchesRequired || allowSisyphusWhenDefaultAtlas
if (!agentMatches) {
log(`[${HOOK_NAME}] Skipped: last agent does not match boulder agent`, {
sessionID,
lastAgent: lastAgent ?? "unknown",
requiredAgent,
boulderAgentExplicitlySet: boulderState.agent !== undefined,
})
return
}

39
src/hooks/comment-checker/cli-runner.ts
View File

@ -58,6 +58,45 @@ export async function processWithCli(
}
}
export interface ApplyPatchEdit {
filePath: string
before: string
after: string
}
export async function processApplyPatchEditsWithCli(
sessionID: string,
edits: ApplyPatchEdit[],
output: { output: string },
cliPath: string,
customPrompt: string | undefined,
debugLog: (...args: unknown[]) => void,
): Promise<void> {
debugLog("processing apply_patch edits:", edits.length)
for (const edit of edits) {
const hookInput: HookInput = {
session_id: sessionID,
tool_name: "Edit",
transcript_path: "",
cwd: process.cwd(),
hook_event_name: "PostToolUse",
tool_input: {
file_path: edit.filePath,
old_string: edit.before,
new_string: edit.after,
},
}
const result = await runCommentChecker(hookInput, cliPath, customPrompt)
if (result.hasComments && result.message) {
debugLog("CLI detected comments for apply_patch file:", edit.filePath)
output.output += `\n\n${result.message}`
}
}
}
export function isCliPathUsable(cliPath: string | null): cliPath is string {
return Boolean(cliPath && existsSync(cliPath))
}

83
src/hooks/comment-checker/hook.apply-patch.test.ts Normal file
View File

@ -0,0 +1,83 @@
import { describe, it, expect, mock, beforeEach } from "bun:test"
const processApplyPatchEditsWithCli = mock(async () => {})
mock.module("./cli-runner", () => ({
initializeCommentCheckerCli: () => {},
getCommentCheckerCliPathPromise: () => Promise.resolve("/tmp/fake-comment-checker"),
isCliPathUsable: () => true,
processWithCli: async () => {},
processApplyPatchEditsWithCli,
}))
const { createCommentCheckerHooks } = await import("./hook")
describe("comment-checker apply_patch integration", () => {
beforeEach(() => {
processApplyPatchEditsWithCli.mockClear()
})
it("runs comment checker using apply_patch metadata.files", async () => {
// given
const hooks = createCommentCheckerHooks()
const input = { tool: "apply_patch", sessionID: "ses_test", callID: "call_test" }
const output = {
title: "ok",
output: "Success. Updated the following files:\nM src/a.ts",
metadata: {
files: [
{
filePath: "/repo/src/a.ts",
before: "const a = 1\n",
after: "// comment\nconst a = 1\n",
type: "update",
},
{
filePath: "/repo/src/old.ts",
movePath: "/repo/src/new.ts",
before: "const b = 1\n",
after: "// moved comment\nconst b = 1\n",
type: "move",
},
{
filePath: "/repo/src/delete.ts",
before: "// deleted\n",
after: "",
type: "delete",
},
],
},
}
// when
await hooks["tool.execute.after"](input, output)
// then
expect(processApplyPatchEditsWithCli).toHaveBeenCalledTimes(1)
expect(processApplyPatchEditsWithCli).toHaveBeenCalledWith(
"ses_test",
[
{ filePath: "/repo/src/a.ts", before: "const a = 1\n", after: "// comment\nconst a = 1\n" },
{ filePath: "/repo/src/new.ts", before: "const b = 1\n", after: "// moved comment\nconst b = 1\n" },
],
expect.any(Object),
"/tmp/fake-comment-checker",
undefined,
expect.any(Function),
)
})
it("skips when apply_patch metadata.files is missing", async () => {
// given
const hooks = createCommentCheckerHooks()
const input = { tool: "apply_patch", sessionID: "ses_test", callID: "call_test" }
const output = { title: "ok", output: "ok", metadata: {} }
// when
await hooks["tool.execute.after"](input, output)
// then
expect(processApplyPatchEditsWithCli).toHaveBeenCalledTimes(0)
})
})

86
src/hooks/comment-checker/hook.ts
View File

@ -1,7 +1,15 @@
import type { PendingCall } from "./types"
import type { CommentCheckerConfig } from "../../config/schema"
import { initializeCommentCheckerCli, getCommentCheckerCliPathPromise, isCliPathUsable, processWithCli } from "./cli-runner"
import z from "zod"
import {
initializeCommentCheckerCli,
getCommentCheckerCliPathPromise,
isCliPathUsable,
processWithCli,
processApplyPatchEditsWithCli,
} from "./cli-runner"
import { registerPendingCall, startPendingCallCleanup, takePendingCall } from "./pending-calls"
import * as fs from "fs"
@ -81,13 +89,7 @@ export function createCommentCheckerHooks(config?: CommentCheckerConfig) {
): Promise<void> => {
debugLog("tool.execute.after:", { tool: input.tool, callID: input.callID })
const pendingCall = takePendingCall(input.callID)
if (!pendingCall) {
debugLog("no pendingCall found for:", input.callID)
return
}
debugLog("processing pendingCall:", pendingCall)
const toolLower = input.tool.toLowerCase()
// Only skip if the output indicates a tool execution failure
const outputLower = output.output.toLowerCase()
@ -102,17 +104,75 @@ export function createCommentCheckerHooks(config?: CommentCheckerConfig) {
return
}
try {
// Wait for CLI path resolution
const cliPath = await getCommentCheckerCliPathPromise()
const ApplyPatchMetadataSchema = z.object({
files: z.array(
z.object({
filePath: z.string(),
movePath: z.string().optional(),
before: z.string(),
after: z.string(),
type: z.string().optional(),
}),
),
})
if (toolLower === "apply_patch") {
const parsed = ApplyPatchMetadataSchema.safeParse(output.metadata)
if (!parsed.success) {
debugLog("apply_patch metadata schema mismatch, skipping")
return
}
const edits = parsed.data.files
.filter((f) => f.type !== "delete")
.map((f) => ({
filePath: f.movePath ?? f.filePath,
before: f.before,
after: f.after,
}))
if (edits.length === 0) {
debugLog("apply_patch had no editable files, skipping")
return
}
try {
const cliPath = await getCommentCheckerCliPathPromise()
if (!isCliPathUsable(cliPath)) {
debugLog("CLI not available, skipping comment check")
return
}
debugLog("using CLI for apply_patch:", cliPath)
await processApplyPatchEditsWithCli(
input.sessionID,
edits,
output,
cliPath,
config?.custom_prompt,
debugLog,
)
} catch (err) {
debugLog("apply_patch comment check failed:", err)
}
return
}
const pendingCall = takePendingCall(input.callID)
if (!pendingCall) {
debugLog("no pendingCall found for:", input.callID)
return
}
debugLog("processing pendingCall:", pendingCall)
try {
const cliPath = await getCommentCheckerCliPathPromise()
if (!isCliPathUsable(cliPath)) {
// CLI not available - silently skip comment checking
debugLog("CLI not available, skipping comment check")
return
}
// CLI mode only
debugLog("using CLI:", cliPath)
await processWithCli(input, pendingCall, output, cliPath, config?.custom_prompt, debugLog)
} catch (err) {

35
src/plugin/event.ts
View File

@ -12,6 +12,8 @@ import { lspManager } from "../tools"
import type { CreatedHooks } from "../create-hooks"
import type { Managers } from "../create-managers"
import { normalizeSessionStatusToIdle } from "./session-status-normalizer"
import { pruneRecentSyntheticIdles } from "./recent-synthetic-idles"
type FirstMessageVariantGate = {
markSessionCreated: (sessionInfo: { id?: string; title?: string; parentID?: string } | undefined) => void
@ -27,7 +29,7 @@ export function createEventHandler(args: {
}): (input: { event: { type: string; properties?: Record<string, unknown> } }) => Promise<void> {
const { ctx, firstMessageVariantGate, managers, hooks } = args
return async (input): Promise<void> => {
const dispatchToHooks = async (input: { event: { type: string; properties?: Record<string, unknown> } }): Promise<void> => {
await Promise.resolve(hooks.autoUpdateChecker?.event?.(input))
await Promise.resolve(hooks.claudeCodeHooks?.event?.(input))
await Promise.resolve(hooks.backgroundNotificationHook?.event?.(input))
@ -47,6 +49,37 @@ export function createEventHandler(args: {
await Promise.resolve(hooks.stopContinuationGuard?.event?.(input))
await Promise.resolve(hooks.compactionTodoPreserver?.event?.(input))
await Promise.resolve(hooks.atlasHook?.handler?.(input))
}
const recentSyntheticIdles = new Map<string, number>()
const DEDUP_WINDOW_MS = 500
return async (input): Promise<void> => {
pruneRecentSyntheticIdles({
recentSyntheticIdles,
now: Date.now(),
dedupWindowMs: DEDUP_WINDOW_MS,
})
if (input.event.type === "session.idle") {
const sessionID = (input.event.properties as Record<string, unknown> | undefined)?.sessionID as string | undefined
if (sessionID) {
const emittedAt = recentSyntheticIdles.get(sessionID)
if (emittedAt && Date.now() - emittedAt < DEDUP_WINDOW_MS) {
recentSyntheticIdles.delete(sessionID)
return
}
}
}
await dispatchToHooks(input)
const syntheticIdle = normalizeSessionStatusToIdle(input)
if (syntheticIdle) {
const sessionID = (syntheticIdle.event.properties as Record<string, unknown>)?.sessionID as string
recentSyntheticIdles.set(sessionID, Date.now())
await dispatchToHooks(syntheticIdle)
}
const { event } = input
const props = event.properties as Record<string, unknown> | undefined

24
src/plugin/recent-synthetic-idles.test.ts Normal file
View File

@ -0,0 +1,24 @@
import { describe, it, expect } from "bun:test"
import { pruneRecentSyntheticIdles } from "./recent-synthetic-idles"
describe("pruneRecentSyntheticIdles", () => {
it("removes entries older than dedup window", () => {
//#given
const recentSyntheticIdles = new Map<string, number>([
["ses_old", 1000],
["ses_new", 1600],
])
//#when
pruneRecentSyntheticIdles({
recentSyntheticIdles,
now: 2000,
dedupWindowMs: 500,
})
//#then
expect(recentSyntheticIdles.has("ses_old")).toBe(false)
expect(recentSyntheticIdles.has("ses_new")).toBe(true)
})
})

13
src/plugin/recent-synthetic-idles.ts Normal file
View File

@ -0,0 +1,13 @@
export function pruneRecentSyntheticIdles(args: {
recentSyntheticIdles: Map<string, number>
now: number
dedupWindowMs: number
}): void {
const { recentSyntheticIdles, now, dedupWindowMs } = args
for (const [sessionID, emittedAt] of recentSyntheticIdles) {
if (now - emittedAt >= dedupWindowMs) {
recentSyntheticIdles.delete(sessionID)
}
}
}

119
src/plugin/session-status-normalizer.test.ts Normal file
View File

@ -0,0 +1,119 @@
import { describe, it, expect } from "bun:test"
import { normalizeSessionStatusToIdle } from "./session-status-normalizer"
type EventInput = { event: { type: string; properties?: Record<string, unknown> } }
describe("normalizeSessionStatusToIdle", () => {
it("converts session.status with idle type to synthetic session.idle event", () => {
//#given - a session.status event with type=idle
const input: EventInput = {
event: {
type: "session.status",
properties: {
sessionID: "ses_abc123",
status: { type: "idle" },
},
},
}
//#when - normalizeSessionStatusToIdle is called
const result = normalizeSessionStatusToIdle(input)
//#then - returns a synthetic session.idle event
expect(result).toEqual({
event: {
type: "session.idle",
properties: {
sessionID: "ses_abc123",
},
},
})
})
it("returns null for session.status with busy type", () => {
//#given - a session.status event with type=busy
const input: EventInput = {
event: {
type: "session.status",
properties: {
sessionID: "ses_abc123",
status: { type: "busy" },
},
},
}
//#when - normalizeSessionStatusToIdle is called
const result = normalizeSessionStatusToIdle(input)
//#then - returns null (no synthetic idle event)
expect(result).toBeNull()
})
it("returns null for session.status with retry type", () => {
//#given - a session.status event with type=retry
const input: EventInput = {
event: {
type: "session.status",
properties: {
sessionID: "ses_abc123",
status: { type: "retry", attempt: 1, message: "retrying", next: 5000 },
},
},
}
//#when - normalizeSessionStatusToIdle is called
const result = normalizeSessionStatusToIdle(input)
//#then - returns null
expect(result).toBeNull()
})
it("returns null for non-session.status events", () => {
//#given - a message.updated event
const input: EventInput = {
event: {
type: "message.updated",
properties: { info: { sessionID: "ses_abc123" } },
},
}
//#when - normalizeSessionStatusToIdle is called
const result = normalizeSessionStatusToIdle(input)
//#then - returns null
expect(result).toBeNull()
})
it("returns null when session.status has no properties", () => {
//#given - a session.status event with no properties
const input: EventInput = {
event: {
type: "session.status",
},
}
//#when - normalizeSessionStatusToIdle is called
const result = normalizeSessionStatusToIdle(input)
//#then - returns null
expect(result).toBeNull()
})
it("returns null when session.status has no status object", () => {
//#given - a session.status event with sessionID but no status
const input: EventInput = {
event: {
type: "session.status",
properties: {
sessionID: "ses_abc123",
},
},
}
//#when - normalizeSessionStatusToIdle is called
const result = normalizeSessionStatusToIdle(input)
//#then - returns null
expect(result).toBeNull()
})
})

22
src/plugin/session-status-normalizer.ts Normal file
View File

@ -0,0 +1,22 @@
type EventInput = { event: { type: string; properties?: Record<string, unknown> } }
type SessionStatus = { type: string }
export function normalizeSessionStatusToIdle(input: EventInput): EventInput | null {
if (input.event.type !== "session.status") return null
const props = input.event.properties
if (!props) return null
const status = props.status as SessionStatus | undefined
if (!status || status.type !== "idle") return null
const sessionID = props.sessionID as string | undefined
if (!sessionID) return null
return {
event: {
type: "session.idle",
properties: { sessionID },
},
}
}

133
src/shared/connected-providers-cache.test.ts Normal file
View File

@ -0,0 +1,133 @@
import { describe, test, expect, beforeEach, afterEach, spyOn } from "bun:test"
import { existsSync, mkdirSync, rmSync } from "fs"
import { join } from "path"
import * as dataPath from "./data-path"
import { updateConnectedProvidersCache, readProviderModelsCache } from "./connected-providers-cache"
const TEST_CACHE_DIR = join(import.meta.dir, "__test-cache__")
describe("updateConnectedProvidersCache", () => {
let cacheDirSpy: ReturnType<typeof spyOn>
beforeEach(() => {
cacheDirSpy = spyOn(dataPath, "getOmoOpenCodeCacheDir").mockReturnValue(TEST_CACHE_DIR)
if (existsSync(TEST_CACHE_DIR)) {
rmSync(TEST_CACHE_DIR, { recursive: true })
}
mkdirSync(TEST_CACHE_DIR, { recursive: true })
})
afterEach(() => {
cacheDirSpy.mockRestore()
if (existsSync(TEST_CACHE_DIR)) {
rmSync(TEST_CACHE_DIR, { recursive: true })
}
})
test("extracts models from provider.list().all response", async () => {
//#given
const mockClient = {
provider: {
list: async () => ({
data: {
connected: ["openai", "anthropic"],
all: [
{
id: "openai",
name: "OpenAI",
env: [],
models: {
"gpt-5.3-codex": { id: "gpt-5.3-codex", name: "GPT-5.3 Codex" },
"gpt-5.2": { id: "gpt-5.2", name: "GPT-5.2" },
},
},
{
id: "anthropic",
name: "Anthropic",
env: [],
models: {
"claude-opus-4-6": { id: "claude-opus-4-6", name: "Claude Opus 4.6" },
"claude-sonnet-4-5": { id: "claude-sonnet-4-5", name: "Claude Sonnet 4.5" },
},
},
],
},
}),
},
}
//#when
await updateConnectedProvidersCache(mockClient)
//#then
const cache = readProviderModelsCache()
expect(cache).not.toBeNull()
expect(cache!.connected).toEqual(["openai", "anthropic"])
expect(cache!.models).toEqual({
openai: ["gpt-5.3-codex", "gpt-5.2"],
anthropic: ["claude-opus-4-6", "claude-sonnet-4-5"],
})
})
test("writes empty models when provider has no models", async () => {
//#given
const mockClient = {
provider: {
list: async () => ({
data: {
connected: ["empty-provider"],
all: [
{
id: "empty-provider",
name: "Empty",
env: [],
models: {},
},
],
},
}),
},
}
//#when
await updateConnectedProvidersCache(mockClient)
//#then
const cache = readProviderModelsCache()
expect(cache).not.toBeNull()
expect(cache!.models).toEqual({})
})
test("writes empty models when all field is missing", async () => {
//#given
const mockClient = {
provider: {
list: async () => ({
data: {
connected: ["openai"],
},
}),
},
}
//#when
await updateConnectedProvidersCache(mockClient)
//#then
const cache = readProviderModelsCache()
expect(cache).not.toBeNull()
expect(cache!.models).toEqual({})
})
test("does nothing when client.provider.list is not available", async () => {
//#given
const mockClient = {}
//#when
await updateConnectedProvidersCache(mockClient)
//#then
const cache = readProviderModelsCache()
expect(cache).toBeNull()
})
})

42
src/shared/connected-providers-cache.ts
View File

@ -149,10 +149,12 @@ export function writeProviderModelsCache(data: { models: Record<string, string[]
*/
export async function updateConnectedProvidersCache(client: {
provider?: {
list?: () => Promise<{ data?: { connected?: string[] } }>
}
model?: {
list?: () => Promise<{ data?: Array<{ id: string; provider: string }> }>
list?: () => Promise<{
data?: {
connected?: string[]
all?: Array<{ id: string; models?: Record<string, unknown> }>
}
}>
}
}): Promise<void> {
if (!client?.provider?.list) {
@ -167,31 +169,23 @@ export async function updateConnectedProvidersCache(client: {
writeConnectedProvidersCache(connected)
// Always update provider-models cache (overwrite with fresh data)
let modelsByProvider: Record<string, string[]> = {}
if (client.model?.list) {
try {
const modelsResult = await client.model.list()
const models = modelsResult.data ?? []
const modelsByProvider: Record<string, string[]> = {}
const allProviders = result.data?.all ?? []
for (const model of models) {
if (!modelsByProvider[model.provider]) {
modelsByProvider[model.provider] = []
}
modelsByProvider[model.provider].push(model.id)
for (const provider of allProviders) {
if (provider.models) {
const modelIds = Object.keys(provider.models)
if (modelIds.length > 0) {
modelsByProvider[provider.id] = modelIds
}
log("[connected-providers-cache] Fetched models from API", {
providerCount: Object.keys(modelsByProvider).length,
totalModels: models.length,
})
} catch (modelErr) {
log("[connected-providers-cache] Error fetching models, writing empty cache", { error: String(modelErr) })
}
} else {
log("[connected-providers-cache] client.model.list not available, writing empty cache")
}
log("[connected-providers-cache] Extracted models from provider list", {
providerCount: Object.keys(modelsByProvider).length,
totalModels: Object.values(modelsByProvider).reduce((sum, ids) => sum + ids.length, 0),
})
writeProviderModelsCache({
models: modelsByProvider,
connected,

92
src/shared/git-worktree/collect-git-diff-stats.test.ts
View File

@ -1,79 +1,79 @@
/// <reference types="bun-types" />
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?? new-file.ts\n"
}
if (subcommand === "ls-files") {
return "new-file.ts\n"
}
throw new Error(`unexpected args: ${args.join(" ")}`)
})
const readFileSyncMock = mock((_path: string, _encoding: string) => {
return "line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\nline10\n"
})
mock.module("node:child_process", () => ({
execSync: execSyncMock,
execFileSync: execFileSyncMock,
}))
mock.module("node:fs", () => ({
readFileSync: readFileSyncMock,
}))
const { collectGitDiffStats } = await import("./collect-git-diff-stats")
import { describe, expect, test, spyOn, beforeEach, afterEach } from "bun:test"
import * as childProcess from "node:child_process"
import * as fs from "node:fs"
describe("collectGitDiffStats", () => {
test("uses execFileSync with arg arrays (no shell injection)", () => {
let execFileSyncSpy: ReturnType<typeof spyOn>
let execSyncSpy: ReturnType<typeof spyOn>
let readFileSyncSpy: ReturnType<typeof spyOn>
beforeEach(() => {
execSyncSpy = spyOn(childProcess, "execSync").mockImplementation(() => {
throw new Error("execSync should not be called")
})
execFileSyncSpy = spyOn(childProcess, "execFileSync").mockImplementation(
((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?? new-file.ts\n"
if (subcommand === "ls-files") return "new-file.ts\n"
throw new Error(`unexpected args: ${args.join(" ")}`)
}) as typeof childProcess.execFileSync
)
readFileSyncSpy = spyOn(fs, "readFileSync").mockImplementation(
((_path: unknown, _encoding: unknown) => {
return "line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\nline10\n"
}) as typeof fs.readFileSync
)
})
afterEach(() => {
execSyncSpy.mockRestore()
execFileSyncSpy.mockRestore()
readFileSyncSpy.mockRestore()
})
test("uses execFileSync with arg arrays (no shell injection)", async () => {
//#given
const { collectGitDiffStats } = await import("./collect-git-diff-stats")
const directory = "/tmp/safe-repo;touch /tmp/pwn"
//#when
const result = collectGitDiffStats(directory)
//#then
expect(execSyncMock).not.toHaveBeenCalled()
expect(execFileSyncMock).toHaveBeenCalledTimes(3)
expect(execSyncSpy).not.toHaveBeenCalled()
expect(execFileSyncSpy).toHaveBeenCalledTimes(3)
const [firstCallFile, firstCallArgs, firstCallOpts] = execFileSyncMock.mock
const [firstCallFile, firstCallArgs, firstCallOpts] = execFileSyncSpy.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
const [secondCallFile, secondCallArgs, secondCallOpts] = execFileSyncSpy.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)
const [thirdCallFile, thirdCallArgs, thirdCallOpts] = execFileSyncMock.mock
const [thirdCallFile, thirdCallArgs, thirdCallOpts] = execFileSyncSpy.mock
.calls[2]! as unknown as [string, string[], { cwd?: string }]
expect(thirdCallFile).toBe("git")
expect(thirdCallArgs).toEqual(["ls-files", "--others", "--exclude-standard"])
expect(thirdCallOpts.cwd).toBe(directory)
expect(thirdCallArgs.join(" ")).not.toContain(directory)
expect(readFileSyncMock).toHaveBeenCalled()
expect(readFileSyncSpy).toHaveBeenCalled()
expect(result).toEqual([
{

62
src/tools/delegate-task/category-resolver.test.ts Normal file
View File

@ -0,0 +1,62 @@
import { describe, test, expect } from "bun:test"
import { resolveCategoryExecution } from "./category-resolver"
import type { ExecutorContext } from "./executor-types"
describe("resolveCategoryExecution", () => {
const createMockExecutorContext = (): ExecutorContext => ({
client: {} as any,
manager: {} as any,
directory: "/tmp/test",
userCategories: {},
sisyphusJuniorModel: undefined,
})
test("returns clear error when category exists but required model is not available", async () => {
//#given
const args = {
category: "deep",
prompt: "test prompt",
description: "Test task",
run_in_background: false,
load_skills: [],
blockedBy: undefined,
enableSkillTools: false,
}
const executorCtx = createMockExecutorContext()
const inheritedModel = undefined
const systemDefaultModel = "anthropic/claude-sonnet-4-5"
//#when
const result = await resolveCategoryExecution(args, executorCtx, inheritedModel, systemDefaultModel)
//#then
expect(result.error).toBeDefined()
expect(result.error).toContain("deep")
expect(result.error).toMatch(/model.*not.*available|requires.*model/i)
expect(result.error).not.toContain("Unknown category")
})
test("returns 'unknown category' error for truly unknown categories", async () => {
//#given
const args = {
category: "definitely-not-a-real-category-xyz123",
prompt: "test prompt",
description: "Test task",
run_in_background: false,
load_skills: [],
blockedBy: undefined,
enableSkillTools: false,
}
const executorCtx = createMockExecutorContext()
const inheritedModel = undefined
const systemDefaultModel = "anthropic/claude-sonnet-4-5"
//#when
const result = await resolveCategoryExecution(args, executorCtx, inheritedModel, systemDefaultModel)
//#then
expect(result.error).toBeDefined()
expect(result.error).toContain("Unknown category")
expect(result.error).toContain("definitely-not-a-real-category-xyz123")
})
})

28
src/tools/delegate-task/category-resolver.ts
View File

@ -29,7 +29,10 @@ export async function resolveCategoryExecution(
const availableModels = await getAvailableModelsForDelegateTask(client)
const resolved = resolveCategoryConfig(args.category!, {
const categoryName = args.category!
const categoryExists = DEFAULT_CATEGORIES[categoryName] !== undefined || userCategories?.[categoryName] !== undefined
const resolved = resolveCategoryConfig(categoryName, {
userCategories,
inheritedModel,
systemDefaultModel,
@ -37,6 +40,27 @@ export async function resolveCategoryExecution(
})
if (!resolved) {
const requirement = CATEGORY_MODEL_REQUIREMENTS[categoryName]
const allCategoryNames = Object.keys({ ...DEFAULT_CATEGORIES, ...userCategories }).join(", ")
if (categoryExists && requirement?.requiresModel) {
return {
agentToUse: "",
categoryModel: undefined,
categoryPromptAppend: undefined,
modelInfo: undefined,
actualModel: undefined,
isUnstableAgent: false,
error: `Category "${categoryName}" requires model "${requirement.requiresModel}" which is not available.
To use this category:
1. Connect a provider with this model: ${requirement.requiresModel}
2. Or configure an alternative model in your oh-my-opencode.json for this category
Available categories: ${allCategoryNames}`,
}
}
return {
agentToUse: "",
categoryModel: undefined,
@ -44,7 +68,7 @@ export async function resolveCategoryExecution(
modelInfo: undefined,
actualModel: undefined,
isUnstableAgent: false,
error: `Unknown category: "${args.category}". Available: ${Object.keys({ ...DEFAULT_CATEGORIES, ...userCategories }).join(", ")}`,
error: `Unknown category: "${categoryName}". Available: ${allCategoryNames}`,
}
}