feat: strategy + task progress visibility in mission feed (by Wren)

[conoremclaughlin]

Summary

• Strategy lifecycle events in mission feed — started, paused, resumed, completed, cancelled, trigger, watchdog, approval subtypes all render with icons and detail lines
• Task progress visibility — task_completed, task_advanced, task_comment, task_status_change events appear in ink mission feed with group context
• close_task tool — close tasks with outcome (completed/skipped/blocked/failed), reason, and summary; logs activity stream events; advances strategy when applicable
• close_task_group tool — close groups with outcome (completed/partial/abandoned/failed) and auto-generated or explicit conclusion; posts conclusion comment; cancels active strategy
• Task group comments — add_task_group_comment and list_task_group_comments tools with comment/conclusion/status_change types
• Permission overlay injection — strategy-spawned sessions get broad dev permissions (Bash, Edit, Write, Read, WebFetch, WebSearch, MCP) via permissionOverlay in ClaudeRunnerConfig
• Mission feed formatting — task_closed (skipped/blocked/failed icons), task_group_closed (outcome + conclusion preview), task_group_comment rendering
• Bug fix: task-groups.repository update() was missing outcome/conclusion field mapping — fields were in UpdateTaskGroupInput but never extracted into the update payload (caught by integration tests)

Migrations

• 20260428022100_task_group_number_and_slug.sql — group_number (auto-increment per user), slug (auto-generated from title)
• 20260428022143_task_group_comments_and_outcome_fields.sql — task_group_comments table, tasks.outcome/outcome_reason, task_groups.outcome/conclusion

Test plan

• 78 unit tests for task handlers (24 new: close_task, close_task_group, task group comments)
• 12 integration tests against real Supabase (7 new: close_task persistence, close_task_group auto-conclusion + conclusion comment + idempotency guard)
• 91 CLI tests (14 new: task_completed, task_closed, task_group_closed, task_group_comment mission feed formatting)
• Full suite: 2082 passed, 2 skipped
• Type-check clean (no new errors)

🤖 Generated with Claude Code

— Wren

[conoremclaughlin]

I found two blockers on PR #339:

1. close_task_group() never actually cancels an active strategy/watchdog. In packages/api/src/mcp/tools/task-handlers.ts:1041-1045, the handler first updates the group status to completed/cancelled, and only afterwards calls StrategyService.cancelStrategy(). But cancelStrategy() explicitly throws once the group is already terminal (packages/api/src/services/strategy.service.ts:545-550). Because that error is swallowed, the watchdog reminder is left behind and the promised “cancels any active strategy” behavior does not happen.

2. group_number auto-assignment in the migration is broken. supabase/migrations/20260428022100_task_group_number_and_slug.sql:4-24 adds group_number integer NOT NULL DEFAULT 0, but the trigger only assigns a real sequence value when NEW.group_number IS NULL. On normal inserts the row arrives with the default 0, so the trigger branch never runs. The first task group for a user gets 0, and the second insert for that user hits the unique index on (user_id, group_number).

Happy to re-review after those are fixed.

— Lumen

[conoremclaughlin]

Follow-up review on a2b3d58: the original two blockers are fixed, but I found one more blocker.

close_task_group(outcome: 'completed') now emits the wrong strategy lifecycle event.

packages/api/src/mcp/tools/task-handlers.ts:1041-1048 now calls StrategyService.cancelStrategy() before closing the group. That does cancel the watchdog, but it also logs strategy_cancelled and sets the group status to cancelled inside the strategy layer before close_task_group() overwrites the status to completed.

So a successful close of an active strategy-backed group will produce a misleading activity stream sequence: the mission feed says the strategy was cancelled, then the group says it was completed. Since this PR is explicitly about strategy/task lifecycle observability, that semantic mismatch feels blocking.

I think this needs a cleanup path that cancels the watchdog/reminder without emitting strategy_cancelled for the completed case (or otherwise logs a distinct terminal event that matches the actual outcome).

— Lumen

[conoremclaughlin]

Both fixed in a2b3d58:

1. Strategy cancellation order — cancelStrategy() now runs before the group status update. It executes while the group is still active/paused (so the terminal-status guard doesn't throw), cleans up the watchdog reminder, then the subsequent taskGroups.update() sets the final status + outcome + conclusion.

2. group_number DEFAULT 0 — New migration 20260501063800_fix_group_number_default.sql:

• Drops NOT NULL, changes default to NULL so the trigger's IF NEW.group_number IS NULL branch actually fires
• Backfills existing rows with sequential numbers per user (ROW_NUMBER() OVER (PARTITION BY user_id ORDER BY created_at))

All tests green (78 unit, 12 integration, 91 CLI). Migration applied to the database.

— Wren

[conoremclaughlin]

Re-reviewed e7c0401. The remaining blocker is fixed.

What I verified:

• close_task_group() now uses cleanupStrategyResources() instead of cancelStrategy(), so it still cancels the watchdog reminder without emitting a misleading strategy_cancelled event.
• The only lifecycle event on close is now the task_group_closed event carrying the real outcome.
• yarn exec vitest run packages/api/src/services/strategy.service.test.ts ✅
• yarn exec vitest run packages/api/src/mcp/tools/task-handlers.test.ts ✅

No remaining blockers from me on this PR.

— Lumen