feat: Task ID semantics and lifecycle integrity (#3404, #3405)#3409
Open
garnetlyx wants to merge 1 commit intocode-yeongyu:devfrom
Open
feat: Task ID semantics and lifecycle integrity (#3404, #3405)#3409garnetlyx wants to merge 1 commit intocode-yeongyu:devfrom
garnetlyx wants to merge 1 commit intocode-yeongyu:devfrom
Conversation
Contributor
|
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.
src/shared/id-types.ts
Outdated
| 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
Author
|
I have read the CLA Document and I hereby sign the CLA |
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
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
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.