From ce28021af16a86279ae90033441e3a92aa4972b8 Mon Sep 17 00:00:00 2001 From: siddseethepalli Date: Sat, 18 Apr 2026 22:54:32 +0000 Subject: [PATCH] fix(wake): distinguish timeout from not-found; tighten proactive-chat timing --- assistant/src/cli/commands/conversations.ts | 12 ++++- assistant/src/prompts/update-bulletin-job.ts | 2 +- .../src/runtime/__tests__/agent-wake.test.ts | 45 ++++++++++++++++++- assistant/src/runtime/agent-wake.ts | 16 +++++-- .../__tests__/proactive-chat-e2e.test.ts | 7 ++- 5 files changed, 73 insertions(+), 9 deletions(-) diff --git a/assistant/src/cli/commands/conversations.ts b/assistant/src/cli/commands/conversations.ts index 3b5ace1e282..f0b54009688 100644 --- a/assistant/src/cli/commands/conversations.ts +++ b/assistant/src/cli/commands/conversations.ts @@ -432,6 +432,7 @@ Examples: const result = await cliIpcCall<{ invoked: boolean; producedToolCalls: boolean; + reason?: "not_found" | "timeout" | "no_resolver"; }>("wake_conversation", { conversationId, hint: opts.hint, @@ -451,12 +452,21 @@ Examples: const wake = result.result!; if (opts.json) { log.info(JSON.stringify({ ok: true, ...wake })); - } else if (wake.invoked) { + return; + } + if (wake.invoked) { log.info( wake.producedToolCalls ? `Wake produced output on conversation ${conversationId}` : `Wake invoked on ${conversationId} (no output produced)`, ); + } else if (wake.reason === "timeout") { + // Conversation exists but stayed busy past the wait-until-idle + // window. This is a transient condition, not an error — the + // caller can retry later. Exit 0. + log.info( + `Conversation ${conversationId} is busy — wake skipped (retry later)`, + ); } else { log.error( `Could not wake conversation ${conversationId} — conversation not found`, diff --git a/assistant/src/prompts/update-bulletin-job.ts b/assistant/src/prompts/update-bulletin-job.ts index f1213e0c6ae..52b3ae63524 100644 --- a/assistant/src/prompts/update-bulletin-job.ts +++ b/assistant/src/prompts/update-bulletin-job.ts @@ -106,7 +106,7 @@ export async function runUpdateBulletinJobIfNeeded(): Promise { if (!wakeResult.invoked) { log.warn( - { conversationId: conv.id }, + { conversationId: conv.id, reason: wakeResult.reason }, "Update bulletin wake silently no-op'd (invoked=false); cleaning up orphan background conversation and leaving checkpoint unchanged so next startup retries", ); // Belt-and-suspenders cleanup: even though `runUpdateBulletinJobIfNeeded` diff --git a/assistant/src/runtime/__tests__/agent-wake.test.ts b/assistant/src/runtime/__tests__/agent-wake.test.ts index 1d9fe53e05b..bab1409fb83 100644 --- a/assistant/src/runtime/__tests__/agent-wake.test.ts +++ b/assistant/src/runtime/__tests__/agent-wake.test.ts @@ -490,12 +490,53 @@ describe("wakeAgentForOpportunity", () => { expect(result.producedToolCalls).toBe(false); }); - test("returns invoked: false when the conversation cannot be resolved", async () => { + test("returns invoked: false with reason 'not_found' when the conversation cannot be resolved", async () => { const result = await wakeAgentForOpportunity( { conversationId: "missing", hint: "x", source: "y" }, { resolveTarget: async () => null }, ); - expect(result).toEqual({ invoked: false, producedToolCalls: false }); + expect(result).toEqual({ + invoked: false, + producedToolCalls: false, + reason: "not_found", + }); + }); + + test("returns invoked: false with reason 'timeout' when the target stays busy past the wait-until-idle window", async () => { + // Resolver returns a target that is permanently `processing`. Fast- + // forward the injected `now` past the 30s deadline so waitUntilIdle + // returns false. Without the distinct `timeout` reason, callers + // cannot tell this case apart from "not_found". + const history: Message[] = []; + const target: WakeTarget = { + conversationId: "conv-busy", + agentLoop: { run: async () => history }, + getMessages: () => history, + pushMessage: () => {}, + emitAgentEvent: () => {}, + isProcessing: () => true, + markProcessing: () => {}, + persistTailMessage: async () => {}, + }; + let t = 0; + const now = () => { + // First call establishes the deadline at +30_000. Every subsequent + // call jumps past the deadline so the polling loop exits after one + // 50ms tick. + const v = t; + t += 31_000; + return v; + }; + + const result = await wakeAgentForOpportunity( + { conversationId: "conv-busy", hint: "x", source: "y" }, + { resolveTarget: async () => target, now }, + ); + expect(result).toEqual({ + invoked: false, + producedToolCalls: false, + reason: "timeout", + }); }); test("agent loop error is treated as a no-op", async () => { diff --git a/assistant/src/runtime/agent-wake.ts b/assistant/src/runtime/agent-wake.ts index 5058904e8f8..7411dbc5e1d 100644 --- a/assistant/src/runtime/agent-wake.ts +++ b/assistant/src/runtime/agent-wake.ts @@ -119,9 +119,19 @@ export interface WakeOptions { source: string; } +/** + * Reason a wake returned `invoked: false`. Callers (CLI, update-bulletin + * job) need to distinguish "conversation doesn't exist" from "conversation + * exists but stayed busy past the wait-until-idle timeout" — the former is + * a user-visible error, the latter is an expected transient condition. + */ +export type WakeSkipReason = "not_found" | "timeout" | "no_resolver"; + export interface WakeResult { invoked: boolean; producedToolCalls: boolean; + /** Present only when `invoked: false`; identifies why the wake was skipped. */ + reason?: WakeSkipReason; } /** @@ -299,7 +309,7 @@ export async function wakeAgentForOpportunity( { conversationId, source }, "agent-wake: no resolver available (default resolver not registered and no deps passed); skipping", ); - return { invoked: false, producedToolCalls: false }; + return { invoked: false, producedToolCalls: false, reason: "no_resolver" }; } const nowFn = deps?.now ?? Date.now; const startedAt = nowFn(); @@ -311,7 +321,7 @@ export async function wakeAgentForOpportunity( { conversationId, source }, "agent-wake: conversation not found; skipping", ); - return { invoked: false, producedToolCalls: false }; + return { invoked: false, producedToolCalls: false, reason: "not_found" }; } const idle = await waitUntilIdle(target, nowFn); @@ -320,7 +330,7 @@ export async function wakeAgentForOpportunity( { conversationId, source }, "agent-wake: conversation still processing after timeout; skipping", ); - return { invoked: false, producedToolCalls: false }; + return { invoked: false, producedToolCalls: false, reason: "timeout" }; } const baseline = target.getMessages(); diff --git a/skills/meet-join/daemon/__tests__/proactive-chat-e2e.test.ts b/skills/meet-join/daemon/__tests__/proactive-chat-e2e.test.ts index fd1e3c94d8b..b8c931b6652 100644 --- a/skills/meet-join/daemon/__tests__/proactive-chat-e2e.test.ts +++ b/skills/meet-join/daemon/__tests__/proactive-chat-e2e.test.ts @@ -549,8 +549,11 @@ describe("proactive-chat E2E — Tier 1 hit → Tier 2 confirms → agent wake expect(blocks[0]!.type).toBe("tool_use"); expect(blocks[0]!.name).toBe("meet_send_chat"); - // Performance envelope — generous headroom for CI runners. - expect(elapsedMs).toBeLessThan(2000); + // Performance envelope — tight enough to catch real regressions but + // loose enough to tolerate slow CI runners. The underlying fake-LLM + // path completes well under 100ms on developer hardware; 500ms is a + // 5x headroom that flags genuine perf drift without flaking. + expect(elapsedMs).toBeLessThan(500); detector.dispose(); } finally {