feat: Task ID semantics and lifecycle integrity (#3404, #3405)#3409
feat: Task ID semantics and lifecycle integrity (#3404, #3405)#3409garnetlyx wants to merge 1 commit into
Conversation
|
All contributors have signed the CLA. Thank you! ✅ |
There was a problem hiding this comment.
3 issues found across 16 files
Confidence score: 2/5
- I’m scoring this as high risk because there are two high-severity, high-confidence issues (7/10, confidence 9/10) with concrete user and security impact in
src/tools/task/task-update.ts. src/tools/task/task-update.tsuses untrustedblockedByvalues injoin(...)before strict ID validation, which can allow path traversal reads outside the task directory and should be treated as merge-blocking until fixed.src/tools/task/task-update.tscan incorrectly allowstatus="completed"whenaddBlockedByis included in the same update, since blocker checks run before merging new blockers;src/shared/id-types.tsalso misses hyphenated session IDs likeses-main, causing incorrect not-found handling.- Pay close attention to
src/tools/task/task-update.ts,src/shared/id-types.ts- path traversal/input validation and completion-guard ordering are the main risks, plus session ID format parsing.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/tools/task/task-update.ts">
<violation number="1" location="src/tools/task/task-update.ts:107">
P1: Completion can be bypassed when `status="completed"` is combined with `addBlockedBy` in the same update, because the guard checks blockers before the new ones are merged.</violation>
<violation number="2" location="src/tools/task/task-update.ts:111">
P1: Validate `blockedBy` IDs before building file paths; the new guard currently uses untrusted strings in `join(...)`, enabling path traversal reads outside the task directory.</violation>
</file>
<file name="src/shared/id-types.ts">
<violation number="1" location="src/shared/id-types.ts:6">
P2: Recognize the hyphenated session IDs used elsewhere in the repo; `ses-main` and similar values currently fall through to the generic not-found message.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| export function detectIdType(id: string): IdType { | ||
| if (id.startsWith("bg_")) return "background" | ||
| if (id.startsWith("T-")) return "task" | ||
| if (id.startsWith("ses_")) return "session" |
There was a problem hiding this comment.
P2: Recognize the hyphenated session IDs used elsewhere in the repo; ses-main and similar values currently fall through to the generic not-found message.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/shared/id-types.ts, line 6:
<comment>Recognize the hyphenated session IDs used elsewhere in the repo; `ses-main` and similar values currently fall through to the generic not-found message.</comment>
<file context>
@@ -0,0 +1,22 @@
+export function detectIdType(id: string): IdType {
+ if (id.startsWith("bg_")) return "background"
+ if (id.startsWith("T-")) return "task"
+ if (id.startsWith("ses_")) return "session"
+ return "unknown"
+}
</file context>
9cbc920 to
71a3f97
Compare
71a3f97 to
dddfc6c
Compare
|
I have read the CLA Document and I hereby sign the CLA |
|
[sisyphus-bot] PR sweep first-pass triage on
Needs rebase + review. Please rebase onto current Assigning |
|
[sisyphus-bot] Hi garnetlyx. 🙏 Thanks for the Task ID semantics and lifecycle integrity work for #3404 / #3408. Picking this back up from the 5/16 triage. The PR is medium-large (644/18 across 14 files) and shows CONFLICTING against current Could you rebase against current |
eb25d29 to
2bfad49
Compare
Summary
Implements GitHub issues #3404 (Task ID semantics) and #3405 (Task lifecycle integrity).
Changes
#3404: Task ID Semantics
src/shared/id-types.ts): AddeddetectIdType()to identify ID types (background/task/session/unknown) andformatIdTypeError()for type-aware error messages.background_output): Now distinguishes session vs task vs background IDs and suggests the correct tool.delegate-task,call-omo-agent,sync-task,background-task) to use unified<task_metadata>format.#3405: Task Lifecycle Integrity
task-update): Prevents marking a task completed if it has unresolved blockedBy dependencies.task-create): Warns (doesn't block) when creating a task with the same subject in the same session.Verification
Summary by cubic
Adds type-aware Task ID handling and enforces dependency rules to prevent ID mix-ups and premature completion. Implements #3404 (Task ID semantics) and #3405 (Task lifecycle integrity).
New Features
detectIdType()andformatIdTypeError()classify IDs (bg_,T-,ses_/ses-) and return helpful hints.background_output: type-aware errors; suggestssession_readforses_*/ses-*andtask_update/task_getforT-...; generic not found forbg_.../unknown.<task_metadata>: emitted only whensession_idis resolved; includessession_idandbackground_task_id.call-omo-agentanddelegate-taskupdated;sync-taskaddstask_idfor foreground tasks.task_update: blockscompletedif anyblockedBytasks are notcompletedordeleted; guard applies even when adding blockers in the same request; invalid IDs count as unresolved.task_create: warns on near-duplicate subjects within the same session (task still created).Migration
<task_metadata>, readbackground_task_idfor background jobs; the block may be omitted untilsession_idis known. Do not rely ontask_idin that block.Written for commit dddfc6c. Summary will update on new commits.