refactor(sisyphus-tasks): migrate tools to OpenCode plugin API #1165
+2,326
−1
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
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
11 issues found across 46 files
Confidence score: 2/5
- Multiple path traversal risks: unvalidated IDs/names are used in filesystem paths in
src/tools/sisyphus-tasks/task-suspend.ts,task-abort.ts,task-remove.ts,src/features/sisyphus-tasks/task-get.ts, andsrc/tools/sisyphus-swarm/teammate-tool.ts, enabling unintended reads/writes/deletes. - Concrete functionality breaks:
src/features/sisyphus-tasks/task-execute.tsnow expects snake_case but callers/tests pass camelCase, andsrc/features/sisyphus-tasks/task-wait.test.tspasses args incorrectly, so task execution can fail at runtime. - Overall risk is high due to security-sensitive filesystem access and user-impacting task execution issues; these look like regression-prone changes.
- Pay close attention to
src/tools/sisyphus-tasks/task-suspend.ts,src/tools/sisyphus-tasks/task-abort.ts,src/tools/sisyphus-tasks/task-remove.ts,src/features/sisyphus-tasks/task-get.ts,src/features/sisyphus-tasks/task-execute.ts,src/features/sisyphus-tasks/task-wait.test.ts,src/tools/sisyphus-swarm/teammate-tool.ts- validate/sanitize inputs and align argument shapes.
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/tools/sisyphus-tasks/task-suspend.ts">
<violation number="1" location="src/tools/sisyphus-tasks/task-suspend.ts:14">
P1: Validate or sanitize `task_id` before using it in a filesystem path to prevent path traversal and unintended file writes.</violation>
</file>
<file name="src/features/sisyphus-tasks/task-execute.ts">
<violation number="1" location="src/features/sisyphus-tasks/task-execute.ts:17">
P2: The tool now only reads snake_case arguments (`task_id`, `agent_id`, `task_dir`), but internal callers/tests pass camelCase (`taskId`, `agentId`, `taskDir`). This will make `args.task_id` undefined and break task execution for existing callers. Consider supporting both naming styles or updating all call sites to snake_case.</violation>
</file>
<file name="src/features/sisyphus-swarm/permission-poller/index.ts">
<violation number="1" location="src/features/sisyphus-swarm/permission-poller/index.ts:64">
P2: startPolling spins on a timer without reading the mailbox or invoking processResponse/onMessage, so permission responses are never handled and callbacks can hang. Wire the poll loop to read unread messages and process permission_response entries.</violation>
</file>
<file name="src/features/sisyphus-tasks/task-resume.ts">
<violation number="1" location="src/features/sisyphus-tasks/task-resume.ts:17">
P2: `readdirSync(taskDir)` will throw if `task_dir` is missing or invalid, causing the tool to crash instead of returning a structured failure. Guard the directory read or fall back to an empty list so the tool can return `task_not_found`.</violation>
</file>
<file name="src/features/sisyphus-tasks/e2e.test.ts">
<violation number="1" location="src/features/sisyphus-tasks/e2e.test.ts:68">
P2: Tool args are snake_case (task_id/task_dir). Passing camelCase args and relying on context means task_id is undefined and task_dir falls back to process.cwd(), so this test will read the wrong file (undefined.json) and can pollute the repo. Pass task_id/task_dir in the args object.</violation>
</file>
<file name="src/tools/sisyphus-tasks/task-update.ts">
<violation number="1" location="src/tools/sisyphus-tasks/task-update.ts:45">
P2: Validate args.status against allowed values before assigning; unchecked strings can corrupt stored tasks and break future reads.</violation>
</file>
<file name="src/tools/sisyphus-tasks/task-abort.ts">
<violation number="1" location="src/tools/sisyphus-tasks/task-abort.ts:15">
P1: Validate task_id to prevent path traversal before constructing the file path; otherwise a crafted task_id can delete arbitrary files outside the task directory.</violation>
</file>
<file name="src/features/sisyphus-tasks/task-wait.test.ts">
<violation number="1" location="src/features/sisyphus-tasks/task-wait.test.ts:24">
P2: Pass the tool arguments using the expected snake_case keys in the single args object. The current call uses `taskId` and a second parameter, so the tool reads from the wrong directory and ignores the task ID, causing this test to fail.</violation>
</file>
<file name="src/tools/sisyphus-tasks/task-remove.ts">
<violation number="1" location="src/tools/sisyphus-tasks/task-remove.ts:15">
P1: Validate `task_id` to prevent path traversal before building the file path; otherwise a caller can delete files outside the task directory.</violation>
</file>
<file name="src/features/sisyphus-tasks/task-get.ts">
<violation number="1" location="src/features/sisyphus-tasks/task-get.ts:14">
P1: Validate task_id to prevent path traversal before joining it into the filesystem path.</violation>
</file>
<file name="src/tools/sisyphus-swarm/teammate-tool.ts">
<violation number="1" location="src/tools/sisyphus-swarm/teammate-tool.ts:19">
P2: Sanitize `team_name`/`name` before using them in filesystem paths to prevent path traversal and unintended writes outside the team directory.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }, | ||
| execute: async (args) => { | ||
| const taskDir = args.task_dir ?? process.cwd() | ||
| const taskPath = join(taskDir, `${args.task_id}.json`) |
There was a problem hiding this comment.
P1: Validate or sanitize task_id before using it in a filesystem path to prevent path traversal and unintended file writes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/sisyphus-tasks/task-suspend.ts, line 14:
<comment>Validate or sanitize `task_id` before using it in a filesystem path to prevent path traversal and unintended file writes.</comment>
<file context>
@@ -0,0 +1,25 @@
+ },
+ execute: async (args) => {
+ const taskDir = args.task_dir ?? process.cwd()
+ const taskPath = join(taskDir, `${args.task_id}.json`)
+ const task = readJsonSafe(taskPath, TaskSchema)
+
</file context>
| }, | ||
| execute: async (args) => { | ||
| const taskDir = args.task_dir ?? process.cwd() | ||
| const taskPath = join(taskDir, `${args.task_id}.json`) |
There was a problem hiding this comment.
P1: Validate task_id to prevent path traversal before constructing the file path; otherwise a crafted task_id can delete arbitrary files outside the task directory.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/sisyphus-tasks/task-abort.ts, line 15:
<comment>Validate task_id to prevent path traversal before constructing the file path; otherwise a crafted task_id can delete arbitrary files outside the task directory.</comment>
<file context>
@@ -0,0 +1,42 @@
+ },
+ execute: async (args) => {
+ const taskDir = args.task_dir ?? process.cwd()
+ const taskPath = join(taskDir, `${args.task_id}.json`)
+
+ if (!existsSync(taskPath)) return JSON.stringify({ success: false })
</file context>
| }, | ||
| execute: async (args) => { | ||
| const taskDir = args.task_dir ?? process.cwd() | ||
| const taskPath = join(taskDir, `${args.task_id}.json`) |
There was a problem hiding this comment.
P1: Validate task_id to prevent path traversal before building the file path; otherwise a caller can delete files outside the task directory.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/sisyphus-tasks/task-remove.ts, line 15:
<comment>Validate `task_id` to prevent path traversal before building the file path; otherwise a caller can delete files outside the task directory.</comment>
<file context>
@@ -0,0 +1,42 @@
+ },
+ execute: async (args) => {
+ const taskDir = args.task_dir ?? process.cwd()
+ const taskPath = join(taskDir, `${args.task_id}.json`)
+
+ if (!existsSync(taskPath)) return JSON.stringify({ success: false })
</file context>
| }, | ||
| execute: async (args) => { | ||
| const taskDir = args.task_dir ?? process.cwd() | ||
| const task = readJsonSafe(join(taskDir, `${args.task_id}.json`), TaskSchema) |
There was a problem hiding this comment.
P1: Validate task_id to prevent path traversal before joining it into the filesystem path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/sisyphus-tasks/task-get.ts, line 14:
<comment>Validate task_id to prevent path traversal before joining it into the filesystem path.</comment>
<file context>
@@ -0,0 +1,17 @@
+ },
+ execute: async (args) => {
+ const taskDir = args.task_dir ?? process.cwd()
+ const task = readJsonSafe(join(taskDir, `${args.task_id}.json`), TaskSchema)
+ return JSON.stringify({ task })
+ },
</file context>
| }, | ||
| execute: async (args) => { | ||
| const taskDir = args.task_dir ?? process.cwd() | ||
| const taskPath = join(taskDir, `${args.task_id}.json`) |
There was a problem hiding this comment.
P2: The tool now only reads snake_case arguments (task_id, agent_id, task_dir), but internal callers/tests pass camelCase (taskId, agentId, taskDir). This will make args.task_id undefined and break task execution for existing callers. Consider supporting both naming styles or updating all call sites to snake_case.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/sisyphus-tasks/task-execute.ts, line 17:
<comment>The tool now only reads snake_case arguments (`task_id`, `agent_id`, `task_dir`), but internal callers/tests pass camelCase (`taskId`, `agentId`, `taskDir`). This will make `args.task_id` undefined and break task execution for existing callers. Consider supporting both naming styles or updating all call sites to snake_case.</comment>
<file context>
@@ -0,0 +1,50 @@
+ },
+ execute: async (args) => {
+ const taskDir = args.task_dir ?? process.cwd()
+ const taskPath = join(taskDir, `${args.task_id}.json`)
+ const task = readJsonSafe(taskPath, TaskSchema)
+
</file context>
| execute: async (args) => { | ||
| const taskDir = args.task_dir ?? process.cwd() | ||
|
|
||
| const files = readdirSync(taskDir).filter((f: string) => f.endsWith(".json")) |
There was a problem hiding this comment.
P2: readdirSync(taskDir) will throw if task_dir is missing or invalid, causing the tool to crash instead of returning a structured failure. Guard the directory read or fall back to an empty list so the tool can return task_not_found.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/sisyphus-tasks/task-resume.ts, line 17:
<comment>`readdirSync(taskDir)` will throw if `task_dir` is missing or invalid, causing the tool to crash instead of returning a structured failure. Guard the directory read or fall back to an empty list so the tool can return `task_not_found`.</comment>
<file context>
@@ -0,0 +1,51 @@
+ execute: async (args) => {
+ const taskDir = args.task_dir ?? process.cwd()
+
+ const files = readdirSync(taskDir).filter((f: string) => f.endsWith(".json"))
+ const busyTasks: string[] = []
+ for (const file of files) {
</file context>
| expect(updateResult.updatedFields).toContain("status") | ||
|
|
||
| //#when: get final task state | ||
| const getResult = await taskGetTool.execute({ taskId: "1" }, context) |
There was a problem hiding this comment.
P2: Tool args are snake_case (task_id/task_dir). Passing camelCase args and relying on context means task_id is undefined and task_dir falls back to process.cwd(), so this test will read the wrong file (undefined.json) and can pollute the repo. Pass task_id/task_dir in the args object.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/sisyphus-tasks/e2e.test.ts, line 68:
<comment>Tool args are snake_case (task_id/task_dir). Passing camelCase args and relying on context means task_id is undefined and task_dir falls back to process.cwd(), so this test will read the wrong file (undefined.json) and can pollute the repo. Pass task_id/task_dir in the args object.</comment>
<file context>
@@ -0,0 +1,183 @@
+ expect(updateResult.updatedFields).toContain("status")
+
+ //#when: get final task state
+ const getResult = await taskGetTool.execute({ taskId: "1" }, context)
+
+ //#then
</file context>
| updatedFields.push("description") | ||
| } | ||
| if (args.status !== undefined) { | ||
| task.status = args.status as Task["status"] |
There was a problem hiding this comment.
P2: Validate args.status against allowed values before assigning; unchecked strings can corrupt stored tasks and break future reads.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/sisyphus-tasks/task-update.ts, line 45:
<comment>Validate args.status against allowed values before assigning; unchecked strings can corrupt stored tasks and break future reads.</comment>
<file context>
@@ -0,0 +1,93 @@
+ updatedFields.push("description")
+ }
+ if (args.status !== undefined) {
+ task.status = args.status as Task["status"]
+ updatedFields.push("status")
+ }
</file context>
| id: "1", subject: "A", description: "D", status: "completed", blocks: [], blockedBy: [] | ||
| })) | ||
|
|
||
| const result = await taskWaitTool.execute({ taskId: "1", timeout: 1000 }, { taskDir }) |
There was a problem hiding this comment.
P2: Pass the tool arguments using the expected snake_case keys in the single args object. The current call uses taskId and a second parameter, so the tool reads from the wrong directory and ignores the task ID, causing this test to fail.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/sisyphus-tasks/task-wait.test.ts, line 24:
<comment>Pass the tool arguments using the expected snake_case keys in the single args object. The current call uses `taskId` and a second parameter, so the tool reads from the wrong directory and ignores the task ID, causing this test to fail.</comment>
<file context>
@@ -0,0 +1,42 @@
+ id: "1", subject: "A", description: "D", status: "completed", blocks: [], blockedBy: []
+ }))
+
+ const result = await taskWaitTool.execute({ taskId: "1", timeout: 1000 }, { taskDir })
+ expect(result.completed).toBe(true)
+ expect(result.task?.status).toBe("completed")
</file context>
| const teamName = args.team_name ?? "default-team" | ||
| const teammateName = args.name ?? `teammate-${Date.now()}` | ||
| const mode = args.mode ?? "default" | ||
| const teamDir = args.team_dir ?? join(process.cwd(), ".sisyphus", "teams", teamName) |
There was a problem hiding this comment.
P2: Sanitize team_name/name before using them in filesystem paths to prevent path traversal and unintended writes outside the team directory.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/sisyphus-swarm/teammate-tool.ts, line 19:
<comment>Sanitize `team_name`/`name` before using them in filesystem paths to prevent path traversal and unintended writes outside the team directory.</comment>
<file context>
@@ -0,0 +1,45 @@
+ const teamName = args.team_name ?? "default-team"
+ const teammateName = args.name ?? `teammate-${Date.now()}`
+ const mode = args.mode ?? "default"
+ const teamDir = args.team_dir ?? join(process.cwd(), ".sisyphus", "teams", teamName)
+
+ const inboxesDir = join(teamDir, "inboxes")
</file context>
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
@opencode-ai/plugin/toolAPIdynamic-truncator.tsChanges
task-update.ts,task-abort.ts,task-create.ts,task-list.ts,task-remove.ts,task-suspend.ts- Convert to OpenCode tool APItask-execute.ts,task-get.ts,task-resume.ts,task-wait.ts- Clean up related logicdynamic-truncator.ts- Strengthen type safety