From b1955cf74d144ad6f99d394d57f82ef04ec236cf Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Wed, 18 Feb 2026 12:28:27 +0700 Subject: [PATCH] fix: unify agent attention notification payloads (#55) * fix: simplify tmux session creation in paseo.json * fix: unify agent attention notification payloads --- packages/app/src/contexts/session-context.tsx | 143 +++--------- .../websocket-server.notifications.test.ts | 123 +++++++++++ .../server/src/server/websocket-server.ts | 37 ++-- .../agent-attention-notification.test.ts | 104 +++++++++ .../shared/agent-attention-notification.ts | 206 ++++++++++++++++++ packages/server/src/shared/messages.ts | 11 + paseo.json | 2 +- 7 files changed, 500 insertions(+), 126 deletions(-) create mode 100644 packages/server/src/server/websocket-server.notifications.test.ts create mode 100644 packages/server/src/shared/agent-attention-notification.test.ts create mode 100644 packages/server/src/shared/agent-attention-notification.ts diff --git a/packages/app/src/contexts/session-context.tsx b/packages/app/src/contexts/session-context.tsx index 7e82f3983..e203c0457 100644 --- a/packages/app/src/contexts/session-context.tsx +++ b/packages/app/src/contexts/session-context.tsx @@ -19,8 +19,12 @@ import type { SessionOutboundMessage, } from "@server/shared/messages"; import { parseServerInfoStatusPayload } from "@server/shared/messages"; +import { + buildAgentAttentionNotificationPayload, + type AgentAttentionNotificationPayload, + type NotificationPermissionRequest, +} from "@server/shared/agent-attention-notification"; import type { AgentLifecycleStatus } from "@server/shared/agent-lifecycle"; -import type { AgentPermissionRequest } from "@server/server/agent/agent-sdk-types"; import type { DaemonClient, ConnectionState } from "@server/client/daemon-client"; import { File } from "expo-file-system"; import { useDaemonConnections } from "./daemon-connections-context"; @@ -52,7 +56,7 @@ export type { const derivePendingPermissionKey = ( agentId: string, - request: AgentPermissionRequest + request: NotificationPermissionRequest ) => { const fallbackId = request.id || @@ -68,41 +72,8 @@ const derivePendingPermissionKey = ( return `${agentId}:${fallbackId}`; }; -const NOTIFICATION_PREVIEW_LIMIT = 220; const HISTORY_STALE_AFTER_MS = 60_000; -const normalizeNotificationText = (text: string): string => - text.replace(/\s+/g, " ").trim(); - -const truncateNotificationText = (text: string, limit: number): string => { - if (text.length <= limit) { - return text; - } - const trimmed = text.slice(0, Math.max(0, limit - 3)).trimEnd(); - return trimmed.length > 0 ? `${trimmed}...` : text.slice(0, limit); -}; - -const buildNotificationPreview = ( - text: string | null | undefined -): string | null => { - if (!text) { - return null; - } - const normalized = normalizeNotificationText(text); - if (!normalized) { - return null; - } - return truncateNotificationText(normalized, NOTIFICATION_PREVIEW_LIMIT); -}; - -const safeStringify = (value: unknown): string | null => { - try { - return JSON.stringify(value); - } catch { - return null; - } -}; - const findLatestAssistantMessageText = (items: StreamItem[]): string | null => { for (let i = items.length - 1; i >= 0; i -= 1) { const item = items[i]; @@ -125,12 +96,12 @@ const mapConnectionState = ( const getLatestPermissionRequest = ( session: SessionState | undefined, agentId: string -): AgentPermissionRequest | null => { +): NotificationPermissionRequest | null => { if (!session) { return null; } - let latest: AgentPermissionRequest | null = null; + let latest: NotificationPermissionRequest | null = null; for (const pending of session.pendingPermissions.values()) { if (pending.agentId === agentId) { latest = pending.request; @@ -142,46 +113,12 @@ const getLatestPermissionRequest = ( const agentPending = session.agents.get(agentId)?.pendingPermissions; if (agentPending && agentPending.length > 0) { - return agentPending[agentPending.length - 1] as AgentPermissionRequest; + return agentPending[agentPending.length - 1] as NotificationPermissionRequest; } return null; }; -const buildPermissionDetails = ( - request: AgentPermissionRequest | null -): string | null => { - if (!request) { - return null; - } - const title = request.title?.trim(); - const description = request.description?.trim(); - const details: string[] = []; - if (title) { - details.push(title); - } - if (description && description !== title) { - details.push(description); - } - if (details.length > 0) { - return details.join(" - "); - } - - const inputPreview = request.input ? safeStringify(request.input) : null; - if (inputPreview) { - return inputPreview; - } - - const metadataPreview = request.metadata - ? safeStringify(request.metadata) - : null; - if (metadataPreview) { - return metadataPreview; - } - - return request.name?.trim() || request.kind; -}; - type FileExplorerPayload = Extract< SessionOutboundMessage, { type: "file_explorer_response" } @@ -403,6 +340,7 @@ export function SessionProvider({ agentId: string; reason: "finished" | "error" | "permission"; timestamp: string; + notification?: AgentAttentionNotificationPayload; }) => { const appState = appStateRef.current; const session = useSessionStore.getState().sessions[serverId]; @@ -423,43 +361,33 @@ export function SessionProvider({ } attentionNotifiedRef.current.set(params.agentId, timestampMs); - const title = - params.reason === "permission" - ? "Agent needs permission" - : "Agent finished"; - let preview: string | null = null; + const head = session?.agentStreamHead.get(params.agentId) ?? []; + const tail = session?.agentStreamTail.get(params.agentId) ?? []; + const assistantMessage = + findLatestAssistantMessageText(head) ?? + findLatestAssistantMessageText(tail); + const permissionRequest = getLatestPermissionRequest( + session, + params.agentId + ); - if (params.reason === "finished") { - const head = session?.agentStreamHead.get(params.agentId) ?? []; - const tail = session?.agentStreamTail.get(params.agentId) ?? []; - const lastMessage = - findLatestAssistantMessageText(head) ?? - findLatestAssistantMessageText(tail); - preview = buildNotificationPreview(lastMessage); - } else if (params.reason === "permission") { - const permissionRequest = getLatestPermissionRequest( - session, - params.agentId - ); - preview = buildNotificationPreview( - buildPermissionDetails(permissionRequest) - ); - } - - const body = - preview ?? - (params.reason === "permission" - ? "Permission requested." - : "Finished working."); + const notification = + params.notification ?? + buildAgentAttentionNotificationPayload({ + reason: params.reason, + serverId, + agentId: params.agentId, + assistantMessage: params.reason === "finished" ? assistantMessage : null, + permissionRequest: + params.reason === "permission" + ? permissionRequest + : null, + }); void sendOsNotification({ - title, - body, - data: { - agentId: params.agentId, - serverId, - reason: params.reason, - }, + title: notification.title, + body: notification.body, + data: notification.data, }); }, [serverId] @@ -879,7 +807,7 @@ export function SessionProvider({ const nextAgents = new Map(); const nextPendingPermissions = new Map< string, - { key: string; agentId: string; request: AgentPermissionRequest } + { key: string; agentId: string; request: NotificationPermissionRequest } >(); const nextStatuses = new Map(); @@ -971,6 +899,7 @@ export function SessionProvider({ agentId, reason: event.reason, timestamp: event.timestamp, + notification: event.notification, }); } } diff --git a/packages/server/src/server/websocket-server.notifications.test.ts b/packages/server/src/server/websocket-server.notifications.test.ts new file mode 100644 index 000000000..4a8ffb00b --- /dev/null +++ b/packages/server/src/server/websocket-server.notifications.test.ts @@ -0,0 +1,123 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +const wsModuleMock = vi.hoisted(() => { + class MockWebSocketServer { + readonly handlers = new Map void>(); + + constructor(_options: unknown) {} + + on(event: string, handler: (...args: any[]) => void) { + this.handlers.set(event, handler); + return this; + } + + close() { + // no-op + } + } + + return { MockWebSocketServer }; +}); + +const pushMocks = vi.hoisted(() => ({ + getAllTokens: vi.fn(() => ["ExponentPushToken[token-1]"]), + sendPush: vi.fn(async () => {}), +})); + +vi.mock("ws", () => ({ + WebSocketServer: wsModuleMock.MockWebSocketServer, +})); + +vi.mock("./session.js", () => ({ + Session: class {}, +})); + +vi.mock("./push/token-store.js", () => ({ + PushTokenStore: class { + getAllTokens = pushMocks.getAllTokens; + removeToken = vi.fn(); + }, +})); + +vi.mock("./push/push-service.js", () => ({ + PushService: class { + sendPush = pushMocks.sendPush; + }, +})); + +import { VoiceAssistantWebSocketServer } from "./websocket-server.js"; + +function createLogger() { + const logger = { + child: vi.fn(() => logger), + trace: vi.fn(), + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }; + return logger; +} + +function createServer(agentManagerOverrides?: Record) { + const agentManager = { + setAgentAttentionCallback: vi.fn(), + getAgent: vi.fn(() => null), + ...agentManagerOverrides, + }; + + const server = new VoiceAssistantWebSocketServer( + {} as any, + createLogger() as any, + "srv-test", + agentManager as any, + {} as any, + {} as any, + "/tmp/paseo-test", + async () => ({} as any), + { allowedOrigins: new Set() } + ); + + return { server, agentManager }; +} + +describe("VoiceAssistantWebSocketServer notification payloads", () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + it("uses assistant preview text for push notifications with markdown removed", () => { + const { server } = createServer({ + getAgent: vi.fn(() => ({ + config: { title: null }, + cwd: "/tmp/worktree", + timeline: [ + { + type: "assistant_message", + text: "**Done**. Updated `README.md` and [link](https://example.com).", + }, + ], + pendingPermissions: new Map(), + })), + }); + + (server as any).broadcastAgentAttention({ + agentId: "agent-1", + provider: "claude", + reason: "finished", + }); + + expect(pushMocks.sendPush).toHaveBeenCalledWith( + ["ExponentPushToken[token-1]"], + { + title: "Agent finished", + body: "Done. Updated README.md and link.", + data: { + serverId: "srv-test", + agentId: "agent-1", + reason: "finished", + }, + } + ); + }); +}); diff --git a/packages/server/src/server/websocket-server.ts b/packages/server/src/server/websocket-server.ts index ada05fb6a..15b056642 100644 --- a/packages/server/src/server/websocket-server.ts +++ b/packages/server/src/server/websocket-server.ts @@ -42,6 +42,11 @@ import { computeShouldSendPush, type ClientAttentionState, } from "./agent-attention-policy.js"; +import { + buildAgentAttentionNotificationPayload, + findLatestAssistantMessageFromTimeline, + findLatestPermissionRequest, +} from "../shared/agent-attention-notification.js"; export type AgentMcpTransportFactory = () => Promise; export type ExternalSocketMetadata = { @@ -819,6 +824,18 @@ export class VoiceAssistantWebSocketServer { } const allStates = clientEntries.map((e) => e.state); + const agent = this.agentManager.getAgent(params.agentId); + const notification = buildAgentAttentionNotificationPayload({ + reason: params.reason, + serverId: this.serverId, + agentId: params.agentId, + assistantMessage: agent + ? findLatestAssistantMessageFromTimeline(agent.timeline) + : null, + permissionRequest: agent + ? findLatestPermissionRequest(agent.pendingPermissions) + : null, + }); // Push is only a fallback when the user is away from desktop/web. // Also suppress push if they're actively using the mobile app. @@ -831,24 +848,7 @@ export class VoiceAssistantWebSocketServer { const tokens = this.pushTokenStore.getAllTokens(); this.logger.info({ tokenCount: tokens.length }, "Sending push notification"); if (tokens.length > 0) { - const agent = this.agentManager.getAgent(params.agentId); - const agentTitle = agent?.config?.title ?? agent?.cwd ?? params.agentId; - const title = - params.reason === "permission" ? "Agent needs permission" : "Agent finished"; - const body = - params.reason === "permission" - ? `Permission requested: ${agentTitle}` - : `Finished: ${agentTitle}`; - - void this.pushService.sendPush(tokens, { - title, - body, - data: { - serverId: this.serverId, - agentId: params.agentId, - reason: params.reason, - }, - }); + void this.pushService.sendPush(tokens, notification); } } @@ -869,6 +869,7 @@ export class VoiceAssistantWebSocketServer { reason: params.reason, timestamp: new Date().toISOString(), shouldNotify, + notification, }, timestamp: new Date().toISOString(), }, diff --git a/packages/server/src/shared/agent-attention-notification.test.ts b/packages/server/src/shared/agent-attention-notification.test.ts new file mode 100644 index 000000000..04b0c3d5e --- /dev/null +++ b/packages/server/src/shared/agent-attention-notification.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, it } from "vitest"; +import { + buildAgentAttentionNotificationPayload, + findLatestAssistantMessageFromTimeline, + findLatestPermissionRequest, +} from "./agent-attention-notification.js"; + +describe("buildAgentAttentionNotificationPayload", () => { + it("builds finished notifications from markdown assistant text", () => { + const payload = buildAgentAttentionNotificationPayload({ + reason: "finished", + serverId: "srv-1", + agentId: "agent-1", + assistantMessage: + "**Done**. Updated `README.md` and [link](https://example.com).", + }); + + expect(payload).toEqual({ + title: "Agent finished", + body: "Done. Updated README.md and link.", + data: { + serverId: "srv-1", + agentId: "agent-1", + reason: "finished", + }, + }); + }); + + it("builds permission notifications from request details", () => { + const payload = buildAgentAttentionNotificationPayload({ + reason: "permission", + serverId: "srv-2", + agentId: "agent-2", + permissionRequest: { + id: "perm-1", + provider: "claude", + name: "exec", + kind: "tool", + title: "**Approve command**", + description: "Run `git push`", + }, + }); + + expect(payload).toEqual({ + title: "Agent needs permission", + body: "Approve command - Run git push", + data: { + serverId: "srv-2", + agentId: "agent-2", + reason: "permission", + }, + }); + }); + + it("uses error-specific defaults when reason is error", () => { + const payload = buildAgentAttentionNotificationPayload({ + reason: "error", + serverId: "srv-3", + agentId: "agent-3", + }); + + expect(payload).toEqual({ + title: "Agent needs attention", + body: "Encountered an error.", + data: { + serverId: "srv-3", + agentId: "agent-3", + reason: "error", + }, + }); + }); +}); + +describe("findLatestAssistantMessageFromTimeline", () => { + it("joins the latest contiguous assistant chunks", () => { + expect( + findLatestAssistantMessageFromTimeline([ + { type: "user_message", text: "start" }, + { type: "assistant_message", text: "Part " }, + { type: "assistant_message", text: "one" }, + { type: "reasoning", text: "thinking..." }, + { type: "assistant_message", text: "Done " }, + { type: "assistant_message", text: "now" }, + ]) + ).toBe("Done now"); + }); +}); + +describe("findLatestPermissionRequest", () => { + it("returns the most recently inserted request", () => { + const pending = new Map([ + [ + "first", + { id: "first", provider: "claude", name: "a", kind: "tool" } as const, + ], + [ + "second", + { id: "second", provider: "claude", name: "b", kind: "tool" } as const, + ], + ]); + + expect(findLatestPermissionRequest(pending)?.id).toBe("second"); + }); +}); diff --git a/packages/server/src/shared/agent-attention-notification.ts b/packages/server/src/shared/agent-attention-notification.ts new file mode 100644 index 000000000..452a3bb2e --- /dev/null +++ b/packages/server/src/shared/agent-attention-notification.ts @@ -0,0 +1,206 @@ +const NOTIFICATION_PREVIEW_LIMIT = 220; + +export type AgentAttentionReason = "finished" | "error" | "permission"; + +export type AgentAttentionNotificationData = { + serverId: string; + agentId: string; + reason: AgentAttentionReason; +}; + +export type AgentAttentionNotificationPayload = { + title: string; + body: string; + data: AgentAttentionNotificationData; +}; + +type BuildAgentAttentionNotificationPayloadInput = { + reason: AgentAttentionReason; + serverId: string; + agentId: string; + assistantMessage?: string | null; + permissionRequest?: NotificationPermissionRequest | null; +}; + +export type NotificationPermissionRequest = { + id: string; + provider: string; + name: string; + kind: "tool" | "plan" | "question" | "mode" | "other"; + title?: string; + description?: string; + input?: Record; + metadata?: Record; +}; + +export type AssistantTimelineItem = + | { type: "assistant_message"; text: string } + | { type: string; text?: string }; + +const normalizeNotificationText = (text: string): string => + text.replace(/\s+/g, " ").trim(); + +const truncateNotificationText = (text: string, limit: number): string => { + if (text.length <= limit) { + return text; + } + const trimmed = text.slice(0, Math.max(0, limit - 3)).trimEnd(); + return trimmed.length > 0 ? `${trimmed}...` : text.slice(0, limit); +}; + +const stripMarkdownToText = (markdown: string): string => { + let text = markdown.replace(/\r\n/g, "\n"); + + // Strip fenced code markers but keep the code content itself. + text = text.replace(/^\s*```[^\n]*$/gm, ""); + text = text.replace(/^\s*~~~[^\n]*$/gm, ""); + + // Markdown links/images. + text = text.replace(/!\[([^\]]*)\]\((?:[^()\\]|\\.)*\)/g, "$1"); + text = text.replace(/\[([^\]]+)\]\((?:[^()\\]|\\.)*\)/g, "$1"); + + // Structural prefixes. + text = text.replace(/^\s{0,3}#{1,6}\s+/gm, ""); + text = text.replace(/^\s{0,3}>+\s?/gm, ""); + text = text.replace(/^\s{0,3}(?:[*+-]|\d+\.)\s+/gm, ""); + text = text.replace(/^\s{0,3}(?:[-*_]\s*){3,}$/gm, ""); + + // Inline markdown markers. + text = text.replace(/`([^`]+)`/g, "$1"); + text = text.replace(/\*\*([^*]+)\*\*/g, "$1"); + text = text.replace(/__([^_]+)__/g, "$1"); + text = text.replace(/\*([^*\n]+)\*/g, "$1"); + text = text.replace(/_([^_\n]+)_/g, "$1"); + text = text.replace(/~~([^~]+)~~/g, "$1"); + + // Angle-bracketed URL autolinks. + text = text.replace(/<([^>\n]+)>/g, "$1"); + + return text; +}; + +const buildNotificationPreview = ( + text: string | null | undefined +): string | null => { + if (!text) { + return null; + } + + const normalized = normalizeNotificationText(stripMarkdownToText(text)); + if (!normalized) { + return null; + } + + return truncateNotificationText(normalized, NOTIFICATION_PREVIEW_LIMIT); +}; + +const safeStringify = (value: unknown): string | null => { + try { + return JSON.stringify(value); + } catch { + return null; + } +}; + +const buildPermissionDetails = ( + request: NotificationPermissionRequest | null | undefined +): string | null => { + if (!request) { + return null; + } + + const title = request.title?.trim(); + const description = request.description?.trim(); + const details: string[] = []; + + if (title) { + details.push(title); + } + if (description && description !== title) { + details.push(description); + } + if (details.length > 0) { + return details.join(" - "); + } + + const inputPreview = request.input ? safeStringify(request.input) : null; + if (inputPreview) { + return inputPreview; + } + + const metadataPreview = request.metadata ? safeStringify(request.metadata) : null; + if (metadataPreview) { + return metadataPreview; + } + + return request.name?.trim() || request.kind; +}; + +export function findLatestAssistantMessageFromTimeline( + timeline: readonly AssistantTimelineItem[] +): string | null { + // Providers may stream assistant content in consecutive chunks. + const chunks: string[] = []; + for (let i = timeline.length - 1; i >= 0; i -= 1) { + const item = timeline[i]; + if (item.type !== "assistant_message" || typeof item.text !== "string") { + if (chunks.length > 0) { + break; + } + continue; + } + chunks.push(item.text); + } + + if (chunks.length === 0) { + return null; + } + + return chunks.reverse().join(""); +} + +export function findLatestPermissionRequest( + pendingPermissions: ReadonlyMap +): NotificationPermissionRequest | null { + let latest: NotificationPermissionRequest | null = null; + for (const request of pendingPermissions.values()) { + latest = request; + } + return latest; +} + +export function buildAgentAttentionNotificationPayload( + input: BuildAgentAttentionNotificationPayloadInput +): AgentAttentionNotificationPayload { + const title = + input.reason === "permission" + ? "Agent needs permission" + : input.reason === "error" + ? "Agent needs attention" + : "Agent finished"; + + const preview = + input.reason === "finished" + ? buildNotificationPreview(input.assistantMessage) + : input.reason === "permission" + ? buildNotificationPreview(buildPermissionDetails(input.permissionRequest)) + : null; + + const body = + preview ?? + (input.reason === "permission" + ? "Permission requested." + : input.reason === "error" + ? "Encountered an error." + : "Finished working."); + + return { + title, + body, + data: { + serverId: input.serverId, + agentId: input.agentId, + reason: input.reason, + }, + }; +} diff --git a/packages/server/src/shared/messages.ts b/packages/server/src/shared/messages.ts index f4bd08bfb..48395cc3e 100644 --- a/packages/server/src/shared/messages.ts +++ b/packages/server/src/shared/messages.ts @@ -332,6 +332,17 @@ export const AgentStreamEventPayloadSchema = z.discriminatedUnion("type", [ reason: z.enum(["finished", "error", "permission"]), timestamp: z.string(), shouldNotify: z.boolean(), + notification: z + .object({ + title: z.string(), + body: z.string(), + data: z.object({ + serverId: z.string(), + agentId: z.string(), + reason: z.enum(["finished", "error", "permission"]), + }), + }) + .optional(), }), ]); diff --git a/paseo.json b/paseo.json index 2e68019e0..865022162 100644 --- a/paseo.json +++ b/paseo.json @@ -8,7 +8,7 @@ "terminals": [ { "name": "dev", - "command": "SESSION_NAME=\"${PWD##*/}\"; if ! tmux has-session -t \"$SESSION_NAME\" 2>/dev/null; then tmux new-session -d -s \"$SESSION_NAME\" -c \"$PWD\"; tmux send-keys -t \"$SESSION_NAME\":0.0 \"npm run dev\" Enter; fi" + "command": "SESSION_NAME=\"${PWD##*/}\"; if ! tmux has-session -t \"$SESSION_NAME\" 2>/dev/null; then tmux new-session -d -s \"$SESSION_NAME\" -c \"$PWD\" \"npm run dev\"; fi" } ] }