feat(web): forward SDK lifecycle events to Web UI (tasks + hooks) (#975)#1939
Open
Wirasm wants to merge 1 commit into
Open
feat(web): forward SDK lifecycle events to Web UI (tasks + hooks) (#975)#1939Wirasm wants to merge 1 commit into
Wirasm wants to merge 1 commit into
Conversation
Implements all 4 phases of issue #975: forward Claude SDK subagent task lifecycle and hook callback events to the Web UI through the existing SSE pipeline. Phase 1 — packages/providers/src/{types.ts,claude/provider.ts} 5 new MessageChunk variants (task_started / task_progress / task_notification / hook_started / hook_response) carrying the same payload the SDK emits. Housekeeping tasks flagged with skip_transcript are suppressed at the provider boundary so they never reach the UI. Phase 2 — packages/workflows/src/{event-emitter.ts,dag-executor.ts} + packages/server/src/adapters/web/workflow-bridge.ts Two new WorkflowEmitterEvent variants (task_activity / hook_activity) aggregate the 5 provider chunks into a node-scoped stream; the SSE bridge maps them to workflow_task_activity and workflow_hook_activity. Both are persisted to workflow_events for timeline replay. Slack bridge's exhaustive-check no-op list updated to keep the never-narrow intact. Phase 3 — packages/web/{lib/types.ts,hooks/{useSSE,useDashboardSSE}.ts, stores/workflow-store.ts,components/workflows/{DagNodeProgress, ExecutionDagNode,WorkflowDagViewer}.tsx} New DagTaskInfo / DagHookInfo arrays under each DagNodeState. SSE handlers added; store mutates the same row across the started → progress → completed/failed lifecycle so the user sees one entry that updates in place. DagNodeProgress renders tasks as a collapsible Subagent tasks sub-list and hooks as inline indicators (e.g. 'PreToolUse(Bash) → approved'). ExecutionDagNode surfaces active/total task and hook counts on the graph card so activity is visible without opening the run detail panel. Phase 4 — packages/providers/src/{types.ts,claude/provider.ts} agentProgressSummaries defaults to true for workflow nodes (the agentProgressSummaries field is forwarded to Claude SDK options). Direct chat calls keep the SDK default (false) so the chat surface is unchanged. Authors can opt out per-node with agentProgressSummaries: false in nodeConfig. Tests: 13 new provider tests (chunks + agentProgressSummaries), 6 new bridge mapper tests, 12 new store reducer tests, and new event variants covered in event-emitter. full `bun run validate` passes (type-check + lint + format:check + bundled checks + all 1300+ tests).
📝 WalkthroughWalkthroughThis pull request implements end-to-end forwarding of Claude SDK subagent task and hook lifecycle events to the Web UI. The feature spans nine files across provider, workflow, server, and frontend layers, adding task progress visibility and hook outcome tracking during DAG node execution. ChangesTask and Hook Activity Lifecycle Support
Sequence Diagram(s)sequenceDiagram
participant SDK as Claude SDK
participant Provider as Provider<br/>(streamClaudeMessages)
participant Executor as DAG Executor
participant Emitter as WorkflowEventEmitter
participant Store as WorkflowEventStore
participant SSEBridge as SSE Bridge<br/>(mapWorkflowEvent)
participant Frontend as Frontend Store<br/>(useDashboardSSE)
SDK->>Provider: system event<br/>(task_started, task_progress, hook_started, hook_response)
Provider->>Provider: normalize fields, apply<br/>agentProgressSummaries flag
Provider->>Executor: yield MessageChunk<br/>(task_started, task_progress, hook_started, hook_response)
Executor->>Emitter: emit TaskActivityEvent<br/>or HookActivityEvent
Executor->>Store: persist task_activity<br/>or hook_activity workflow event
Store->>SSEBridge: WorkflowEventEmitter<br/>broadcasts stored event
SSEBridge->>SSEBridge: map to workflow_task_activity<br/>or workflow_hook_activity SSE payload
SSEBridge->>Frontend: send SSE event<br/>with type, taskId/hookId, activity, metadata
Frontend->>Frontend: handleTaskActivity or<br/>handleHookActivity: upsert DagTaskInfo/DagHookInfo on node<br/>tolerate out-of-order, merge partial fields
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/web/src/stores/workflow-store.test.ts (1)
659-680: ⚡ Quick winAdd regression tests for “terminal first, then late started” ordering.
Current tests assert seeding on out-of-order terminal events, but not the follow-up
startedevent. Add cases that sendcompleted/responsefirst, thenstarted, and assert state does not regress from terminal to started.Also applies to: 780-802
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/stores/workflow-store.test.ts` around lines 659 - 680, Add regression tests that send a terminal task notification before a later "started" notification and assert the state does not regress: use the same pattern as the existing test (calling useWorkflowStore.getState().handleWorkflowStatus, .handleDagNode, and .handleTaskActivity with taskEvent for completed/response first), then dispatch a subsequent handleTaskActivity with activity 'started' for the same runId/nodeId/taskId and assert the stored task activity remains 'completed' (or terminal) and not 'started'; add analogous tests for response->started ordering as well, reusing identifiers like run-t5, nodeId 'plan', taskId 't-1' and referencing handleTaskActivity, taskEvent, statusEvent, dagNodeEvent and useWorkflowStore so they’re easy to find, and apply the same additional test case pattern mentioned for the other block around lines 780-802.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/web/src/components/workflows/DagNodeProgress.tsx`:
- Around line 100-108: The toggle button in DagNodeProgress should expose its
expanded state and reference the collapsible container for assistive tech: add
aria-expanded={expanded} and aria-controls pointing to the collapsible section
id on the button (e.g., aria-controls={`details-${id}`}), and add a matching id
on the collapsible container element (use a stable unique id via useId or the
node id), then apply the same changes to the second toggle instance around lines
136-195 so both buttons include aria-expanded and aria-controls with matching
container ids.
In `@packages/web/src/stores/workflow-store.ts`:
- Around line 348-364: The started-event handler currently replaces an existing
task row (using tasks[taskIdx] = newTask), which causes late 'started' events to
regress state and drop fields like summary/usage/lastToolName; instead, when
taskIdx >= 0 merge the new started fields into the existing task object (update
activity/startedAt/updatedAt and only set description/taskType if present)
preserving any existing fields (e.g.,
completed/response/summary/usage/lastToolName); apply the same merge approach to
the other similar blocks referenced (lines handling other activities around the
same file, e.g., the blocks at ~365-394 and ~423-437) so out-of-order events
update fields incrementally rather than overwriting the entire object.
---
Nitpick comments:
In `@packages/web/src/stores/workflow-store.test.ts`:
- Around line 659-680: Add regression tests that send a terminal task
notification before a later "started" notification and assert the state does not
regress: use the same pattern as the existing test (calling
useWorkflowStore.getState().handleWorkflowStatus, .handleDagNode, and
.handleTaskActivity with taskEvent for completed/response first), then dispatch
a subsequent handleTaskActivity with activity 'started' for the same
runId/nodeId/taskId and assert the stored task activity remains 'completed' (or
terminal) and not 'started'; add analogous tests for response->started ordering
as well, reusing identifiers like run-t5, nodeId 'plan', taskId 't-1' and
referencing handleTaskActivity, taskEvent, statusEvent, dagNodeEvent and
useWorkflowStore so they’re easy to find, and apply the same additional test
case pattern mentioned for the other block around lines 780-802.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1bcb2711-5767-4043-8806-760f9e750653
📒 Files selected for processing (19)
packages/adapters/src/chat/slack/workflow-bridge.tspackages/providers/src/claude/provider.test.tspackages/providers/src/claude/provider.tspackages/providers/src/types.tspackages/server/package.jsonpackages/server/src/adapters/web/workflow-bridge.test.tspackages/server/src/adapters/web/workflow-bridge.tspackages/web/src/components/workflows/DagNodeProgress.tsxpackages/web/src/components/workflows/ExecutionDagNode.tsxpackages/web/src/components/workflows/WorkflowDagViewer.tsxpackages/web/src/hooks/useDashboardSSE.tspackages/web/src/hooks/useSSE.tspackages/web/src/lib/types.tspackages/web/src/stores/workflow-store.test.tspackages/web/src/stores/workflow-store.tspackages/workflows/src/dag-executor.tspackages/workflows/src/event-emitter.test.tspackages/workflows/src/event-emitter.tspackages/workflows/src/store.ts
👮 Files not reviewed due to content moderation or server errors (7)
- packages/providers/src/types.ts
- packages/workflows/src/event-emitter.ts
- packages/providers/src/claude/provider.ts
- packages/workflows/src/dag-executor.ts
- packages/workflows/src/event-emitter.test.ts
- packages/server/src/adapters/web/workflow-bridge.ts
- packages/server/src/adapters/web/workflow-bridge.test.ts
Comment on lines
100
to
108
| <button | ||
| type="button" | ||
| onClick={(e): void => { | ||
| e.stopPropagation(); | ||
| setExpanded(prev => !prev); | ||
| }} | ||
| className="text-text-tertiary hover:text-text-secondary shrink-0 text-xs cursor-pointer" | ||
| aria-label={expanded ? 'Collapse iterations' : 'Expand iterations'} | ||
| aria-label={expanded ? 'Collapse details' : 'Expand details'} | ||
| > |
There was a problem hiding this comment.
Expose collapse state to assistive tech on the details toggle.
At Line [100], the toggle button has an aria-label but does not expose expanded state. Add aria-expanded + aria-controls on the button, and an id on the collapsible container so screen readers can track state changes.
Suggested patch
function DagNodeItem({
@@
const hasTasks = (node.tasks?.length ?? 0) > 0;
const hasHooks = (node.hooks?.length ?? 0) > 0;
const hasSubItems = hasIterations || hasTasks || hasHooks;
+ const detailsId = `dag-node-details-${node.nodeId}`;
@@
{hasSubItems && (
<button
type="button"
onClick={(e): void => {
e.stopPropagation();
setExpanded(prev => !prev);
}}
className="text-text-tertiary hover:text-text-secondary shrink-0 text-xs cursor-pointer"
aria-label={expanded ? 'Collapse details' : 'Expand details'}
+ aria-expanded={expanded}
+ aria-controls={detailsId}
>
{expanded ? '\u25BC' : '\u25B6'}
</button>
)}
@@
{expanded && hasSubItems && (
- <div className="ml-6 mt-0.5 space-y-1">
+ <div id={detailsId} className="ml-6 mt-0.5 space-y-1">Also applies to: 136-195
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/web/src/components/workflows/DagNodeProgress.tsx` around lines 100 -
108, The toggle button in DagNodeProgress should expose its expanded state and
reference the collapsible container for assistive tech: add
aria-expanded={expanded} and aria-controls pointing to the collapsible section
id on the button (e.g., aria-controls={`details-${id}`}), and add a matching id
on the collapsible container element (use a stable unique id via useId or the
node id), then apply the same changes to the second toggle instance around lines
136-195 so both buttons include aria-expanded and aria-controls with matching
container ids.
Comment on lines
+348
to
+364
| if (event.activity === 'started') { | ||
| // Replace any prior started row for the same taskId (e.g. a | ||
| // re-spawn) — keeps the list flat rather than duplicating. | ||
| const newTask: DagTaskInfo = { | ||
| taskId: event.taskId, | ||
| activity: 'started', | ||
| startedAt: now, | ||
| updatedAt: now, | ||
| ...(event.description !== undefined ? { description: event.description } : {}), | ||
| ...(event.taskType !== undefined ? { taskType: event.taskType } : {}), | ||
| }; | ||
| if (taskIdx >= 0) { | ||
| tasks[taskIdx] = newTask; | ||
| } else { | ||
| tasks.push(newTask); | ||
| } | ||
| } else { |
There was a problem hiding this comment.
Late started events currently overwrite newer task/hook state.
Out-of-order streams are partially handled, but if completed/response arrives first and started arrives later, the started branch replaces the existing row and regresses state (completed/response → started) while dropping metadata. Task seeding also drops fields like summary/usage/lastToolName on first non-started event.
Suggested fix (merge instead of replace when row already exists)
- if (event.activity === 'started') {
+ if (event.activity === 'started') {
const newTask: DagTaskInfo = {
taskId: event.taskId,
activity: 'started',
startedAt: now,
updatedAt: now,
...(event.description !== undefined ? { description: event.description } : {}),
...(event.taskType !== undefined ? { taskType: event.taskType } : {}),
};
if (taskIdx >= 0) {
- tasks[taskIdx] = newTask;
+ const prev = tasks[taskIdx];
+ tasks[taskIdx] = {
+ ...prev,
+ // never regress terminal/progress info on late "started"
+ activity: prev.activity === 'started' ? 'started' : prev.activity,
+ startedAt: Math.min(prev.startedAt, now),
+ updatedAt: Math.max(prev.updatedAt, now),
+ ...(prev.description === undefined && event.description !== undefined
+ ? { description: event.description }
+ : {}),
+ ...(prev.taskType === undefined && event.taskType !== undefined
+ ? { taskType: event.taskType }
+ : {}),
+ };
} else {
tasks.push(newTask);
}
} else {
if (taskIdx < 0) {
tasks.push({
taskId: event.taskId,
activity: event.activity,
startedAt: now,
updatedAt: now,
+ ...(event.description !== undefined ? { description: event.description } : {}),
+ ...(event.taskType !== undefined ? { taskType: event.taskType } : {}),
+ ...(event.summary !== undefined ? { summary: event.summary } : {}),
+ ...(event.lastToolName !== undefined ? { lastToolName: event.lastToolName } : {}),
+ ...(event.usage !== undefined ? { usage: event.usage } : {}),
});
} else {- if (event.activity === 'started') {
+ if (event.activity === 'started') {
const newHook: DagHookInfo = {
hookId: event.hookId,
hookName: event.hookName,
hookEvent: event.hookEvent,
activity: 'started',
startedAt: now,
updatedAt: now,
};
if (hookIdx >= 0) {
- hooks[hookIdx] = newHook;
+ const prev = hooks[hookIdx];
+ hooks[hookIdx] = {
+ ...prev,
+ hookName: prev.hookName || event.hookName,
+ hookEvent: prev.hookEvent || event.hookEvent,
+ // keep response outcome if it already arrived
+ activity: prev.activity === 'response' ? 'response' : 'started',
+ startedAt: Math.min(prev.startedAt, now),
+ updatedAt: Math.max(prev.updatedAt, now),
+ };
} else {
hooks.push(newHook);
}
} else {Also applies to: 365-394, 423-437
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/web/src/stores/workflow-store.ts` around lines 348 - 364, The
started-event handler currently replaces an existing task row (using
tasks[taskIdx] = newTask), which causes late 'started' events to regress state
and drop fields like summary/usage/lastToolName; instead, when taskIdx >= 0
merge the new started fields into the existing task object (update
activity/startedAt/updatedAt and only set description/taskType if present)
preserving any existing fields (e.g.,
completed/response/summary/usage/lastToolName); apply the same merge approach to
the other similar blocks referenced (lines handling other activities around the
same file, e.g., the blocks at ~365-394 and ~423-437) so out-of-order events
update fields incrementally rather than overwriting the entire object.
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
1 participant
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.
Closes #975
Summary
Implements all 4 phases of #975: forwards Claude SDK subagent task lifecycle
and hook callback events to the Web UI through the existing SSE pipeline.
Until now the SDK fired
task_started/task_progress/task_notificationand
hook_started/hook_responsesystemevents and they hit theclaude.system_message_unhandleddebug log. After this PR, DAG nodesspawning subagents or invoking hooks show live activity under the parent
node in the run detail view and as at-a-glance counts on the graph card.
Phase 1 — provider (packages/providers)
MessageChunkvariants:task_started,task_progress,task_notification,hook_started,hook_response.streamClaudeMessagesinclaude/provider.tsswitches on the SDKsystem subtype and yields the new chunks. Housekeeping tasks flagged
with
skip_transcriptare suppressed at the provider boundary sothey never reach the UI.
provider.test.ts(chunk shape, ordering,housekeeping suppression,
agentProgressSummariesopt-in).Phase 2 — workflow engine (packages/workflows + server)
WorkflowEmitterEventvariants:task_activity(carriesactivity: started|progress|completed|failed|stopped, plussummary,usage,lastToolName,taskType) andhook_activity(carrieshookEvent,hookName,outcome,exitCode).dag-executor.tsaggregates the 5 provider chunks into the 2emitter events; both are persisted to
workflow_eventsfor timelinereplay.
WORKFLOW_EVENT_TYPESextended withtask_activityandhook_activity.server/src/adapters/web/workflow-bridge.tsmaps them to SSEworkflow_task_activityandworkflow_hook_activity.workflow-bridge.test.tscovers the mapper.Phase 3 — Web UI (packages/web)
DagTaskInfo/DagHookInfoarrays under eachDagNodeState.useSSEanduseDashboardSSEroute the new event types to twonew store handlers (
handleTaskActivity,handleHookActivity).The store mutates the same row across the started → progress →
completed/failed lifecycle so the user sees one entry that updates
in place; the same row preserves accumulated fields
(
descriptionfromstartedsurvives aprogressevent thatomits it).
DagNodeProgressrenders tasks as a collapsible Subagent taskssub-list and hooks as inline indicators (e.g.
PreToolUse(Bash) → approved) under the parent node.ExecutionDagNode+WorkflowDagViewersurface active/totaltask and hook counts on the graph card so activity is visible
without opening the run detail panel.
workflow-store.test.ts.Phase 4 — agentProgressSummaries (packages/providers)
agentProgressSummaries: trueis set for workflow nodes bydefault. Direct chat calls (no
nodeConfig) keep the SDK default(
false) so the chat surface is unchanged. Authors can opt outper-node with
agentProgressSummaries: falsein nodeConfig.NodeConfig.agentProgressSummariesfield documents theoverride.
Test plan
bun run validate(type-check + lint + format:check + bundled checksSummary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests