Implement unified Claude Tasks system with single multi-action tool #1356
+2,070
−684
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
Remove mailbox types and swarm config schema. Update docs. 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
Remove old storage and types implementation, replaced by claude-tasks. 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
- Task schema with Zod validation (pending, in_progress, completed, deleted) - Storage utilities: getTaskDir, readJsonSafe, writeJsonAtomic, acquireLock - Atomic writes with temp file + rename - File-based locking with 30s stale threshold 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
Add Zod schemas for task CRUD operations input validation. 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
Create new tasks with sequential ID generation and lock-based concurrency. 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
Retrieve task by ID with null-safe handling. 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
Update tasks with status transitions and owner claim validation. 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
- TaskList for summary view of all tasks - Export all claude-tasks tool factories from index 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
Remind agents to use task tools after 10 turns without task operations. 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
… hook - Add disabled_tools config option to disable specific tools by name - Register tasks-todowrite-disabler hook name in schema 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
Grant task_* and teammate permissions to atlas, sisyphus, prometheus, and sisyphus-junior agents. 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
Add optional execute field with task_id and task_dir for task-based delegation. 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
Prevent crashes when output is not a string by adding typeof checks. 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
- Export SisyphusConfig and SisyphusTasksConfig types - Add task_tool to TARGET_TOOLS list 🤖 Generated with [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
- Add new_task_system_enabled to OhMyOpenCodeConfigSchema - Remove enabled from SisyphusTasksConfigSchema (keep storage_path, claude_code_compat) - Update index.ts to gate on new_task_system_enabled - Update plugin-config.ts default for config initialization - Update test configs in task.test.ts and storage.test.ts Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <[email protected]>
- Add explicit ToolDefinition return type to createTask function - Fix planDemoteConfig to use 'subagent' mode instead of 'all'
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 32 files
Confidence score: 2/5
- High-severity security risk in
src/tools/task/task.ts: unsanitizedidenables path traversal and deletion of arbitrary JSON files viahandleDelete. - Concurrency handling is fragile:
acquireLockresults are ignored insrc/tools/task/task.ts, andsrc/features/claude-tasks/storage.tshas non-atomic lock acquisition and unsafe release, risking data corruption. - Score is low because multiple high-severity issues (security and locking) are concrete and likely to impact users.
- Pay close attention to
src/tools/task/task.tsandsrc/features/claude-tasks/storage.ts- path traversal deletion and lock race/ownership handling.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/features/claude-tasks/storage.ts">
<violation number="1" location="src/features/claude-tasks/storage.ts:78">
P1: Race condition in lock acquisition. The pattern `if (existsSync) ... writeFileSync` is not atomic. Two processes can simultaneously check, find no lock, and both write, causing a race where both believe they acquired the lock.
Use `writeFileSync` with the `{ flag: 'wx' }` option to ensure atomic creation, and handle the `EEXIST` error to check for staleness.</violation>
<violation number="2" location="src/features/claude-tasks/storage.ts:105">
P2: Unsafe lock release mechanism. The `release` function unconditionally deletes the lock file. If a lock was stolen by another process (due to staleness), the original owner will delete the new owner's lock upon completion.
Store a unique ID in the lock file and verify it matches before deletion.</violation>
</file>
<file name="src/features/claude-tasks/types.test.ts">
<violation number="1" location="src/features/claude-tasks/types.test.ts:20">
P1: The schema tested here is inconsistent with the implementation in `src/tools/task/task.ts`. The test explicitly rejects "open" as a status and requires `subject`/`blockedBy`, whereas the tool uses `status: "open"` and `title`/`dependsOn`.
The tool implementation must be updated to use the unified schema defined in `src/features/claude-tasks/types.ts` to match the design spec and these tests.</violation>
</file>
<file name="src/hooks/task-reminder/index.ts">
<violation number="1" location="src/hooks/task-reminder/index.ts:5">
P2: The reminder message instructs the model to use `TaskCreate` and `TaskUpdate`, but the PR replaces separate tools with a single `task` tool using actions (`create`, `update`).
Using nonexistent tool names may cause hallucinations or tool use errors. Update the message to reference the `task` tool with specific actions.</violation>
<violation number="2" location="src/hooks/task-reminder/index.ts:20">
P2: Session counters are stored in a Map but never cleaned up, leading to a memory leak as sessions accumulate.
Add a 'session.deleted' event handler to remove session IDs from `sessionCounters` when sessions end, following the pattern in `src/hooks/context-window-monitor.ts`.</violation>
</file>
<file name="src/tools/task/task.ts">
<violation number="1" location="src/tools/task/task.ts:89">
P1: Concurrency Issue: The result of `acquireLock` is ignored. If the lock cannot be acquired (e.g. another process is writing), this code proceeds anyway, leading to potential data corruption. Check `lock.acquired` and abort if false.</violation>
<violation number="2" location="src/tools/task/task.ts:180">
P0: Security Risk: The `id` parameter is not sanitized, allowing path traversal (e.g. `../package`). In `handleDelete`, this vulnerability permits the deletion of any JSON file on the system (like `package.json`). Validate that the ID contains only safe characters.</violation>
</file>
<file name="src/plugin-config.ts">
<violation number="1" location="src/plugin-config.ts:116">
P1: Configuration merge logic is flawed for this field. Because `new_task_system_enabled` has a default value in the schema, `loadConfigFromPath` returns a value (false) even when the field is missing in the project config. This causes `mergeConfigs` to overwrite any user-level setting (e.g., true) with the project-level default (false).
Recommended fix: Remove `.default(false)` from the schema (make it optional) and apply the default value after merging.</violation>
</file>
<file name="src/features/claude-tasks/AGENTS.md">
<violation number="1" location="src/features/claude-tasks/AGENTS.md:43">
P2: Function signature mismatch: `getTaskDir` takes only `config`, not `teamName`.
The implementation in `src/features/claude-tasks/storage.ts` is `getTaskDir(config)`. It does not accept a team name and does not append it to the path.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| ): Promise<string> { | ||
| const validatedArgs = TaskGetInputSchema.parse(args) | ||
| const taskDir = getTaskDir(config) | ||
| const taskPath = join(taskDir, `${validatedArgs.id}.json`) |
There was a problem hiding this comment.
P0: Security Risk: The id parameter is not sanitized, allowing path traversal (e.g. ../package). In handleDelete, this vulnerability permits the deletion of any JSON file on the system (like package.json). Validate that the ID contains only safe characters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/task/task.ts, line 180:
<comment>Security Risk: The `id` parameter is not sanitized, allowing path traversal (e.g. `../package`). In `handleDelete`, this vulnerability permits the deletion of any JSON file on the system (like `package.json`). Validate that the ID contains only safe characters.</comment>
<file context>
@@ -0,0 +1,253 @@
+): Promise<string> {
+ const validatedArgs = TaskGetInputSchema.parse(args)
+ const taskDir = getTaskDir(config)
+ const taskPath = join(taskDir, `${validatedArgs.id}.json`)
+
+ const task = readJsonSafe(taskPath, TaskObjectSchema)
</file context>
| const lockPath = join(dirPath, ".lock") | ||
| const now = Date.now() | ||
|
|
||
| if (existsSync(lockPath)) { |
There was a problem hiding this comment.
P1: Race condition in lock acquisition. The pattern if (existsSync) ... writeFileSync is not atomic. Two processes can simultaneously check, find no lock, and both write, causing a race where both believe they acquired the lock.
Use writeFileSync with the { flag: 'wx' } option to ensure atomic creation, and handle the EEXIST error to check for staleness.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/claude-tasks/storage.ts, line 78:
<comment>Race condition in lock acquisition. The pattern `if (existsSync) ... writeFileSync` is not atomic. Two processes can simultaneously check, find no lock, and both write, causing a race where both believe they acquired the lock.
Use `writeFileSync` with the `{ flag: 'wx' }` option to ensure atomic creation, and handle the `EEXIST` error to check for staleness.</comment>
<file context>
@@ -0,0 +1,112 @@
+ const lockPath = join(dirPath, ".lock")
+ const now = Date.now()
+
+ if (existsSync(lockPath)) {
+ try {
+ const lockContent = readFileSync(lockPath, "utf-8")
</file context>
|
|
||
| test("rejects invalid status values", () => { | ||
| //#given | ||
| const invalidStatuses = ["open", "closed", "archived", ""] |
There was a problem hiding this comment.
P1: The schema tested here is inconsistent with the implementation in src/tools/task/task.ts. The test explicitly rejects "open" as a status and requires subject/blockedBy, whereas the tool uses status: "open" and title/dependsOn.
The tool implementation must be updated to use the unified schema defined in src/features/claude-tasks/types.ts to match the design spec and these tests.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/claude-tasks/types.test.ts, line 20:
<comment>The schema tested here is inconsistent with the implementation in `src/tools/task/task.ts`. The test explicitly rejects "open" as a status and requires `subject`/`blockedBy`, whereas the tool uses `status: "open"` and `title`/`dependsOn`.
The tool implementation must be updated to use the unified schema defined in `src/features/claude-tasks/types.ts` to match the design spec and these tests.</comment>
<file context>
@@ -0,0 +1,174 @@
+
+ test("rejects invalid status values", () => {
+ //#given
+ const invalidStatuses = ["open", "closed", "archived", ""]
+
+ //#when
</file context>
| ): Promise<string> { | ||
| const validatedArgs = TaskCreateInputSchema.parse(args) | ||
| const taskDir = getTaskDir(config) | ||
| const lock = acquireLock(taskDir) |
There was a problem hiding this comment.
P1: Concurrency Issue: The result of acquireLock is ignored. If the lock cannot be acquired (e.g. another process is writing), this code proceeds anyway, leading to potential data corruption. Check lock.acquired and abort if false.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/task/task.ts, line 89:
<comment>Concurrency Issue: The result of `acquireLock` is ignored. If the lock cannot be acquired (e.g. another process is writing), this code proceeds anyway, leading to potential data corruption. Check `lock.acquired` and abort if false.</comment>
<file context>
@@ -0,0 +1,253 @@
+): Promise<string> {
+ const validatedArgs = TaskCreateInputSchema.parse(args)
+ const taskDir = getTaskDir(config)
+ const lock = acquireLock(taskDir)
+
+ try {
</file context>
| // Load user config first (base) | ||
| let config: OhMyOpenCodeConfig = | ||
| loadConfigFromPath(userConfigPath, ctx) ?? {}; | ||
| loadConfigFromPath(userConfigPath, ctx) ?? { new_task_system_enabled: false }; |
There was a problem hiding this comment.
P1: Configuration merge logic is flawed for this field. Because new_task_system_enabled has a default value in the schema, loadConfigFromPath returns a value (false) even when the field is missing in the project config. This causes mergeConfigs to overwrite any user-level setting (e.g., true) with the project-level default (false).
Recommended fix: Remove .default(false) from the schema (make it optional) and apply the default value after merging.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/plugin-config.ts, line 116:
<comment>Configuration merge logic is flawed for this field. Because `new_task_system_enabled` has a default value in the schema, `loadConfigFromPath` returns a value (false) even when the field is missing in the project config. This causes `mergeConfigs` to overwrite any user-level setting (e.g., true) with the project-level default (false).
Recommended fix: Remove `.default(false)` from the schema (make it optional) and apply the default value after merging.</comment>
<file context>
@@ -113,7 +113,7 @@ export function loadPluginConfig(
// Load user config first (base)
let config: OhMyOpenCodeConfig =
- loadConfigFromPath(userConfigPath, ctx) ?? {};
+ loadConfigFromPath(userConfigPath, ctx) ?? { new_task_system_enabled: false };
// Override with project config
</file context>
| release: () => { | ||
| try { | ||
| if (existsSync(lockPath)) { | ||
| unlinkSync(lockPath) |
There was a problem hiding this comment.
P2: Unsafe lock release mechanism. The release function unconditionally deletes the lock file. If a lock was stolen by another process (due to staleness), the original owner will delete the new owner's lock upon completion.
Store a unique ID in the lock file and verify it matches before deletion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/claude-tasks/storage.ts, line 105:
<comment>Unsafe lock release mechanism. The `release` function unconditionally deletes the lock file. If a lock was stolen by another process (due to staleness), the original owner will delete the new owner's lock upon completion.
Store a unique ID in the lock file and verify it matches before deletion.</comment>
<file context>
@@ -0,0 +1,112 @@
+ release: () => {
+ try {
+ if (existsSync(lockPath)) {
+ unlinkSync(lockPath)
+ }
+ } catch {
</file context>
|
|
||
| const TASK_TOOLS = new Set(["task"]) | ||
| const TURN_THRESHOLD = 10 | ||
| const REMINDER_MESSAGE = ` |
There was a problem hiding this comment.
P2: The reminder message instructs the model to use TaskCreate and TaskUpdate, but the PR replaces separate tools with a single task tool using actions (create, update).
Using nonexistent tool names may cause hallucinations or tool use errors. Update the message to reference the task tool with specific actions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/task-reminder/index.ts, line 5:
<comment>The reminder message instructs the model to use `TaskCreate` and `TaskUpdate`, but the PR replaces separate tools with a single `task` tool using actions (`create`, `update`).
Using nonexistent tool names may cause hallucinations or tool use errors. Update the message to reference the `task` tool with specific actions.</comment>
<file context>
@@ -0,0 +1,45 @@
+
+const TASK_TOOLS = new Set(["task"])
+const TURN_THRESHOLD = 10
+const REMINDER_MESSAGE = `
+
+The task tools haven't been used recently. If you're working on tasks that would benefit from tracking progress, consider using TaskCreate to add new tasks and TaskUpdate to update task status (set to in_progress when starting, completed when done).`
</file context>
| } | ||
|
|
||
| export function createTaskReminderHook(_ctx: PluginInput) { | ||
| const sessionCounters = new Map<string, number>() |
There was a problem hiding this comment.
P2: Session counters are stored in a Map but never cleaned up, leading to a memory leak as sessions accumulate.
Add a 'session.deleted' event handler to remove session IDs from sessionCounters when sessions end, following the pattern in src/hooks/context-window-monitor.ts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/task-reminder/index.ts, line 20:
<comment>Session counters are stored in a Map but never cleaned up, leading to a memory leak as sessions accumulate.
Add a 'session.deleted' event handler to remove session IDs from `sessionCounters` when sessions end, following the pattern in `src/hooks/context-window-monitor.ts`.</comment>
<file context>
@@ -0,0 +1,45 @@
+}
+
+export function createTaskReminderHook(_ctx: PluginInput) {
+ const sessionCounters = new Map<string, number>()
+
+ const toolExecuteAfter = async (input: ToolExecuteInput, output: ToolExecuteOutput) => {
</file context>
|
|
||
| ## STORAGE UTILITIES | ||
|
|
||
| ### getTaskDir(teamName, config) |
There was a problem hiding this comment.
P2: Function signature mismatch: getTaskDir takes only config, not teamName.
The implementation in src/features/claude-tasks/storage.ts is getTaskDir(config). It does not accept a team name and does not append it to the path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/claude-tasks/AGENTS.md, line 43:
<comment>Function signature mismatch: `getTaskDir` takes only `config`, not `teamName`.
The implementation in `src/features/claude-tasks/storage.ts` is `getTaskDir(config)`. It does not accept a team name and does not append it to the path.</comment>
<file context>
@@ -0,0 +1,102 @@
+
+## STORAGE UTILITIES
+
+### getTaskDir(teamName, config)
+
+Returns: `.sisyphus/tasks/{teamName}` (or custom path from config)
</file context>
This was referenced Feb 1, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Changes
New Features
src/tools/task/): Single tool with 5 actions (create,list,get,update,delete) using action-based dispatch patternT-{uuid}task IDsthreadIDfor session trackingdependsOnfieldreadyfilter for tasks with all dependencies completedsrc/hooks/task-reminder/): Reminds agents about pending tasks after tool executionssrc/features/claude-tasks/): Flat directory structure with JSON-based task files and atomic writesConfiguration
new_task_system_enabledtop-level flag in config schemadisabled_toolssetting for disabling specific toolsCleanup
src/features/sisyphus-swarm/(mailbox system)src/features/sisyphus-tasks/(legacy task system)Other
executeoption in delegate-task typesTesting
bun run typecheck bun testAll 768 lines of new tests for the unified Task tool covering:
readyflagRelated Issues
Summary by cubic
Unifies task management into a single “task” tool with create/list/get/update/delete actions and adds a reminder hook to nudge agents to track work. Removes the legacy Sisyphus Tasks/Swarm and introduces safer storage with atomic writes and file locking.
New Features
Migration
Written for commit 6acd5e8. Summary will update on new commits.