Merge pull request #3052 from code-yeongyu/fix/p0-1-task-system-default-split-brain
fix: resolve task_system default split-brain
This commit is contained in:
@@ -955,7 +955,7 @@ When enabled, two companion hooks are active: `hashline-read-enhancer` (annotate
|
|||||||
| `aggressive_truncation` | `false` | Aggressively truncate when token limit exceeded |
|
| `aggressive_truncation` | `false` | Aggressively truncate when token limit exceeded |
|
||||||
| `auto_resume` | `false` | Auto-resume after thinking block recovery |
|
| `auto_resume` | `false` | Auto-resume after thinking block recovery |
|
||||||
| `disable_omo_env` | `false` | Disable auto-injected `<omo-env>` block (date/time/locale). Improves cache hit rate. |
|
| `disable_omo_env` | `false` | Disable auto-injected `<omo-env>` block (date/time/locale). Improves cache hit rate. |
|
||||||
| `task_system` | `true` | Enable Sisyphus task system |
|
| `task_system` | `false` | Enable Sisyphus task system |
|
||||||
| `dynamic_context_pruning.enabled` | `false` | Auto-prune old tool outputs to manage context window |
|
| `dynamic_context_pruning.enabled` | `false` | Auto-prune old tool outputs to manage context window |
|
||||||
| `dynamic_context_pruning.notification` | `detailed` | Pruning notifications: `off` / `minimal` / `detailed` |
|
| `dynamic_context_pruning.notification` | `detailed` | Pruning notifications: `off` / `minimal` / `detailed` |
|
||||||
| `turn_protection.turns` | `3` | Recent turns protected from pruning (1–10) |
|
| `turn_protection.turns` | `3` | Recent turns protected from pruning (1–10) |
|
||||||
|
|||||||
@@ -0,0 +1,40 @@
|
|||||||
|
import { describe, expect, test } from "bun:test"
|
||||||
|
|
||||||
|
import { REPLACEMENT_MESSAGE } from "./constants"
|
||||||
|
import { createTasksTodowriteDisablerHook } from "./hook"
|
||||||
|
|
||||||
|
describe("createTasksTodowriteDisablerHook", () => {
|
||||||
|
describe("#given experimental.task_system is omitted", () => {
|
||||||
|
test("#when TodoWrite runs #then it is allowed by default", async () => {
|
||||||
|
// given
|
||||||
|
const hook = createTasksTodowriteDisablerHook({})
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = hook["tool.execute.before"](
|
||||||
|
{ tool: "TodoWrite", sessionID: "ses_123", callID: "call_123" },
|
||||||
|
{ args: {} },
|
||||||
|
)
|
||||||
|
|
||||||
|
// then
|
||||||
|
await expect(result).resolves.toBeUndefined()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe("#given experimental.task_system is enabled", () => {
|
||||||
|
test("#when TodoWrite runs #then it is blocked", async () => {
|
||||||
|
// given
|
||||||
|
const hook = createTasksTodowriteDisablerHook({
|
||||||
|
experimental: { task_system: true },
|
||||||
|
})
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = hook["tool.execute.before"](
|
||||||
|
{ tool: "TodoWrite", sessionID: "ses_123", callID: "call_123" },
|
||||||
|
{ args: {} },
|
||||||
|
)
|
||||||
|
|
||||||
|
// then
|
||||||
|
await expect(result).rejects.toThrow(REPLACEMENT_MESSAGE)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -1,3 +1,4 @@
|
|||||||
|
import { isTaskSystemEnabled } from "../../shared";
|
||||||
import { BLOCKED_TOOLS, REPLACEMENT_MESSAGE } from "./constants";
|
import { BLOCKED_TOOLS, REPLACEMENT_MESSAGE } from "./constants";
|
||||||
|
|
||||||
export interface TasksTodowriteDisablerConfig {
|
export interface TasksTodowriteDisablerConfig {
|
||||||
@@ -9,14 +10,14 @@ export interface TasksTodowriteDisablerConfig {
|
|||||||
export function createTasksTodowriteDisablerHook(
|
export function createTasksTodowriteDisablerHook(
|
||||||
config: TasksTodowriteDisablerConfig,
|
config: TasksTodowriteDisablerConfig,
|
||||||
) {
|
) {
|
||||||
const isTaskSystemEnabled = config.experimental?.task_system ?? true;
|
const taskSystemEnabled = isTaskSystemEnabled(config);
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"tool.execute.before": async (
|
"tool.execute.before": async (
|
||||||
input: { tool: string; sessionID: string; callID: string },
|
input: { tool: string; sessionID: string; callID: string },
|
||||||
_output: { args: Record<string, unknown> },
|
_output: { args: Record<string, unknown> },
|
||||||
) => {
|
) => {
|
||||||
if (!isTaskSystemEnabled) {
|
if (!taskSystemEnabled) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -78,7 +78,7 @@ describe("tasks-todowrite-disabler", () => {
|
|||||||
).resolves.toBeUndefined()
|
).resolves.toBeUndefined()
|
||||||
})
|
})
|
||||||
|
|
||||||
test("should block TodoWrite when experimental is undefined because task_system defaults to enabled", async () => {
|
test("should not block TodoWrite when experimental is undefined because task_system defaults to disabled", async () => {
|
||||||
// given
|
// given
|
||||||
const hook = createTasksTodowriteDisablerHook({})
|
const hook = createTasksTodowriteDisablerHook({})
|
||||||
const input = {
|
const input = {
|
||||||
@@ -93,7 +93,7 @@ describe("tasks-todowrite-disabler", () => {
|
|||||||
// when / then
|
// when / then
|
||||||
await expect(
|
await expect(
|
||||||
hook["tool.execute.before"](input, output)
|
hook["tool.execute.before"](input, output)
|
||||||
).rejects.toThrow("TodoRead/TodoWrite are DISABLED")
|
).resolves.toBeUndefined()
|
||||||
})
|
})
|
||||||
|
|
||||||
test("should not block TodoRead when flag is false", async () => {
|
test("should not block TodoRead when flag is false", async () => {
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import { createBuiltinAgents } from "../agents";
|
import { createBuiltinAgents } from "../agents";
|
||||||
import { createSisyphusJuniorAgentWithOverrides } from "../agents/sisyphus-junior";
|
import { createSisyphusJuniorAgentWithOverrides } from "../agents/sisyphus-junior";
|
||||||
import type { OhMyOpenCodeConfig } from "../config";
|
import type { OhMyOpenCodeConfig } from "../config";
|
||||||
import { log, migrateAgentConfig } from "../shared";
|
import { isTaskSystemEnabled, log, migrateAgentConfig } from "../shared";
|
||||||
import { AGENT_NAME_MAP } from "../shared/migration";
|
import { AGENT_NAME_MAP } from "../shared/migration";
|
||||||
import { getAgentDisplayName } from "../shared/agent-display-names";
|
import { getAgentDisplayName } from "../shared/agent-display-names";
|
||||||
import { registerAgentName } from "../features/claude-code-session-state";
|
import { registerAgentName } from "../features/claude-code-session-state";
|
||||||
@@ -90,7 +90,7 @@ export async function applyAgentConfig(params: {
|
|||||||
params.pluginConfig.browser_automation_engine?.provider ?? "playwright";
|
params.pluginConfig.browser_automation_engine?.provider ?? "playwright";
|
||||||
const currentModel = params.config.model as string | undefined;
|
const currentModel = params.config.model as string | undefined;
|
||||||
const disabledSkills = new Set<string>(params.pluginConfig.disabled_skills ?? []);
|
const disabledSkills = new Set<string>(params.pluginConfig.disabled_skills ?? []);
|
||||||
const useTaskSystem = params.pluginConfig.experimental?.task_system ?? false;
|
const useTaskSystem = isTaskSystemEnabled(params.pluginConfig);
|
||||||
const disableOmoEnv = params.pluginConfig.experimental?.disable_omo_env ?? false;
|
const disableOmoEnv = params.pluginConfig.experimental?.disable_omo_env ?? false;
|
||||||
|
|
||||||
const includeClaudeAgents = params.pluginConfig.claude_code?.agents ?? true;
|
const includeClaudeAgents = params.pluginConfig.claude_code?.agents ?? true;
|
||||||
|
|||||||
@@ -1281,6 +1281,10 @@ describe("per-agent todowrite/todoread deny when task_system enabled", () => {
|
|||||||
await handler(config)
|
await handler(config)
|
||||||
|
|
||||||
//#then
|
//#then
|
||||||
|
const lastCall =
|
||||||
|
createBuiltinAgentsMock.mock.calls[createBuiltinAgentsMock.mock.calls.length - 1]
|
||||||
|
expect(lastCall?.[11]).toBe(false)
|
||||||
|
|
||||||
const agentResult = config.agent as Record<string, { permission?: Record<string, unknown> }>
|
const agentResult = config.agent as Record<string, { permission?: Record<string, unknown> }>
|
||||||
expect(agentResult[getAgentDisplayName("sisyphus")]?.permission?.todowrite).toBeUndefined()
|
expect(agentResult[getAgentDisplayName("sisyphus")]?.permission?.todowrite).toBeUndefined()
|
||||||
expect(agentResult[getAgentDisplayName("sisyphus")]?.permission?.todoread).toBeUndefined()
|
expect(agentResult[getAgentDisplayName("sisyphus")]?.permission?.todoread).toBeUndefined()
|
||||||
@@ -1315,6 +1319,10 @@ describe("per-agent todowrite/todoread deny when task_system enabled", () => {
|
|||||||
await handler(config)
|
await handler(config)
|
||||||
|
|
||||||
//#then
|
//#then
|
||||||
|
const lastCall =
|
||||||
|
createBuiltinAgentsMock.mock.calls[createBuiltinAgentsMock.mock.calls.length - 1]
|
||||||
|
expect(lastCall?.[11]).toBe(false)
|
||||||
|
|
||||||
const agentResult = config.agent as Record<string, { permission?: Record<string, unknown> }>
|
const agentResult = config.agent as Record<string, { permission?: Record<string, unknown> }>
|
||||||
expect(agentResult[getAgentDisplayName("sisyphus")]?.permission?.todowrite).toBeUndefined()
|
expect(agentResult[getAgentDisplayName("sisyphus")]?.permission?.todowrite).toBeUndefined()
|
||||||
expect(agentResult[getAgentDisplayName("sisyphus")]?.permission?.todoread).toBeUndefined()
|
expect(agentResult[getAgentDisplayName("sisyphus")]?.permission?.todoread).toBeUndefined()
|
||||||
|
|||||||
@@ -218,6 +218,16 @@ describe("applyToolConfig", () => {
|
|||||||
|
|
||||||
describe("#given task_system is undefined", () => {
|
describe("#given task_system is undefined", () => {
|
||||||
describe("#when applying tool config", () => {
|
describe("#when applying tool config", () => {
|
||||||
|
it("#then should not disable todo tools globally by default", () => {
|
||||||
|
const params = createParams({})
|
||||||
|
|
||||||
|
applyToolConfig(params)
|
||||||
|
|
||||||
|
const tools = params.config.tools as Record<string, unknown>
|
||||||
|
expect(tools.todowrite).toBeUndefined()
|
||||||
|
expect(tools.todoread).toBeUndefined()
|
||||||
|
})
|
||||||
|
|
||||||
it.each([
|
it.each([
|
||||||
"atlas",
|
"atlas",
|
||||||
"sisyphus",
|
"sisyphus",
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import type { OhMyOpenCodeConfig } from "../config";
|
import type { OhMyOpenCodeConfig } from "../config";
|
||||||
import { getAgentDisplayName, getAgentListDisplayName } from "../shared/agent-display-names";
|
import { getAgentDisplayName, getAgentListDisplayName } from "../shared/agent-display-names";
|
||||||
|
import { isTaskSystemEnabled } from "../shared";
|
||||||
|
|
||||||
type AgentWithPermission = { permission?: Record<string, unknown> };
|
type AgentWithPermission = { permission?: Record<string, unknown> };
|
||||||
|
|
||||||
@@ -25,7 +26,7 @@ export function applyToolConfig(params: {
|
|||||||
pluginConfig: OhMyOpenCodeConfig;
|
pluginConfig: OhMyOpenCodeConfig;
|
||||||
agentResult: Record<string, unknown>;
|
agentResult: Record<string, unknown>;
|
||||||
}): void {
|
}): void {
|
||||||
const taskSystemEnabled = params.pluginConfig.experimental?.task_system ?? false
|
const taskSystemEnabled = isTaskSystemEnabled(params.pluginConfig)
|
||||||
const denyTodoTools = taskSystemEnabled
|
const denyTodoTools = taskSystemEnabled
|
||||||
? { todowrite: "deny", todoread: "deny" }
|
? { todowrite: "deny", todoread: "deny" }
|
||||||
: {}
|
: {}
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ import { describe, expect, test } from "bun:test"
|
|||||||
import { tool } from "@opencode-ai/plugin"
|
import { tool } from "@opencode-ai/plugin"
|
||||||
|
|
||||||
import type { ToolsRecord } from "./types"
|
import type { ToolsRecord } from "./types"
|
||||||
import { trimToolsToCap } from "./tool-registry"
|
import { createToolRegistry, trimToolsToCap } from "./tool-registry"
|
||||||
|
|
||||||
const fakeTool = tool({
|
const fakeTool = tool({
|
||||||
description: "test tool",
|
description: "test tool",
|
||||||
@@ -27,3 +27,57 @@ describe("#given tool trimming prioritization", () => {
|
|||||||
expect(filteredTools).toHaveProperty("read")
|
expect(filteredTools).toHaveProperty("read")
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("#given task_system configuration", () => {
|
||||||
|
test("#when task_system is omitted #then task tools are not registered by default", () => {
|
||||||
|
const result = createToolRegistry({
|
||||||
|
ctx: { directory: "/tmp" } as Parameters<typeof createToolRegistry>[0]["ctx"],
|
||||||
|
pluginConfig: {},
|
||||||
|
managers: {
|
||||||
|
backgroundManager: {},
|
||||||
|
tmuxSessionManager: {},
|
||||||
|
skillMcpManager: {},
|
||||||
|
} as Parameters<typeof createToolRegistry>[0]["managers"],
|
||||||
|
skillContext: {
|
||||||
|
mergedSkills: [],
|
||||||
|
availableSkills: [],
|
||||||
|
browserProvider: "playwright",
|
||||||
|
disabledSkills: new Set(),
|
||||||
|
},
|
||||||
|
availableCategories: [],
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(result.taskSystemEnabled).toBe(false)
|
||||||
|
expect(result.filteredTools).not.toHaveProperty("task_create")
|
||||||
|
expect(result.filteredTools).not.toHaveProperty("task_get")
|
||||||
|
expect(result.filteredTools).not.toHaveProperty("task_list")
|
||||||
|
expect(result.filteredTools).not.toHaveProperty("task_update")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("#when task_system is enabled #then task tools are registered", () => {
|
||||||
|
const result = createToolRegistry({
|
||||||
|
ctx: { directory: "/tmp" } as Parameters<typeof createToolRegistry>[0]["ctx"],
|
||||||
|
pluginConfig: {
|
||||||
|
experimental: { task_system: true },
|
||||||
|
},
|
||||||
|
managers: {
|
||||||
|
backgroundManager: {},
|
||||||
|
tmuxSessionManager: {},
|
||||||
|
skillMcpManager: {},
|
||||||
|
} as Parameters<typeof createToolRegistry>[0]["managers"],
|
||||||
|
skillContext: {
|
||||||
|
mergedSkills: [],
|
||||||
|
availableSkills: [],
|
||||||
|
browserProvider: "playwright",
|
||||||
|
disabledSkills: new Set(),
|
||||||
|
},
|
||||||
|
availableCategories: [],
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(result.taskSystemEnabled).toBe(true)
|
||||||
|
expect(result.filteredTools).toHaveProperty("task_create")
|
||||||
|
expect(result.filteredTools).toHaveProperty("task_get")
|
||||||
|
expect(result.filteredTools).toHaveProperty("task_list")
|
||||||
|
expect(result.filteredTools).toHaveProperty("task_update")
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|||||||
@@ -29,7 +29,7 @@ import {
|
|||||||
} from "../tools"
|
} from "../tools"
|
||||||
import { getMainSessionID } from "../features/claude-code-session-state"
|
import { getMainSessionID } from "../features/claude-code-session-state"
|
||||||
import { filterDisabledTools } from "../shared/disabled-tools"
|
import { filterDisabledTools } from "../shared/disabled-tools"
|
||||||
import { log } from "../shared"
|
import { isTaskSystemEnabled, log } from "../shared"
|
||||||
|
|
||||||
import type { Managers } from "../create-managers"
|
import type { Managers } from "../create-managers"
|
||||||
import type { SkillContext } from "./skill-context"
|
import type { SkillContext } from "./skill-context"
|
||||||
@@ -175,8 +175,7 @@ export function createToolRegistry(args: {
|
|||||||
nativeSkills: "skills" in ctx ? (ctx as { skills: SkillLoadOptions["nativeSkills"] }).skills : undefined,
|
nativeSkills: "skills" in ctx ? (ctx as { skills: SkillLoadOptions["nativeSkills"] }).skills : undefined,
|
||||||
})
|
})
|
||||||
|
|
||||||
// task_system defaults to true since v3.14 — delegation (oracle, subagents) requires it
|
const taskSystemEnabled = isTaskSystemEnabled(pluginConfig)
|
||||||
const taskSystemEnabled = pluginConfig.experimental?.task_system ?? true
|
|
||||||
const taskToolsRecord: Record<string, ToolDefinition> = taskSystemEnabled
|
const taskToolsRecord: Record<string, ToolDefinition> = taskSystemEnabled
|
||||||
? {
|
? {
|
||||||
task_create: createTaskCreateTool(pluginConfig, ctx),
|
task_create: createTaskCreateTool(pluginConfig, ctx),
|
||||||
|
|||||||
@@ -72,3 +72,4 @@ export * from "./plugin-command-discovery"
|
|||||||
export { SessionCategoryRegistry } from "./session-category-registry"
|
export { SessionCategoryRegistry } from "./session-category-registry"
|
||||||
export * from "./plugin-identity"
|
export * from "./plugin-identity"
|
||||||
export * from "./log-legacy-plugin-startup-warning"
|
export * from "./log-legacy-plugin-startup-warning"
|
||||||
|
export * from "./task-system-enabled"
|
||||||
|
|||||||
@@ -0,0 +1,9 @@
|
|||||||
|
export interface TaskSystemConfig {
|
||||||
|
experimental?: {
|
||||||
|
task_system?: boolean
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
export function isTaskSystemEnabled(config: TaskSystemConfig): boolean {
|
||||||
|
return config.experimental?.task_system ?? false
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user