diff --git a/doc/idle-voice-inbound-cleanup.md b/doc/idle-voice-inbound-cleanup.md index a0830f19912..8f657190966 100644 --- a/doc/idle-voice-inbound-cleanup.md +++ b/doc/idle-voice-inbound-cleanup.md @@ -42,6 +42,11 @@ index covers unconfirmed admission identities; startup inventory is paged. ## Deadlines and quiet state +Rejection returns after its decision transaction commits. Its notification wakes +the registered cleanup worker; the HTTP request does not wait for provider account +verification, inspection, or hangup. The saved denial immediately rejects further +unauthorized call input while physical termination remains tracked separately. + Pending unbound admissions retain their original two-minute approval or ten-minute guest-intake expiry. Rejection is immediately due. A cleanup claim saves a 30-second retry before provider I/O. An unconfirmed end receipt keeps that retry; diff --git a/doc/plans/2026-10-25-voice-inbound-cleanup-sleep-audit.md b/doc/plans/2026-10-25-voice-inbound-cleanup-sleep-audit.md index 5f1a1779168..2bed1a020eb 100644 --- a/doc/plans/2026-10-25-voice-inbound-cleanup-sleep-audit.md +++ b/doc/plans/2026-10-25-voice-inbound-cleanup-sleep-audit.md @@ -218,3 +218,30 @@ Database typecheck and migration numbering/safety checks also pass. Local logs: `/tmp/sleep23-migrator-before.log`, `/tmp/sleep23-migrator-after.log`, and `/tmp/sleep23-migrator-types.log`. Fresh remote checks remain required; the original CI failure is not classified as a flake. + +## Denial request latency review repair + +Review finding 4370459974 correctly identified that denial awaited account +verification, provider inspection and endSession, despite already committing the +rejection and its cleanup notification. The request now returns the saved denial; +the registered worker owns physical termination and persists its retry. No detached +promise or second dispatch path is added. Per-attempt account verification stays +with the provider boundary, protecting rotated credentials from a different +account. This patch does not cache provider authority indefinitely. + +A new real-producer regression holds verifyAgent unresolved, proves the committed +denial returns, checks that cleanup is still unconfirmed with a saved retry, then +releases the provider and verifies confirmed termination. It failed before the +fix (`/tmp/sleep23-denial-before.log`) and passed after +(`/tmp/sleep23-denial-after.log`). Existing explicit one-shot tests now invoke +that one-shot path rather than relying on the HTTP decision to do cleanup. +All 130 full voice and app wiring cases pass on a fresh migrated PostgreSQL DB; +production server typecheck passes. Logs: `/tmp/sleep23-denial-acceptance.log` +and `/tmp/sleep23-denial-types.log`. + +Finding 4370459800 about company deletion remains separately unresolved. Missing +credentials and account mismatch retain cleanup debt. There is a tested recovery +path through restoring the original account, but a permanently unavailable +provider needs a deliberate ownership/product policy. A retry cap, elapsed time, +or ambiguous 404 is not evidence that a billable call ended. Do not manufacture +cleanup confirmation or silently discard the obligation to unblock deletion. diff --git a/server/src/__tests__/voice-session-store.integration.test.ts b/server/src/__tests__/voice-session-store.integration.test.ts index d15679ada8c..c1979204a3e 100644 --- a/server/src/__tests__/voice-session-store.integration.test.ts +++ b/server/src/__tests__/voice-session-store.integration.test.ts @@ -138,8 +138,10 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu const ended = f.sign({ type: "call.status", call_id: f.call.providerSessionId, status: "ended" }); await f.service.inbound.lifecycle(f.endpoint.publicId, ended.body, ended.headers); } else { + await f.service.inbound.registerCleanupWork(coordinator, () => true).ready; await f.service.inbound.decide(f.companyId, f.endpointId, f.call.id, f.input.caller, { approve: false, approvalCode: f.call.approvalCode }); + await settle(); expect(f.transport.endSession).toHaveBeenCalledWith(f.call.providerSessionId); } await settle(); @@ -153,6 +155,39 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu } finally { await coordinator.stop(); vi.useRealTimers(); } }); + it("returns a committed denial while provider cleanup is still waiting", async () => { + const f = await inboundFixture(); + let release!: () => void; + const pendingProvider = new Promise(resolve => { release = resolve; }); + f.transport.verifyAgent.mockImplementation(async () => { + await pendingProvider; + return { id: "agent_test", organizationId: `org_${f.companyId}`, name: "Test voice" }; + }); + const onError = vi.fn(), coordinator = createDeliveryWorkCoordinator({ owner: db, canRun: () => true, onError }); + let denial: ReturnType | undefined; + try { + await f.service.inbound.registerCleanupWork(coordinator, () => true).ready; + let returned = false; + denial = f.service.inbound.decide(f.companyId, f.endpointId, f.call.id, f.input.caller, + { approve: false, approvalCode: f.call.approvalCode }).then(result => { returned = true; return result; }); + await vi.waitFor(() => expect(returned).toBe(true), { timeout: 2000 }); + expect(await denial).toMatchObject({ state: "denied" }); + await vi.waitFor(() => expect(f.transport.verifyAgent).toHaveBeenCalled(), { timeout: 5000 }); + expect(f.transport.endSession).not.toHaveBeenCalled(); + const [pending] = await db.select().from(chatVoiceInboundCalls).where(eq(chatVoiceInboundCalls.id, f.call.id)); + expect(pending.cleanupConfirmedAt).toBeNull(); + expect(pending.cleanupNextAttemptAt).not.toBeNull(); + release(); + await vi.waitFor(async () => { + const [settled] = await db.select().from(chatVoiceInboundCalls).where(eq(chatVoiceInboundCalls.id, f.call.id)); + expect(settled.cleanupConfirmedAt).not.toBeNull(); + }, { timeout: 5000 }); + expect(f.transport.endSession).toHaveBeenCalledOnce(); expect(onError).not.toHaveBeenCalled(); + } finally { + release(); await denial; await coordinator.stop(); + } + }); + it("wakes inbound cleanup from a real signed pre-call and retains an unconfirmed end", async () => { const f = await inboundFixture(); const ended = f.sign({ type: "call.status", call_id: f.call.providerSessionId, status: "ended" }); @@ -201,6 +236,7 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu const result = await f.service.inbound.decide(f.companyId, f.endpointId, f.call.id, f.input.caller, { approve: false, approvalCode: f.call.approvalCode }); expect(result.state).toBe("ended"); + await f.service.inbound.reconcile(1000); const [pending] = await db.select().from(chatVoiceInboundCalls).where(eq(chatVoiceInboundCalls.id, f.call.id)); expect(pending.cleanupConfirmedAt).toBeNull(); expect(pending.cleanupNextAttemptAt).not.toBeNull(); await db.update(chatVoiceInboundCalls).set({ expiresAt: new Date(0), updatedAt: new Date(0) }).where(eq(chatVoiceInboundCalls.id, f.call.id)); @@ -225,6 +261,7 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu f.transport.verifyAgent.mockResolvedValue({ id: "other-agent", organizationId: "another-account", name: "Other voice" }); await f.service.inbound.decide(f.companyId, f.endpointId, f.call.id, f.input.caller, { approve: false, approvalCode: f.call.approvalCode }); + await f.service.inbound.reconcile(1000); expect(f.transport.inspectSession).not.toHaveBeenCalled(); expect(f.transport.endSession).not.toHaveBeenCalled(); const [pending] = await db.select().from(chatVoiceInboundCalls).where(eq(chatVoiceInboundCalls.id, f.call.id)); expect(pending.cleanupConfirmedAt).toBeNull(); expect(pending.cleanupNextAttemptAt).not.toBeNull(); @@ -1511,6 +1548,7 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu it("denies expired and rejected calls and closes the existing provider leg without creating work", async () => { const f = await inboundFixture(); expect(await f.service.inbound.decide(f.companyId, f.endpointId, f.call.id, f.input.caller, {approve: false, approvalCode: f.call.approvalCode})).toMatchObject({state: "denied"}); + await f.service.inbound.reconcile(1000); expect(f.transport.endSession).toHaveBeenCalledWith(f.event.session_id); expect(await f.tool("submit_request", {text: "Try again"})).toMatchObject({authorization: "denied", requestAccepted: false}); const expired = await inboundFixture(); diff --git a/server/src/services/voice/voice-inbound-service.ts b/server/src/services/voice/voice-inbound-service.ts index 87eecd773fa..5c781d5a21b 100644 --- a/server/src/services/voice/voice-inbound-service.ts +++ b/server/src/services/voice/voice-inbound-service.ts @@ -249,9 +249,9 @@ export function voiceInboundService(db: Db, options: { }); if (admission.state === "approved") return projection(admission); if (!input.approve) { - // Bound calls have already been handed to live-session recovery. Unbound - // rejection uses the same durable claim as scheduled cleanup. - await cleanup.check(admission.id).catch(error => logger.warn({ err: error, callId: admission.id }, "Incoming call termination remains pending")); + // The committed admission signal owns cleanup. Return the saved denial + // without waiting for provider account verification or physical hangup. + // Bound calls have already been handed to live-session recovery. return projection(admission); } const row = await endpoint(companyId, endpointId), credentials = await options.credentials(row, admission.credentialFingerprint);