fix(task): improve task lifecycle UX with 6 targeted fixes

[josephgoksu]

Summary

Addresses 6 task lifecycle UX defects identified during real-world plan execution:

• session_id auto-inference: Infer from hook_session.json when MCP transport doesn't provide one — eliminates the friction of every task action=next failing on first attempt
• Task skip action: New skip action (pending/in_progress → skipped) with reason tracking, plus skipped tasks treated as resolved in dependency graph
• Hook resilience: continue-check gracefully degrades with /tw-next fallback instead of failing silently when repo can't be opened
• Explicit tasks passthrough: plan action=generate now respects caller-provided tasks array, bypassing LLM generation entirely
• Anti-overlap prompts: Planner, decomposer, and expander agents now instructed to merge overlapping tasks instead of splitting implementation/testing
• StatusSkipped model: Added skipped status to task lifecycle state machine

Changed files

File	Change
cmd/mcp_server.go	session_id fallback from hook_session.json, skip action docs
cmd/hook.go	Graceful error handling with /tw-next fallback
internal/mcp/types.go	TaskActionSkip enum value
internal/mcp/handlers.go	handleTaskSkip handler, explicit tasks passthrough
internal/memory/task_store.go	SkipTask() method, dependency query fix
internal/memory/repository_tasks.go	SkipTask wrapper
internal/task/models.go	StatusSkipped constant
internal/app/plan.go	ExplicitTask type, early return path
internal/config/prompts.go	Anti-overlap instructions in 3 agent prompts

Test plan

• go build ./... passes
• go test ./internal/memory/... ./internal/mcp/... ./internal/task/... ./internal/config/... ./internal/app/... — 19 tests pass
• Verify task action=skip via MCP with a pending task
• Verify task action=next works without explicit session_id
• Verify plan action=generate with explicit tasks array bypasses LLM
• Verify hook continue-check outputs /tw-next fallback on repo error

Greptile Summary

This PR delivers 6 targeted UX improvements to the task lifecycle: session ID auto-inference from hook_session.json, a new skip action with dependency-graph awareness, hook graceful degradation with /tw-next fallback hints, explicit tasks passthrough for plan action=generate, anti-overlap prompt instructions across all three agent prompts, and the StatusSkipped model constant.

The core mechanics are correct — the SQL dependency fix (NOT IN (completed, skipped)), the SkipTask lifecycle implementation, and the session inference fallback all work as intended. However, there are several documentation gaps that create usability friction:

• Stale struct comment: TaskToolParams struct comment omits the newly added skip action, leaving callers without documentation of what it does or what fields it requires.
• Undocumented dual-use: PlanToolParams.Tasks field is documented only for expand but is now silently also consumed by generate with entirely different semantics (user edits vs. LLM-bypass), creating a trap for callers who won't know what behavior to expect.
• DRY violation: ExplicitTask in internal/app/plan.go is structurally identical to TaskInput in internal/mcp/types.go with a manual field-by-field mapping that creates maintenance burden — any future field additions must be made in three places.
• Stale schema comment: The inline SQL comment for the status column in sqlite.go still lists the old set of allowed values and doesn't include skipped.

Confidence Score: 4/5

• Safe to merge — no runtime-breaking bugs found; all logic is correct. Issues are documentation drift and a DRY violation in types.
• Score held to 4 (not 5) due to: (1) Stale/incomplete API documentation (TaskToolParams struct comment omits skip action, PlanToolParams.Tasks field silently dual-purposed for two actions with different semantics but only one documented); (2) DRY violation in type design (ExplicitTask duplicates mcp.TaskInput structure with manual field-by-field mapping). All logic changes are correct: SQL parameter count matches placeholders, SkipTask rows-affected guard prevents silent no-ops, session inference is properly nil/empty-guarded, explicit tasks bypass is cleanly isolated.
• internal/mcp/types.go (stale struct comment and undocumented dual-use of Tasks field), internal/app/plan.go (ExplicitTask DRY violation)

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([pending]) -->|task action=start| B([in_progress])
    A -->|task action=skip| E([skipped])
    B -->|task action=complete| C([verifying])
    B -->|task action=skip| E
    C -->|validation pass| D([completed])
    C -->|validation fail| F([failed])

    subgraph Dependency Resolution
        D -->|unblocks dependents| G{GetNextTask}
        E -->|treated as resolved| G
        G -->|NOT IN completed,skipped| H([next pending task])
    end

    subgraph Session ID Inference
        I[MCP tool call] --> J{session.ID available?}
        J -->|yes| K[use transport session ID]
        J -->|no| L{hook_session.json exists?}
        L -->|yes, SessionID != empty| M[use hook session ID]
        L -->|no| N[empty session ID / caller must provide]
    end

Loading

Comments Outside Diff (3)

1. internal/mcp/types.go, line 141-152 (link)

   The TaskToolParams struct comment still lists only next, current, start, complete and omits the newly added skip action. This leaves callers without documentation of what skip does or what fields it requires.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: internal/mcp/types.go
   Line: 141-152
   
   Comment:
   The `TaskToolParams` struct comment still lists only `next, current, start, complete` and omits the newly added `skip` action. This leaves callers without documentation of what `skip` does or what fields it requires.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

   

2. internal/mcp/types.go, line 335-337 (link)

   PlanToolParams.Tasks is now silently used by both expand and generate actions with completely different semantics, but the documentation only mentions expand. This undocumented dual-use creates a trap for callers: if they send tasks to generate expecting expand-style feedback behavior, they'll instead trigger LLM-bypass behavior.

   Also update the PlanToolParams required-fields comment block to document tasks as optional for both actions.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: internal/mcp/types.go
   Line: 335-337
   
   Comment:
   `PlanToolParams.Tasks` is now silently used by both `expand` and `generate` actions with completely different semantics, but the documentation only mentions `expand`. This undocumented dual-use creates a trap for callers: if they send `tasks` to `generate` expecting `expand`-style feedback behavior, they'll instead trigger LLM-bypass behavior.
   
   
   
   Also update the `PlanToolParams` required-fields comment block to document `tasks` as optional for both actions.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

3. internal/memory/sqlite.go, line 199 (link)

   The inline schema comment lists allowed status values but was not updated to include skipped.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: internal/memory/sqlite.go
   Line: 199
   
   Comment:
   The inline schema comment lists allowed status values but was not updated to include `skipped`.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

   

_{Last reviewed commit: cb4e9db}

Greptile also left 1 inline comment on this PR.

[josephgoksu]

Addressed Greptile review comment (DRY violation for ExplicitTask/TaskInput): moved TaskInput to the shared task package, aliased in mcp/types.go, and removed the manual field-by-field mapping in handlers.go. See commit dbe41ba.