Repository navigation
fix(chat-mastra): emit agent lifecycle events for pane status indicators - #2450
Merged
Kitenite merged 2 commits intoMar 13, 2026
Merged
Conversation
MastraChat runs the agent in-process (not in a terminal), so the shell-based hook chain that terminal agents use to call /hook/complete never fires. This means agent_start and agent_end harness events were silently swallowed — the desktop UI never received AGENT_LIFECYCLE notifications to show working/review/idle pane indicators. Add an onLifecycleEvent callback to subscribeToSessionEvents that the desktop app wires to notificationsEmitter, emitting Start, Stop, and PermissionRequest events directly — the same path terminal agents use.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughIntroduces a LifecycleEvent type and propagation path: runtime emits lifecycle events, subscribeToSessionEvents accepts an onLifecycleEvent callback, ChatMastraService accepts and forwards that callback, and the desktop router maps events to AGENT_LIFECYCLE notifications. Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as Runtime Agent
participant Runtime as subscribeToSessionEvents
participant Service as ChatMastraService
participant Router as Desktop Router
participant Emitter as notificationsEmitter
Agent->>Runtime: emit lifecycle (PermissionRequest/Start/Stop)
Runtime->>Runtime: build LifecycleEvent {sessionId,eventType}
Runtime->>Service: call onLifecycleEvent(event)
Service->>Router: onLifecycleEvent -> handleLifecycleEvent(event)
Router->>Router: resolvePaneIdFromSession(event.sessionId)
Router->>Emitter: emit AGENT_LIFECYCLE with paneId & eventType
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 5 files
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="apps/desktop/src/lib/trpc/routers/chat-mastra-service/index.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/chat-mastra-service/index.ts:18">
P2: Don't swallow errors while resolving pane IDs; log at least a warning so lifecycle mapping failures are diagnosable.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+18
to
+20
| } catch { | ||
| // App state not initialized yet | ||
| } |
Contributor
There was a problem hiding this comment.
P2: Don't swallow errors while resolving pane IDs; log at least a warning so lifecycle mapping failures are diagnosable.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/chat-mastra-service/index.ts, line 18:
<comment>Don't swallow errors while resolving pane IDs; log at least a warning so lifecycle mapping failures are diagnosable.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.) </comment>
<file context>
@@ -1,14 +1,42 @@
+ return paneId;
+ }
+ }
+ } catch {
+ // App state not initialized yet
+ }
</file context>
Suggested change
| } catch { | |
| // App state not initialized yet | |
| } | |
| } catch (error) { | |
| console.warn("[chat-mastra-service] Failed to resolve paneId from sessionId", { | |
| sessionId, | |
| error, | |
| }); | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/chat-mastra-service/index.ts`:
- Around line 24-30: handleLifecycleEvent currently emits only paneId and
eventType but consumers expect a full AgentLifecycleEvent (workspaceId, tabId,
paneId); update the code so the resolution step returns all three identifiers
(e.g., change or extend resolvePaneIdFromSession to return {paneId, workspaceId,
tabId} or add a new resolvePaneContext(sessionId) helper), then call
notificationsEmitter.emit(NOTIFICATION_EVENTS.AGENT_LIFECYCLE, { paneId,
workspaceId, tabId, eventType: event.eventType }) so
shouldSuppressForVisiblePane and handleAgentLifecycle receive workspaceId and
tabId alongside paneId. Ensure the resolver iterates panes as before and
populates workspaceId and tabId when a match is found and handle the missing
case consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 388994c9-1519-4081-92f0-c995be1b84db
📒 Files selected for processing (5)
apps/desktop/src/lib/trpc/routers/chat-mastra-service/index.tspackages/chat-mastra/src/server/trpc/index.tspackages/chat-mastra/src/server/trpc/service.tspackages/chat-mastra/src/server/trpc/utils/runtime/index.tspackages/chat-mastra/src/server/trpc/utils/runtime/runtime.ts
Comment on lines
+24
to
+30
| function handleLifecycleEvent(event: LifecycleEvent): void { | ||
| const paneId = resolvePaneIdFromSession(event.sessionId); | ||
| notificationsEmitter.emit(NOTIFICATION_EVENTS.AGENT_LIFECYCLE, { | ||
| paneId, | ||
| eventType: event.eventType, | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Missing workspaceId and tabId in emitted lifecycle event.
The handleLifecycleEvent function emits only paneId and eventType, but downstream consumers expect a complete AgentLifecycleEvent. Specifically:
notification-manager.ts:shouldSuppressForVisiblePanerequiresworkspaceId,tabId, andpaneIdto determine if notifications should be suppressed for visible panes (returnsfalseearly if any are missing, causing notifications to always show).handleAgentLifecycleusesworkspaceIdto get the workspace name for notification titles.
Since resolvePaneIdFromSession already iterates the panes, consider extracting all required fields at once.
Proposed fix to include workspaceId and tabId
-function resolvePaneIdFromSession(sessionId: string): string | undefined {
+function resolvePaneInfoFromSession(sessionId: string): {
+ paneId: string;
+ tabId: string;
+ workspaceId: string;
+} | undefined {
try {
const tabsState = appState.data.tabsState;
if (!tabsState) return undefined;
for (const [paneId, pane] of Object.entries(tabsState.panes ?? {})) {
if (pane.chatMastra?.sessionId === sessionId) {
- return paneId;
+ const tab = tabsState.tabs?.[pane.tabId];
+ if (!tab) return undefined;
+ return {
+ paneId,
+ tabId: pane.tabId,
+ workspaceId: tab.workspaceId,
+ };
}
}
} catch {
// App state not initialized yet
}
return undefined;
}
function handleLifecycleEvent(event: LifecycleEvent): void {
- const paneId = resolvePaneIdFromSession(event.sessionId);
+ const paneInfo = resolvePaneInfoFromSession(event.sessionId);
notificationsEmitter.emit(NOTIFICATION_EVENTS.AGENT_LIFECYCLE, {
- paneId,
+ paneId: paneInfo?.paneId,
+ tabId: paneInfo?.tabId,
+ workspaceId: paneInfo?.workspaceId,
eventType: event.eventType,
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function handleLifecycleEvent(event: LifecycleEvent): void { | |
| const paneId = resolvePaneIdFromSession(event.sessionId); | |
| notificationsEmitter.emit(NOTIFICATION_EVENTS.AGENT_LIFECYCLE, { | |
| paneId, | |
| eventType: event.eventType, | |
| }); | |
| } | |
| function resolvePaneInfoFromSession(sessionId: string): { | |
| paneId: string; | |
| tabId: string; | |
| workspaceId: string; | |
| } | undefined { | |
| try { | |
| const tabsState = appState.data.tabsState; | |
| if (!tabsState) return undefined; | |
| for (const [paneId, pane] of Object.entries(tabsState.panes ?? {})) { | |
| if (pane.chatMastra?.sessionId === sessionId) { | |
| const tab = tabsState.tabs?.[pane.tabId]; | |
| if (!tab) return undefined; | |
| return { | |
| paneId, | |
| tabId: pane.tabId, | |
| workspaceId: tab.workspaceId, | |
| }; | |
| } | |
| } | |
| } catch { | |
| // App state not initialized yet | |
| } | |
| return undefined; | |
| } | |
| function handleLifecycleEvent(event: LifecycleEvent): void { | |
| const paneInfo = resolvePaneInfoFromSession(event.sessionId); | |
| notificationsEmitter.emit(NOTIFICATION_EVENTS.AGENT_LIFECYCLE, { | |
| paneId: paneInfo?.paneId, | |
| tabId: paneInfo?.tabId, | |
| workspaceId: paneInfo?.workspaceId, | |
| eventType: event.eventType, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/chat-mastra-service/index.ts` around lines
24 - 30, handleLifecycleEvent currently emits only paneId and eventType but
consumers expect a full AgentLifecycleEvent (workspaceId, tabId, paneId); update
the code so the resolution step returns all three identifiers (e.g., change or
extend resolvePaneIdFromSession to return {paneId, workspaceId, tabId} or add a
new resolvePaneContext(sessionId) helper), then call
notificationsEmitter.emit(NOTIFICATION_EVENTS.AGENT_LIFECYCLE, { paneId,
workspaceId, tabId, eventType: event.eventType }) so
shouldSuppressForVisiblePane and handleAgentLifecycle receive workspaceId and
tabId alongside paneId. Ensure the resolver iterates panes as before and
populates workspaceId and tabId when a match is found and handle the missing
case consistently.
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
Kitenite
deleted the
kitenite/mastrachat-is-not-calling-hooks-like-running-done
branch
March 13, 2026 05:05
This branch was previously deployed
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
/hook/completenever firesagent_startandagent_endharness events were silently swallowed — the UI never receivedAGENT_LIFECYCLEnotifications, so pane status indicators (working/review/idle) never updatedonLifecycleEventcallback tosubscribeToSessionEventsthat the desktop app wires directly tonotificationsEmitter, emittingStart,Stop, andPermissionRequestevents through the same path terminal agents useTest plan
Summary by cubic
Emit agent lifecycle events from MastraChat so pane status indicators update correctly. In-process sessions now send Start, Stop, and PermissionRequest signals through the same notification path as terminal agents.
onLifecycleEventtosubscribeToSessionEventsand plumbed it throughChatMastraService.sessionIdtopaneIdand emitsNOTIFICATION_EVENTS.AGENT_LIFECYCLEvianotificationsEmitter.LifecycleEventfrom@superset/chat-mastra/server/trpcfor desktop wiring.Written for commit c009661. Summary will update on new commits.
Summary by CodeRabbit