refactor(server): extract agent attention notification policy
What changed: - Extracted client/push attention decision rules from websocket-server into a dedicated agent-attention-policy module. - Replaced websocket-server inline branch logic with calls to computeShouldNotifyClient and computeShouldSendPush. - Added deterministic unit tests that cover each notification branch and push suppression rule. Why: - The websocket server carried dense policy branching mixed with transport/session concerns. - Pulling policy into a focused module lowers logic density and makes future changes safer without altering behavior. - Direct unit tests provide confidence for future edits without requiring websocket wiring. Notes for next agent: - This commit intentionally preserves existing behavior and only restructures policy ownership. - If attention behavior needs product changes, update tests in agent-attention-policy.test.ts first. - websocket-server.relay-reconnect.test.ts still covers transport/session reconnection behavior separately.
This commit is contained in:
@@ -0,0 +1,239 @@
|
|||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import {
|
||||||
|
computeShouldNotifyClient,
|
||||||
|
computeShouldSendPush,
|
||||||
|
type ClientAttentionState,
|
||||||
|
} from "./agent-attention-policy.js";
|
||||||
|
|
||||||
|
function state(overrides: Partial<ClientAttentionState>): ClientAttentionState {
|
||||||
|
return {
|
||||||
|
deviceType: null,
|
||||||
|
focusedAgentId: null,
|
||||||
|
isStale: false,
|
||||||
|
appVisible: false,
|
||||||
|
...overrides,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("computeShouldNotifyClient", () => {
|
||||||
|
it("suppresses notifications when someone is actively focused on the agent", () => {
|
||||||
|
const activeOnAgent = state({
|
||||||
|
deviceType: "web",
|
||||||
|
focusedAgentId: "agent-1",
|
||||||
|
isStale: false,
|
||||||
|
appVisible: true,
|
||||||
|
});
|
||||||
|
const staleMobile = state({
|
||||||
|
deviceType: "mobile",
|
||||||
|
focusedAgentId: null,
|
||||||
|
isStale: true,
|
||||||
|
appVisible: false,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(
|
||||||
|
computeShouldNotifyClient({
|
||||||
|
clientState: staleMobile,
|
||||||
|
allClientStates: [activeOnAgent, staleMobile],
|
||||||
|
agentId: "agent-1",
|
||||||
|
})
|
||||||
|
).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("notifies unidentified clients by default when agent is not actively focused", () => {
|
||||||
|
const unknownClient = state({
|
||||||
|
deviceType: null,
|
||||||
|
focusedAgentId: null,
|
||||||
|
isStale: false,
|
||||||
|
appVisible: false,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(
|
||||||
|
computeShouldNotifyClient({
|
||||||
|
clientState: unknownClient,
|
||||||
|
allClientStates: [unknownClient],
|
||||||
|
agentId: "agent-2",
|
||||||
|
})
|
||||||
|
).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("notifies active visible clients when they are focused on an agent", () => {
|
||||||
|
const focusedWeb = state({
|
||||||
|
deviceType: "web",
|
||||||
|
focusedAgentId: "agent-2",
|
||||||
|
isStale: false,
|
||||||
|
appVisible: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(
|
||||||
|
computeShouldNotifyClient({
|
||||||
|
clientState: focusedWeb,
|
||||||
|
allClientStates: [focusedWeb],
|
||||||
|
agentId: "agent-3",
|
||||||
|
})
|
||||||
|
).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("suppresses active clients that are not focused on an agent", () => {
|
||||||
|
const activeButNotFocused = state({
|
||||||
|
deviceType: "web",
|
||||||
|
focusedAgentId: null,
|
||||||
|
isStale: false,
|
||||||
|
appVisible: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(
|
||||||
|
computeShouldNotifyClient({
|
||||||
|
clientState: activeButNotFocused,
|
||||||
|
allClientStates: [activeButNotFocused],
|
||||||
|
agentId: "agent-4",
|
||||||
|
})
|
||||||
|
).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("suppresses stale mobile notifications when an active web client exists", () => {
|
||||||
|
const staleMobile = state({
|
||||||
|
deviceType: "mobile",
|
||||||
|
focusedAgentId: null,
|
||||||
|
isStale: true,
|
||||||
|
appVisible: false,
|
||||||
|
});
|
||||||
|
const activeWeb = state({
|
||||||
|
deviceType: "web",
|
||||||
|
focusedAgentId: null,
|
||||||
|
isStale: false,
|
||||||
|
appVisible: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(
|
||||||
|
computeShouldNotifyClient({
|
||||||
|
clientState: staleMobile,
|
||||||
|
allClientStates: [staleMobile, activeWeb],
|
||||||
|
agentId: "agent-5",
|
||||||
|
})
|
||||||
|
).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("allows stale mobile notifications when no active web client exists", () => {
|
||||||
|
const staleMobile = state({
|
||||||
|
deviceType: "mobile",
|
||||||
|
focusedAgentId: null,
|
||||||
|
isStale: true,
|
||||||
|
appVisible: false,
|
||||||
|
});
|
||||||
|
const staleWeb = state({
|
||||||
|
deviceType: "web",
|
||||||
|
focusedAgentId: null,
|
||||||
|
isStale: true,
|
||||||
|
appVisible: false,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(
|
||||||
|
computeShouldNotifyClient({
|
||||||
|
clientState: staleMobile,
|
||||||
|
allClientStates: [staleMobile, staleWeb],
|
||||||
|
agentId: "agent-6",
|
||||||
|
})
|
||||||
|
).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("suppresses stale web notifications when a mobile client is also present", () => {
|
||||||
|
const staleWeb = state({
|
||||||
|
deviceType: "web",
|
||||||
|
focusedAgentId: null,
|
||||||
|
isStale: true,
|
||||||
|
appVisible: false,
|
||||||
|
});
|
||||||
|
const staleMobile = state({
|
||||||
|
deviceType: "mobile",
|
||||||
|
focusedAgentId: null,
|
||||||
|
isStale: true,
|
||||||
|
appVisible: false,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(
|
||||||
|
computeShouldNotifyClient({
|
||||||
|
clientState: staleWeb,
|
||||||
|
allClientStates: [staleWeb, staleMobile],
|
||||||
|
agentId: "agent-7",
|
||||||
|
})
|
||||||
|
).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("allows stale web notifications when there are no mobile or unidentified clients", () => {
|
||||||
|
const staleWeb = state({
|
||||||
|
deviceType: "web",
|
||||||
|
focusedAgentId: null,
|
||||||
|
isStale: true,
|
||||||
|
appVisible: false,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(
|
||||||
|
computeShouldNotifyClient({
|
||||||
|
clientState: staleWeb,
|
||||||
|
allClientStates: [staleWeb],
|
||||||
|
agentId: "agent-8",
|
||||||
|
})
|
||||||
|
).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("computeShouldSendPush", () => {
|
||||||
|
it("never sends push for error attention events", () => {
|
||||||
|
expect(
|
||||||
|
computeShouldSendPush({
|
||||||
|
reason: "error",
|
||||||
|
allClientStates: [],
|
||||||
|
})
|
||||||
|
).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("suppresses push when any active web client exists", () => {
|
||||||
|
expect(
|
||||||
|
computeShouldSendPush({
|
||||||
|
reason: "finished",
|
||||||
|
allClientStates: [
|
||||||
|
state({
|
||||||
|
deviceType: "web",
|
||||||
|
isStale: false,
|
||||||
|
appVisible: true,
|
||||||
|
}),
|
||||||
|
],
|
||||||
|
})
|
||||||
|
).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("suppresses push when a mobile app is actively visible", () => {
|
||||||
|
expect(
|
||||||
|
computeShouldSendPush({
|
||||||
|
reason: "permission",
|
||||||
|
allClientStates: [
|
||||||
|
state({
|
||||||
|
deviceType: "mobile",
|
||||||
|
isStale: false,
|
||||||
|
appVisible: true,
|
||||||
|
}),
|
||||||
|
],
|
||||||
|
})
|
||||||
|
).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("sends push when no active web client or foreground mobile client exists", () => {
|
||||||
|
expect(
|
||||||
|
computeShouldSendPush({
|
||||||
|
reason: "finished",
|
||||||
|
allClientStates: [
|
||||||
|
state({
|
||||||
|
deviceType: "mobile",
|
||||||
|
isStale: true,
|
||||||
|
appVisible: false,
|
||||||
|
}),
|
||||||
|
state({
|
||||||
|
deviceType: "web",
|
||||||
|
isStale: true,
|
||||||
|
appVisible: false,
|
||||||
|
}),
|
||||||
|
],
|
||||||
|
})
|
||||||
|
).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,91 @@
|
|||||||
|
export type ClientAttentionState = {
|
||||||
|
deviceType: "web" | "mobile" | null;
|
||||||
|
focusedAgentId: string | null;
|
||||||
|
isStale: boolean;
|
||||||
|
appVisible: boolean;
|
||||||
|
};
|
||||||
|
|
||||||
|
export type AgentAttentionReason = "finished" | "error" | "permission";
|
||||||
|
|
||||||
|
type ComputeClientNotificationInput = {
|
||||||
|
clientState: ClientAttentionState;
|
||||||
|
allClientStates: ClientAttentionState[];
|
||||||
|
agentId: string;
|
||||||
|
};
|
||||||
|
|
||||||
|
type ComputePushNotificationInput = {
|
||||||
|
reason: AgentAttentionReason;
|
||||||
|
allClientStates: ClientAttentionState[];
|
||||||
|
};
|
||||||
|
|
||||||
|
function hasActiveClientOnAgent(
|
||||||
|
allClientStates: ClientAttentionState[],
|
||||||
|
agentId: string
|
||||||
|
): boolean {
|
||||||
|
return allClientStates.some(
|
||||||
|
(state) => state.focusedAgentId === agentId && state.appVisible && !state.isStale
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
function hasActiveWebClient(allClientStates: ClientAttentionState[]): boolean {
|
||||||
|
return allClientStates.some((state) => state.deviceType === "web" && !state.isStale);
|
||||||
|
}
|
||||||
|
|
||||||
|
function hasOtherCompetingClient(
|
||||||
|
clientState: ClientAttentionState,
|
||||||
|
allClientStates: ClientAttentionState[]
|
||||||
|
): boolean {
|
||||||
|
return allClientStates.some(
|
||||||
|
(state) =>
|
||||||
|
state !== clientState && (state.deviceType === "mobile" || state.deviceType === null)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
function hasActiveForegroundMobileClient(allClientStates: ClientAttentionState[]): boolean {
|
||||||
|
return allClientStates.some(
|
||||||
|
(state) => state.deviceType === "mobile" && state.appVisible && !state.isStale
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
export function computeShouldNotifyClient({
|
||||||
|
clientState,
|
||||||
|
allClientStates,
|
||||||
|
agentId,
|
||||||
|
}: ComputeClientNotificationInput): boolean {
|
||||||
|
if (hasActiveClientOnAgent(allClientStates, agentId)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (clientState.deviceType === null) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!clientState.isStale && clientState.appVisible && clientState.focusedAgentId !== null) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!clientState.isStale) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (clientState.deviceType === "mobile") {
|
||||||
|
return !hasActiveWebClient(allClientStates);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (clientState.deviceType === "web") {
|
||||||
|
return !hasOtherCompetingClient(clientState, allClientStates);
|
||||||
|
}
|
||||||
|
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function computeShouldSendPush({
|
||||||
|
reason,
|
||||||
|
allClientStates,
|
||||||
|
}: ComputePushNotificationInput): boolean {
|
||||||
|
if (reason === "error") {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
return !hasActiveWebClient(allClientStates) && !hasActiveForegroundMobileClient(allClientStates);
|
||||||
|
}
|
||||||
@@ -34,6 +34,11 @@ import type {
|
|||||||
VoiceMcpStdioConfig,
|
VoiceMcpStdioConfig,
|
||||||
VoiceSpeakHandler,
|
VoiceSpeakHandler,
|
||||||
} from "./voice-types.js";
|
} from "./voice-types.js";
|
||||||
|
import {
|
||||||
|
computeShouldNotifyClient,
|
||||||
|
computeShouldSendPush,
|
||||||
|
type ClientAttentionState,
|
||||||
|
} from "./agent-attention-policy.js";
|
||||||
|
|
||||||
export type AgentMcpTransportFactory = () => Promise<Transport>;
|
export type AgentMcpTransportFactory = () => Promise<Transport>;
|
||||||
export type ExternalSocketMetadata = {
|
export type ExternalSocketMetadata = {
|
||||||
@@ -652,12 +657,7 @@ export class VoiceAssistantWebSocketServer {
|
|||||||
|
|
||||||
private readonly ACTIVITY_THRESHOLD_MS = 120_000;
|
private readonly ACTIVITY_THRESHOLD_MS = 120_000;
|
||||||
|
|
||||||
private getClientActivityState(session: Session): {
|
private getClientActivityState(session: Session): ClientAttentionState {
|
||||||
deviceType: "web" | "mobile" | null;
|
|
||||||
focusedAgentId: string | null;
|
|
||||||
isStale: boolean;
|
|
||||||
appVisible: boolean;
|
|
||||||
} {
|
|
||||||
const activity = session.getClientActivity();
|
const activity = session.getClientActivity();
|
||||||
if (!activity) {
|
if (!activity) {
|
||||||
return { deviceType: null, focusedAgentId: null, isStale: true, appVisible: false };
|
return { deviceType: null, focusedAgentId: null, isStale: true, appVisible: false };
|
||||||
@@ -673,58 +673,6 @@ export class VoiceAssistantWebSocketServer {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
private computeShouldNotifyForClient(
|
|
||||||
clientState: {
|
|
||||||
deviceType: "web" | "mobile" | null;
|
|
||||||
focusedAgentId: string | null;
|
|
||||||
isStale: boolean;
|
|
||||||
appVisible: boolean;
|
|
||||||
},
|
|
||||||
allClientStates: Array<{
|
|
||||||
deviceType: "web" | "mobile" | null;
|
|
||||||
focusedAgentId: string | null;
|
|
||||||
isStale: boolean;
|
|
||||||
appVisible: boolean;
|
|
||||||
}>,
|
|
||||||
agentId: string
|
|
||||||
): boolean {
|
|
||||||
const isAnyoneActiveOnAgent = allClientStates.some(
|
|
||||||
(state) => state.focusedAgentId === agentId && state.appVisible && !state.isStale
|
|
||||||
);
|
|
||||||
if (isAnyoneActiveOnAgent) {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (clientState.deviceType === null) {
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!clientState.isStale && clientState.appVisible && clientState.focusedAgentId !== null) {
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!clientState.isStale) {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
|
|
||||||
const hasActiveWebClient = allClientStates.some(
|
|
||||||
(state) => state.deviceType === "web" && !state.isStale
|
|
||||||
);
|
|
||||||
|
|
||||||
if (clientState.deviceType === "mobile") {
|
|
||||||
return !hasActiveWebClient;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (clientState.deviceType === "web") {
|
|
||||||
const hasOtherClient = allClientStates.some(
|
|
||||||
(state) => state !== clientState && (state.deviceType === "mobile" || state.deviceType === null)
|
|
||||||
);
|
|
||||||
return !hasOtherClient;
|
|
||||||
}
|
|
||||||
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
private broadcastAgentAttention(params: {
|
private broadcastAgentAttention(params: {
|
||||||
agentId: string;
|
agentId: string;
|
||||||
provider: AgentProvider;
|
provider: AgentProvider;
|
||||||
@@ -732,12 +680,7 @@ export class VoiceAssistantWebSocketServer {
|
|||||||
}): void {
|
}): void {
|
||||||
const clientEntries: Array<{
|
const clientEntries: Array<{
|
||||||
ws: WebSocketLike;
|
ws: WebSocketLike;
|
||||||
state: {
|
state: ClientAttentionState;
|
||||||
deviceType: "web" | "mobile" | null;
|
|
||||||
focusedAgentId: string | null;
|
|
||||||
isStale: boolean;
|
|
||||||
appVisible: boolean;
|
|
||||||
};
|
|
||||||
}> = [];
|
}> = [];
|
||||||
|
|
||||||
for (const [ws, connection] of this.sessions) {
|
for (const [ws, connection] of this.sessions) {
|
||||||
@@ -749,19 +692,12 @@ export class VoiceAssistantWebSocketServer {
|
|||||||
|
|
||||||
const allStates = clientEntries.map((e) => e.state);
|
const allStates = clientEntries.map((e) => e.state);
|
||||||
|
|
||||||
const hasActiveWebClient = allStates.some(
|
// Push is only a fallback when the user is away from desktop/web.
|
||||||
(state) => state.deviceType === "web" && !state.isStale
|
|
||||||
);
|
|
||||||
const hasActiveMobileForegroundClient = allStates.some(
|
|
||||||
(state) => state.deviceType === "mobile" && state.appVisible && !state.isStale
|
|
||||||
);
|
|
||||||
|
|
||||||
// Push is only a fallback when the user is away from their desktop/web.
|
|
||||||
// Also suppress push if they're actively using the mobile app.
|
// Also suppress push if they're actively using the mobile app.
|
||||||
const shouldSendPush =
|
const shouldSendPush = computeShouldSendPush({
|
||||||
params.reason !== "error" &&
|
reason: params.reason,
|
||||||
!hasActiveWebClient &&
|
allClientStates: allStates,
|
||||||
!hasActiveMobileForegroundClient;
|
});
|
||||||
|
|
||||||
if (shouldSendPush) {
|
if (shouldSendPush) {
|
||||||
const tokens = this.pushTokenStore.getAllTokens();
|
const tokens = this.pushTokenStore.getAllTokens();
|
||||||
@@ -789,7 +725,11 @@ export class VoiceAssistantWebSocketServer {
|
|||||||
}
|
}
|
||||||
|
|
||||||
for (const { ws, state } of clientEntries) {
|
for (const { ws, state } of clientEntries) {
|
||||||
const shouldNotify = this.computeShouldNotifyForClient(state, allStates, params.agentId);
|
const shouldNotify = computeShouldNotifyClient({
|
||||||
|
clientState: state,
|
||||||
|
allClientStates: allStates,
|
||||||
|
agentId: params.agentId,
|
||||||
|
});
|
||||||
|
|
||||||
const message = wrapSessionMessage({
|
const message = wrapSessionMessage({
|
||||||
type: "agent_stream",
|
type: "agent_stream",
|
||||||
|
|||||||
Reference in New Issue
Block a user