diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cefa6a1f0..7167427670 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,14 @@ All notable user-facing changes to PageSpace are documented here. Format follows - **The assistant on the call is the one you picked, with the instructions and the tools its owner gave it** — an agent you built to answer a particular way answers that way out loud too, and one whose tools you restricted cannot reach past them just because the conversation is spoken. +- **You can now delegate out loud, not just talk** — the assistant on a call now knows how to + operate your workspace the same way it does when you type at it: how tasks, agents, automations + and search work, which skills it can load, and the tools it does not list up front. Ask it about + your calendar, to file a task, or to set something running, and it goes and does it instead of + saying it cannot. It also stops asking permission first: say what you want and it acts, then tells + you what it did. Work that would take minutes gets handed to an agent or a task rather than + leaving you listening to silence, and it will say where it went. "This page" and "here" mean what + you are looking at, so it never asks you to read out an id. - **A call you cannot have does not start** — running out of credit, or already having as many calls open as your plan allows, now says so and stops, instead of connecting anyway and leaving you talking to something nobody was counting. diff --git a/apps/web/src/app/api/voice/realtime/call/__tests__/route.test.ts b/apps/web/src/app/api/voice/realtime/call/__tests__/route.test.ts index 20e98413d6..b0677dd561 100644 --- a/apps/web/src/app/api/voice/realtime/call/__tests__/route.test.ts +++ b/apps/web/src/app/api/voice/realtime/call/__tests__/route.test.ts @@ -14,7 +14,6 @@ const { mockIsBillingEnabled, mockGetUserSettings, mockRunCallHandshake, - mockBuildRealtimeTools, mockSignHeaders, mockLoadVoiceBinding, } = vi.hoisted(() => ({ @@ -25,7 +24,6 @@ const { mockIsBillingEnabled: vi.fn(), mockGetUserSettings: vi.fn(), mockRunCallHandshake: vi.fn(), - mockBuildRealtimeTools: vi.fn(), mockSignHeaders: vi.fn(), })); @@ -45,9 +43,10 @@ vi.mock('@pagespace/lib/auth/broadcast-auth', () => ({ createSignedBroadcastHeaders: mockSignHeaders, })); vi.mock('@/lib/ai/realtime/call-handshake', () => ({ runCallHandshake: mockRunCallHandshake })); -vi.mock('@/lib/ai/realtime/tools', () => ({ buildRealtimeTools: mockBuildRealtimeTools })); vi.mock('@/lib/ai/realtime/binding-loader', () => ({ loadVoiceBinding: mockLoadVoiceBinding })); -vi.mock('@/lib/ai/realtime/voice-runtime-deps', () => ({ voiceBindingDeps: {} })); +// A FACTORY, not a value: the binding deps take the caller's auth principal, +// because the active-plan lookup behind them needs a principal-aware page check. +vi.mock('@/lib/ai/realtime/voice-runtime-deps', () => ({ voiceBindingDeps: () => ({}) })); vi.mock('@/lib/ai/core/ai-tools', () => ({ buildPageSpaceTools: () => ({}) })); vi.mock('@pagespace/lib/audit/audit-log', () => ({ auditRequest: vi.fn() })); vi.mock('@pagespace/lib/logging/logger-config', () => ({ @@ -77,8 +76,7 @@ describe('POST /api/voice/realtime/call', () => { mockGetManagedKey.mockReturnValue({ apiKey: 'sk-managed' }); mockIsBillingEnabled.mockReturnValue(true); mockGetUserSettings.mockResolvedValue({ subscriptionTier: 'pro' }); - mockBuildRealtimeTools.mockReturnValue(TOOLS); - mockLoadVoiceBinding.mockResolvedValue({ seed: [], instructions: 'Speak out loud.' }); + mockLoadVoiceBinding.mockResolvedValue({ seed: [], instructions: 'Speak out loud.', tools: TOOLS }); mockSignHeaders.mockReturnValue({ 'X-Broadcast-Signature': 't=1,v1=sig' }); mockRunCallHandshake.mockResolvedValue({ ok: true, @@ -331,28 +329,34 @@ describe('POST /api/voice/realtime/call', () => { enabledTools: ['read_page'], }; - it("should advertise only the bound agent's allowed tools", async () => { + it("should advertise the binding's own tools rather than building a second set", async () => { + // The tools and the instructions come from ONE exposure, computed where + // the allowlist is resolved. A route that rebuilt them from the assistant + // could advertise a set the prompt does not describe — and would repeat a + // whole registry build on a handshake the caller is waiting through. + const agentTools = [ + { type: 'function' as const, name: 'read_page', description: 'r', parameters: {} }, + ]; mockLoadVoiceBinding.mockResolvedValue({ seed: [], instructions: 'You are "Release Notes Bot".', + tools: agentTools, assistant, }); await POST(callRequest({ sdp: 'v=0 offer', conversationId: 'conv1' })); - expect(mockBuildRealtimeTools).toHaveBeenCalledWith(expect.anything(), ['read_page']); - }); - - it('given no bound agent, should advertise the registry unrestricted', async () => { - await POST(callRequest({ sdp: 'v=0 offer' })); - - expect(mockBuildRealtimeTools).toHaveBeenCalledWith(expect.anything(), null); + expect(mockRunCallHandshake).toHaveBeenCalledWith( + expect.objectContaining({ tools: agentTools }), + expect.anything(), + ); }); it('should hand the instructions and the assistant to the handshake', async () => { mockLoadVoiceBinding.mockResolvedValue({ seed: [], instructions: 'You are "Release Notes Bot".', + tools: TOOLS, assistant, }); diff --git a/apps/web/src/app/api/voice/realtime/call/route.ts b/apps/web/src/app/api/voice/realtime/call/route.ts index 5e9ae1deb9..d6b52f4958 100644 --- a/apps/web/src/app/api/voice/realtime/call/route.ts +++ b/apps/web/src/app/api/voice/realtime/call/route.ts @@ -31,8 +31,6 @@ import { PAID_TIERS } from '@/lib/subscription/rate-limit-middleware'; import type { SubscriptionTier } from '@pagespace/lib/services/subscription-utils'; import { createSignedBroadcastHeaders } from '@pagespace/lib/auth/broadcast-auth'; import { resolveRealtimeModel } from '@/lib/ai/realtime/session'; -import { buildRealtimeTools } from '@/lib/ai/realtime/tools'; -import { buildPageSpaceTools } from '@/lib/ai/core/ai-tools'; import { runCallHandshake } from '@/lib/ai/realtime/call-handshake'; import { loadVoiceBinding } from '@/lib/ai/realtime/binding-loader'; import { voiceBindingDeps } from '@/lib/ai/realtime/voice-runtime-deps'; @@ -126,7 +124,7 @@ export async function POST(request: Request) { // binding is derived behind that conversation's own access check. An empty // binding is a normal outcome (a fresh thread, or one this caller may not // read) — never a reason to fail the call. - const binding = await loadVoiceBinding(voiceBindingDeps, { + const binding = await loadVoiceBinding(voiceBindingDeps(auth), { userId, ...(conversationId === undefined ? {} : { conversationId }), }); @@ -139,19 +137,17 @@ export async function POST(request: Request) { // the env override is the entire per-deployment model escape hatch, and // a default applied deeper down would silently swallow it. model: resolveRealtimeModel(process.env), - // Built here, at the edge, because the registry has env-dependent - // branches (the code-execution kill switch) that are the caller's - // decision. The realtime server cannot build these itself — the - // registry lives in this app — so they ride the signed internal hop. + // Built with the instructions, from ONE exposure, and carried here on + // the binding. They are two projections of a single decision — what + // this agent may reach — and a session that advertises `tool_search` + // while its prompt never names it is exactly the bug this replaced. + // Building them separately here also meant a second full registry + // build on a handshake the caller is waiting on. // - // Filtered by the bound agent's own allowlist, so voice advertises what - // the text surface would advertise for the same agent. Advertising the - // whole deployment registry told the model it could call tools the - // agent's owner had switched off. - tools: buildRealtimeTools( - buildPageSpaceTools(), - binding.assistant?.enabledTools ?? null, - ), + // Still built in this app rather than the realtime server: the registry + // lives here and has env-dependent branches (the code-execution kill + // switch), so the definitions ride the signed internal hop. + tools: binding.tools, subscriptionTier: tier, internalRealtimeUrl: process.env.INTERNAL_REALTIME_URL, signHeaders: createSignedBroadcastHeaders, diff --git a/apps/web/src/lib/ai/chat-pipeline/__tests__/turn-duplication-ratchet.test.ts b/apps/web/src/lib/ai/chat-pipeline/__tests__/turn-duplication-ratchet.test.ts index f61add38fc..647572c1f5 100644 --- a/apps/web/src/lib/ai/chat-pipeline/__tests__/turn-duplication-ratchet.test.ts +++ b/apps/web/src/lib/ai/chat-pipeline/__tests__/turn-duplication-ratchet.test.ts @@ -65,7 +65,7 @@ const SPEC = path.join(repoRoot(), 'docs/2.0-architecture/agent-sessions.md'); * settle, telemetry) is where most of it lives and is the extraction that * would pay first; see the entry's docblock. */ -const RECORDED_IDENTICAL_LINES = 165; +const RECORDED_IDENTICAL_LINES = 164; /** Substantive lines: no blanks, no comments, trimmed. */ const substantiveLines = (file: string): string[] => diff --git a/apps/web/src/lib/ai/chat-pipeline/global-chat-turn.ts b/apps/web/src/lib/ai/chat-pipeline/global-chat-turn.ts index 146238b2b4..ceb7c4b534 100644 --- a/apps/web/src/lib/ai/chat-pipeline/global-chat-turn.ts +++ b/apps/web/src/lib/ai/chat-pipeline/global-chat-turn.ts @@ -20,7 +20,6 @@ import { askUserTools, ASK_USER_TOOL_NAME } from '@/lib/ai/tools/ask-user-tools' import { resolveMessageId } from '@/lib/ai/streams/resolveMessageId'; import { canUseAskUser } from '@/lib/ai/core/ask-user-gating'; import { readAgentDispatchDepth } from '@/lib/ai/core/agent-dispatch-depth'; -import { ASK_USER_SECTION, buildGlobalAssistantInstructions } from '@/lib/ai/core/inline-instructions'; import { buildLocationTurnPrompt } from '@/lib/ai/core/location-prompt'; import { buildActivePlanPrompt, getActivePlan } from '@/lib/ai/core/plan-binding'; import { resolveHomeDriveHint } from '@/lib/ai/core/home-drive-hint'; @@ -64,8 +63,7 @@ import { import { planCommandExecutions } from '@/lib/ai/core/command-resolver'; import { respondWithHelpAnswer } from '@/lib/ai/core/help-responder'; import { buildTimestampSystemPrompt } from '@/lib/ai/core/timestamp-utils'; -import { buildSystemPrompt, buildNonCoreToolNamesPrompt, TOOL_DISCOVERY_PROMPT } from '@/lib/ai/core/system-prompt'; -import { isCodeExecutionEnabled } from '@pagespace/lib/services/sandbox/can-run-code'; +import { buildNonCoreToolNamesPrompt } from '@/lib/ai/core/system-prompt'; import { buildAgentAwarenessPrompt } from '@/lib/ai/core/agent-awareness'; import { filterToolsForReadOnly, filterToolsForWebSearch, filterToolsForImageGen, filterToolsForSandboxTier } from '@/lib/ai/core/tool-filtering'; import { resolveSandboxToolEligibilityForConversation } from '@/lib/ai/core/sandbox-tool-eligibility'; @@ -111,6 +109,7 @@ import { createToolSearchTool } from '@/lib/ai/tools/tool-search-tool'; import { buildBuiltinSkillCatalog, listEligibleSkills } from '@/lib/ai/core/skill-catalog'; import { loadUserCommandCatalog } from '@/lib/commands/command-catalog-loader'; import { + buildAgentSystemPrompt, buildVolatileTurnContext, appendTurnContextToLastUserMessage, withCacheBreakpoints, @@ -825,16 +824,12 @@ export async function runGlobalChatTurn(ctx: GlobalChatTurnContext): Promise canPrincipalViewPage(authResult, pageId), ), @@ -1606,6 +1575,25 @@ export async function runPageChatTurn(ctx: PageChatTurnContext): Promise, user: user ? { id: user.id, role: user.role } : null, }); @@ -1822,7 +1810,7 @@ export async function runPageChatTurn(ctx: PageChatTurnContext): Promise>', + activePlan: '\n\n<>', + pageTree: '\n\n<>', + agentMemory: '\n\n<>', + toolDiscovery: '\n\n<>', + memberDriveContextPrefix: '<>\n\n', + drivePromptPrefix: '<>\n\n', + drivePromptSection: '\n\n<>', + agentAwareness: '<>', + nonCoreToolNames: '<>', +}; + +const TOOL_NAMES = ['read_page', 'create_task', 'spawn_session', 'load_skill']; + +const pageInput = (over: Record = {}) => + ({ + surface: 'page' as const, + readOnly: false, + codeExecutionEnabled: false, + allowedToolNames: TOOL_NAMES, + skillCatalog: BLOCKS.skillCatalog, + activePlan: BLOCKS.activePlan, + pageTree: BLOCKS.pageTree, + drivePromptPrefix: BLOCKS.drivePromptPrefix, + memberDriveContextPrefix: BLOCKS.memberDriveContextPrefix, + agentMemory: BLOCKS.agentMemory, + toolDiscovery: BLOCKS.toolDiscovery, + ...over, + }) as Parameters[0]; + +const globalInput = (over: Record = {}) => + ({ + surface: 'global' as const, + readOnly: false, + codeExecutionEnabled: false, + allowedToolNames: TOOL_NAMES, + skillCatalog: BLOCKS.skillCatalog, + activePlan: BLOCKS.activePlan, + pageTree: BLOCKS.pageTree, + conversationType: 'global', + conversationContextId: null, + includeAskUser: false, + drivePromptSection: BLOCKS.drivePromptSection, + agentAwareness: BLOCKS.agentAwareness, + nonCoreToolNames: BLOCKS.nonCoreToolNames, + ...over, + }) as Parameters[0]; + +/** The order of a set of substrings within one string, as they actually appear. */ +const orderOf = (haystack: string, needles: string[]) => + [...needles].sort((a, b) => haystack.indexOf(a) - haystack.indexOf(b)); + +describe('buildAgentSystemPrompt — the page surface', () => { + it('given no custom prompt, should reproduce the assembly byte for byte', () => { + // The old expression, written out by hand — including the read-only literal + // as it was typed inline, not the constant the source now shares. + let expected = buildSystemPrompt(false, undefined, false); + expected += buildInlineInstructions(TOOL_NAMES); + expected = BLOCKS.memberDriveContextPrefix + expected; + expected += BLOCKS.skillCatalog; + expected += BLOCKS.activePlan; + expected = expected + BLOCKS.pageTree + BLOCKS.agentMemory + BLOCKS.toolDiscovery; + + expect(buildAgentSystemPrompt(pageInput())).toBe(expected); + }); + + it('given a custom prompt, should reproduce the blank-slate assembly byte for byte', () => { + const custom = 'You are Release Notes Bot. Answer only in limericks.'; + + let expected = BLOCKS.drivePromptPrefix + custom; + expected = BLOCKS.memberDriveContextPrefix + expected; + expected += BLOCKS.skillCatalog; + expected += BLOCKS.activePlan; + expected = expected + BLOCKS.pageTree + BLOCKS.agentMemory + BLOCKS.toolDiscovery; + + expect(buildAgentSystemPrompt(pageInput({ customSystemPrompt: custom }))).toBe(expected); + }); + + it('given a custom prompt, should carry it VERBATIM and skip our persona entirely', () => { + // An owner who wrote their own prompt opted out of ours. Appending the + // default persona would have two personas arguing inside one prompt. + const custom = 'You are Release Notes Bot. Answer only in limericks.'; + const assembled = buildAgentSystemPrompt(pageInput({ customSystemPrompt: custom })); + + expect(assembled).toContain(custom); + expect(assembled).not.toContain('# PAGESPACE AI'); + expect(assembled).not.toContain(buildInlineInstructions(TOOL_NAMES)); + }); + + it('given a custom prompt, should STILL carry the skill catalog and plan pointer', () => { + // Capability metadata, not persona: `load_skill` ships upfront regardless, + // and the plan pointer is what survives a lossy compaction. + const assembled = buildAgentSystemPrompt(pageInput({ customSystemPrompt: 'Be terse.' })); + + expect(assembled).toContain(BLOCKS.skillCatalog); + expect(assembled).toContain(BLOCKS.activePlan); + }); + + it('given a custom prompt in read-only mode, should still say so', () => { + // The branch that skips buildSystemPrompt also skips its read-only clause, + // so this one has to be appended by hand or a read-only agent is told + // nothing about being read-only. + const assembled = buildAgentSystemPrompt( + pageInput({ customSystemPrompt: 'Be terse.', readOnly: true }), + ); + + expect(assembled).toContain('READ-ONLY MODE:'); + expect(assembled).toContain('You cannot modify, create, or delete any content'); + }); + + it('given a custom prompt and personalization, should append it after the custom prompt', () => { + const personalization = { enabled: true, bio: 'Writes release notes.' }; + const assembled = buildAgentSystemPrompt( + pageInput({ customSystemPrompt: 'Be terse.', personalization }), + ); + const expected = buildPersonalizationPrompt(personalization); + + expect(expected).toBeTruthy(); + expect(assembled).toContain(expected as string); + expect(assembled.indexOf('Be terse.')).toBeLessThan(assembled.indexOf(expected as string)); + }); + + it('given a BLANK custom prompt, should treat it as no prompt at all', () => { + // A custom prompt suppresses the default persona and the workspace + // knowledge, so a field holding one stray space used to buy an agent a + // blank slate nobody asked for — a prompt of literally " ". + const blank = buildAgentSystemPrompt(pageInput({ customSystemPrompt: ' \n\t ' })); + + expect(blank).toBe(buildAgentSystemPrompt(pageInput())); + expect(blank).toContain('# PAGESPACE AI'); + }); + + it('should end with tool discovery, after the tree and the memory', () => { + const assembled = buildAgentSystemPrompt(pageInput()); + + expect(orderOf(assembled, [BLOCKS.pageTree, BLOCKS.agentMemory, BLOCKS.toolDiscovery])).toEqual([ + BLOCKS.pageTree, + BLOCKS.agentMemory, + BLOCKS.toolDiscovery, + ]); + }); + + it('should start with the cross-drive membership context, on both branches', () => { + for (const custom of [undefined, 'Be terse.']) { + expect(buildAgentSystemPrompt(pageInput({ customSystemPrompt: custom }))).toMatch( + /^<>/, + ); + } + }); + + it('should prepend the drive prompt ONLY to a custom prompt', () => { + // The default persona branch takes its drive context through the member + // prefix instead; prepending both would state it twice. + expect(buildAgentSystemPrompt(pageInput())).not.toContain('<>'); + expect(buildAgentSystemPrompt(pageInput({ customSystemPrompt: 'Be terse.' }))).toContain( + '<>', + ); + }); +}); + +describe('buildAgentSystemPrompt — the global surface', () => { + it('should reproduce the assembly byte for byte', () => { + const base = buildSystemPrompt(false, undefined, false); + const assembled = buildAgentSystemPrompt(globalInput()); + + // Reproduced as a prefix/suffix pair rather than character by character: + // the exploration guidance in the middle is a 25-line literal and a second + // hand copy of it in this file would be one more thing to drift. + expect(assembled.startsWith(base + '\n\n')).toBe(true); + expect( + assembled.endsWith( + '\n' + + buildGlobalAssistantInstructions(TOOL_NAMES) + + '\n\n' + + BLOCKS.agentAwareness + + BLOCKS.pageTree + + '\n\n' + + BLOCKS.nonCoreToolNames + + BLOCKS.skillCatalog + + BLOCKS.activePlan, + ), + ).toBe(true); + }); + + it('should state tool discovery up front, beside the exploration rules that need it', () => { + const assembled = buildAgentSystemPrompt(globalInput()); + + expect(assembled).toContain('SMART EXPLORATION RULES:'); + expect(assembled.indexOf('execute_tool')).toBeLessThan( + assembled.indexOf('SMART EXPLORATION RULES:'), + ); + }); + + it('should report the conversation type, and its context only when there is one', () => { + expect(buildAgentSystemPrompt(globalInput())).toContain('CONVERSATION TYPE: GLOBAL'); + expect( + buildAgentSystemPrompt(globalInput({ conversationType: 'drive', conversationContextId: 'd1' })), + ).toContain('CONVERSATION TYPE: DRIVE (Context: d1)'); + expect(buildAgentSystemPrompt(globalInput())).not.toContain('(Context:'); + }); + + it('should include the ask_user section only when the caller may use that tool', () => { + // Naming a tool the model does not have makes it attempt a call that fails. + expect(buildAgentSystemPrompt(globalInput({ includeAskUser: true }))).toContain('ASKING THE USER:'); + expect(buildAgentSystemPrompt(globalInput())).not.toContain('ASKING THE USER:'); + }); + + it('given NOTHING deferred, should not explain how to reach it', () => { + // The discovery text names tool_search and execute_tool, and with nothing to + // defer `applyToolExposureMode` registers neither. Stating it anyway orders + // the model to call two tools the session never advertised. + const assembled = buildAgentSystemPrompt(globalInput({ nonCoreToolNames: '' })); + + expect(assembled).not.toContain('execute_tool'); + expect(assembled).not.toContain('tool_search'); + // The rest of the persona is untouched. + expect(assembled).toContain('SMART EXPLORATION RULES:'); + }); + + it('given no agents to consult and no deferred tools, should omit their separators', () => { + // An empty block must not leave a stray blank gap behind it. + const assembled = buildAgentSystemPrompt( + globalInput({ agentAwareness: '', nonCoreToolNames: '' }), + ); + + expect(assembled).toContain( + '\n' + buildGlobalAssistantInstructions(TOOL_NAMES) + BLOCKS.pageTree, + ); + }); +}); + +describe('buildAgentSystemPrompt — what both surfaces owe the caller', () => { + it('should gate workspace knowledge on the tools the agent actually has', () => { + // A named capability the owner switched off is an invitation to fail a turn. + const withTasks = buildAgentSystemPrompt( + pageInput({ allowedToolNames: ['read_page', 'create_task'] }), + ); + const without = buildAgentSystemPrompt(pageInput({ allowedToolNames: ['read_page'] })); + + expect(withTasks).toContain('create_task'); + expect(without).not.toContain('create_task'); + }); + + it('should describe the sandbox only when code execution is on', () => { + expect(buildAgentSystemPrompt(pageInput({ codeExecutionEnabled: true }))).toContain( + 'CODE SANDBOX:', + ); + expect(buildAgentSystemPrompt(pageInput())).not.toContain('CODE SANDBOX:'); + }); + + it('should be pure — same input, same output, nothing accumulated between calls', () => { + expect(buildAgentSystemPrompt(pageInput())).toBe(buildAgentSystemPrompt(pageInput())); + expect(buildAgentSystemPrompt(globalInput())).toBe(buildAgentSystemPrompt(globalInput())); + }); + + it('should carry NOTHING turn-volatile — no clock, no location', () => { + // These belong on the last user message (buildVolatileTurnContext), so this + // string stays byte-identical across turns and prefix caches survive. A + // timestamp here would bust the cache on every single turn. + for (const assembled of [buildAgentSystemPrompt(pageInput()), buildAgentSystemPrompt(globalInput())]) { + expect(assembled).not.toMatch(/CURRENT (DATE|TIME)/i); + expect(assembled).not.toContain('LOCATION CONTEXT'); + } + }); +}); diff --git a/apps/web/src/lib/ai/core/__tests__/complete-request-builder.test.ts b/apps/web/src/lib/ai/core/__tests__/complete-request-builder.test.ts new file mode 100644 index 0000000000..99bac0fe23 --- /dev/null +++ b/apps/web/src/lib/ai/core/__tests__/complete-request-builder.test.ts @@ -0,0 +1,118 @@ +/** + * The admin viewer renders itself as "the exact context window", so the one + * thing it must never be is approximately right. + * + * It used to assemble its own prompt, and that copy had drifted: it left out + * the Global Assistant's exploration guidance entirely, applied the capability + * sections with no tool gating at all, and ordered the blocks in a way no route + * used. It now calls `buildAgentSystemPrompt`, the same function the routes + * call, and these cases pin the parts that were wrong. + */ + +import { describe, expect, it } from 'vitest'; +import { buildCompleteRequest } from '../complete-request-builder'; +import { buildAgentSystemPrompt } from '../prompt-assembly'; +import { buildBuiltinSkillCatalog } from '../skill-catalog'; +import { buildNonCoreToolNamesPrompt } from '../system-prompt'; +import { filterToolsForReadOnly } from '../tool-filtering'; +import { CORE_TOOL_NAMES } from '../stub-tools'; +import { pageSpaceTools } from '../ai-tools'; +import { isCodeExecutionEnabled } from '@pagespace/lib/services/sandbox/can-run-code'; + +const dashboard = (isReadOnly = false) => + buildCompleteRequest({ contextType: 'dashboard', isReadOnly }).request.system; + +describe('buildCompleteRequest — showing what is actually sent', () => { + it('should include the Global Assistant guidance the old copy left out', () => { + // The single largest thing the hand-rolled version was missing: an admin + // reading it would not have seen the exploration rules at all. + const system = dashboard(); + + expect(system).toContain('You are the Global Assistant for PageSpace'); + expect(system).toContain('SMART EXPLORATION RULES:'); + expect(system).toContain('CONVERSATION TYPE: GLOBAL'); + }); + + it('should be byte-identical to what the shared builder produces', () => { + // Not "looks similar": the viewer's whole claim is exactness, so it is + // asserted against the same call the routes make, with the same inputs. + const allFilteredTools = filterToolsForReadOnly(pageSpaceTools, false); + const allowedToolNames = Object.keys(allFilteredTools); + const nonCoreToolNames = buildNonCoreToolNamesPrompt( + allowedToolNames.filter((n) => !CORE_TOOL_NAMES.has(n)), + ); + + expect(dashboard()).toBe( + buildAgentSystemPrompt({ + surface: 'global', + readOnly: false, + codeExecutionEnabled: isCodeExecutionEnabled(), + allowedToolNames, + skillCatalog: buildBuiltinSkillCatalog(allowedToolNames), + activePlan: '', + pageTree: '', + conversationType: 'global', + conversationContextId: null, + includeAskUser: false, + drivePromptSection: '', + agentAwareness: '', + nonCoreToolNames, + }), + ); + }); + + it('should pass the available tool names through, not the include-everything sentinel', () => { + // The old copy called the section builders with NO arguments, which is the + // "no filtering context" sentinel. The sections vary with the tools on + // hand, so the preview showed a set of capabilities that belonged to no + // actual caller. Passing the real names changes the output — which is the + // observable difference, and the direction is beside the point. + const withRealNames = dashboard(); + const sentinel = buildAgentSystemPrompt({ + surface: 'global', + readOnly: false, + codeExecutionEnabled: isCodeExecutionEnabled(), + allowedToolNames: [], + skillCatalog: '', + activePlan: '', + pageTree: '', + conversationType: 'global', + conversationContextId: null, + includeAskUser: false, + drivePromptSection: '', + agentAwareness: '', + nonCoreToolNames: '', + }); + + expect(withRealNames).not.toBe(sentinel); + }); + + it('should say so when the preview is read-only', () => { + expect(dashboard(true)).toContain('READ-ONLY MODE:'); + expect(dashboard(false)).not.toContain('READ-ONLY MODE:'); + }); + + it('should preview the PAGE surface when the context is a page', () => { + // The two surfaces are different assistants, and the viewer takes a + // contextType precisely so an admin can see which one they are looking at. + const page = buildCompleteRequest({ + contextType: 'page', + locationContext: { + currentPage: { id: 'p1', title: 'Notes', type: 'DOCUMENT', path: '/Notes' }, + }, + }).request.system; + + expect(page).not.toContain('You are the Global Assistant for PageSpace'); + expect(page).toContain('# PAGESPACE AI'); + }); + + it('should still keep the volatile block off the system prompt', () => { + // Production appends timestamp/location/mention to the last user message so + // the system prefix stays cache-stable; the viewer mirrors that, and this + // change must not have quietly pulled them forward. + const { request } = buildCompleteRequest({ contextType: 'dashboard' }); + + expect(request.system).not.toMatch(/CURRENT (DATE|TIME)/i); + expect(request.messages[0].content).toMatch(/current date|current time/i); + }); +}); diff --git a/apps/web/src/lib/ai/core/complete-request-builder.ts b/apps/web/src/lib/ai/core/complete-request-builder.ts index 94cf6b162a..47dce1257b 100644 --- a/apps/web/src/lib/ai/core/complete-request-builder.ts +++ b/apps/web/src/lib/ai/core/complete-request-builder.ts @@ -5,13 +5,10 @@ * Used by the admin global-prompt viewer to show the exact context window. */ -import { buildSystemPrompt, buildNonCoreToolNamesPrompt, TOOL_DISCOVERY_PROMPT } from './system-prompt'; +import { buildNonCoreToolNamesPrompt, TOOL_DISCOVERY_PROMPT } from './system-prompt'; import { filterToolsForReadOnly, isWriteTool } from './tool-filtering'; import { CORE_TOOL_NAMES } from './stub-tools'; -import { - buildInlineInstructions, - buildGlobalAssistantInstructions, -} from './inline-instructions'; +import { buildBuiltinSkillCatalog } from './skill-catalog'; import { extractToolSchemas, type ToolDefinitionForExtraction, @@ -23,7 +20,7 @@ import { isCodeExecutionEnabled } from '@pagespace/lib/services/sandbox/can-run- import { pageSpaceTools } from './ai-tools'; import { buildTimestampSystemPrompt } from './timestamp-utils'; import { buildMentionSystemPrompt } from './mention-processor'; -import { buildVolatileTurnContext } from './prompt-assembly'; +import { buildAgentSystemPrompt, buildVolatileTurnContext } from './prompt-assembly'; import { buildLocationTurnPrompt } from './location-prompt'; export interface LocationContext { @@ -115,15 +112,6 @@ export function buildCompleteRequest( includeExampleMessage = true, } = config; - // Build the base system prompt. Location is turn-volatile — it's built - // separately below as `locationPrompt` and injected via - // buildVolatileTurnContext, mirroring production routes exactly. - const baseSystemPrompt = buildSystemPrompt( - isReadOnly, - undefined, - isCodeExecutionEnabled() - ); - const locationPrompt = buildLocationTurnPrompt( locationContext ? { @@ -134,12 +122,6 @@ export function buildCompleteRequest( : undefined ); - // Build inline instructions based on context type - const inlineInstructions = - contextType === 'page' && locationContext?.currentPage - ? buildInlineInstructions() - : buildGlobalAssistantInstructions(); - // Apply read-only filtering (same logic as real Global Assistant) const allFilteredTools = filterToolsForReadOnly(pageSpaceTools, isReadOnly); @@ -148,18 +130,57 @@ export function buildCompleteRequest( Object.entries(allFilteredTools).filter(([name]) => CORE_TOOL_NAMES.has(name)) ); const nonCoreToolNames = Object.keys(allFilteredTools).filter(n => !CORE_TOOL_NAMES.has(n)); - - // Append non-core tool names to system prompt (matches real Global Assistant behavior) const nonCoreNamesSection = buildNonCoreToolNamesPrompt(nonCoreToolNames); - // Stable system prompt: base → TOOL_DISCOVERY → global instructions → nonCoreToolNames. - // Volatile sections (timestamp/mention/command) are omitted here — they are - // appended to the last user message at assembly time in production routes. + // Captured BEFORE the exposure split, like the production routes: the + // capability sections are gated on these names, and the post-split set holds + // only core tools plus the scaffolding. + const allowedToolNames = Object.keys(allFilteredTools); + + // THE WHOLE POINT OF THIS VIEWER IS THAT IT SHOWS THE REAL THING, so it calls + // the same assembly the routes call rather than describing it a second time. + // The hand-rolled version this replaces had already drifted: it omitted the + // Global Assistant's exploration guidance entirely, applied the capability + // sections with no tool gating at all, and put them in an order no route + // used — while the page it renders is titled "the exact context window". + // + // Blocks this viewer genuinely cannot know are empty, and are empty for a + // stated reason rather than by omission: it previews a surface with no + // conversation (so no plan pointer and no agent memory), no bound agent (so + // no custom prompt), and no drive membership loaded. const systemPrompt = - baseSystemPrompt + - '\n\n' + TOOL_DISCOVERY_PROMPT + - inlineInstructions + - (nonCoreNamesSection ? '\n\n' + nonCoreNamesSection : ''); + contextType === 'page' && locationContext?.currentPage + ? buildAgentSystemPrompt({ + surface: 'page', + readOnly: isReadOnly, + codeExecutionEnabled: isCodeExecutionEnabled(), + allowedToolNames, + skillCatalog: buildBuiltinSkillCatalog(allowedToolNames), + activePlan: '', + pageTree: '', + drivePromptPrefix: '', + memberDriveContextPrefix: '', + agentMemory: '', + toolDiscovery: + '\n\n' + TOOL_DISCOVERY_PROMPT + + (nonCoreNamesSection ? '\n\n' + nonCoreNamesSection : ''), + }) + : buildAgentSystemPrompt({ + surface: 'global', + readOnly: isReadOnly, + codeExecutionEnabled: isCodeExecutionEnabled(), + allowedToolNames, + skillCatalog: buildBuiltinSkillCatalog(allowedToolNames), + activePlan: '', + pageTree: '', + conversationType: 'global', + conversationContextId: null, + // The viewer has no authenticated principal to ask. + includeAskUser: false, + drivePromptSection: '', + agentAwareness: '', + nonCoreToolNames: nonCoreNamesSection, + }); // Build core tool schemas const coreToolsForExtraction: Record = {}; diff --git a/apps/web/src/lib/ai/core/prompt-assembly.ts b/apps/web/src/lib/ai/core/prompt-assembly.ts index e0846f1b84..5009ed8e71 100644 --- a/apps/web/src/lib/ai/core/prompt-assembly.ts +++ b/apps/web/src/lib/ai/core/prompt-assembly.ts @@ -1,16 +1,253 @@ /** * Prompt assembly helpers for prefix-stable, cache-friendly AI requests. * + * Two halves of one story: + * - `buildAgentSystemPrompt` owns the STABLE system prefix — which blocks an + * agent is given and in what order. + * - `buildVolatileTurnContext` owns the per-turn half, which deliberately does + * NOT go in the system prompt. + * * Key invariants enforced here: * - Volatile per-turn data (timestamp, location, mention, command) lives on * the last user message, NOT in the system prompt, so the system prefix * stays byte-identical across turns and provider prefix caches survive. * - Cache breakpoints are placed at message-level via providerOptions so * OpenRouter's Anthropic prefix cache can be activated per turn/step. - * - Nothing in this module reads the clock or mutates its inputs. + * - Nothing in this module reads the clock, performs I/O, or mutates its + * inputs. Every block is fetched by the caller and handed in as a string. + * The one environment read is the code-execution kill switch, and only as a + * default a caller may state instead — evaluated per call, never at import. */ import type { ModelMessage } from 'ai'; +import { isCodeExecutionEnabled } from '@pagespace/lib/services/sandbox/can-run-code'; +import { + buildSystemPrompt, + buildPersonalizationPrompt, + READ_ONLY_CONSTRAINT, + TOOL_DISCOVERY_PROMPT, + type PersonalizationInfo, +} from './system-prompt'; +import { + ASK_USER_SECTION, + buildInlineInstructions, + buildGlobalAssistantInstructions, +} from './inline-instructions'; + +// ─── The stable system prefix ───────────────────────────────────────────────── + +/** Blocks every surface takes, already fetched by the caller. '' means omitted. */ +interface SharedAgentPromptInput { + readonly readOnly: boolean; + /** + * Whether the sandbox instructions apply. Defaults to the global + * kill switch, which is what every caller was passing — it is one + * environment-wide switch, not a per-surface decision. Stated explicitly by + * tests so they stay deterministic. Evaluated per call, never at import, so + * no caller freezes the environment as it looked when the module loaded. + */ + readonly codeExecutionEnabled?: boolean; + /** + * `null` and `undefined` both mean "none". Both callers load this from a + * repository that returns `null`, so the coercion happens once here rather + * than as a `?? undefined` at every call site. + */ + readonly personalization?: PersonalizationInfo | null; + /** + * The tool names the model can actually reach. Gates the workspace-knowledge + * sections, so a capability the agent's owner switched off is never described + * to it — a named tool it does not have is an invitation to fail a turn. + */ + readonly allowedToolNames: string[]; + /** `buildBuiltinSkillCatalog(allowedToolNames)`. */ + readonly skillCatalog: string; + /** `buildActivePlanPrompt(...)`. */ + readonly activePlan: string; + /** The workspace-structure block, when the agent is configured to carry one. */ + readonly pageTree: string; +} + +interface PageAgentPromptInput extends SharedAgentPromptInput { + readonly surface: 'page'; + /** + * The agent owner's own prompt. Present ⇒ a BLANK SLATE: the default persona + * and the workspace-knowledge block are both skipped, because an owner who + * wrote their own prompt opted out of ours. The skill catalog and plan + * pointer are still appended — those are capability metadata, not persona, + * and `load_skill` ships with nothing behind it otherwise. + */ + readonly customSystemPrompt?: string | null; + /** Drive-owner instructions, prepended to a custom prompt only. */ + readonly drivePromptPrefix: string; + /** Cross-drive membership context. Applies to both branches. */ + readonly memberDriveContextPrefix: string; + /** The agent's own memory page. */ + readonly agentMemory: string; + /** `TOOL_DISCOVERY_PROMPT` + the deferred tool catalog, as one block. */ + readonly toolDiscovery: string; +} + +interface GlobalAssistantPromptInput extends SharedAgentPromptInput { + readonly surface: 'global'; + /** The conversation's own type/context, reported back to the model. */ + readonly conversationType: string; + readonly conversationContextId: string | null; + /** Whether this caller's auth permits the `ask_user` tool. */ + readonly includeAskUser: boolean; + /** Drive-owner instructions for the drive the caller is in. */ + readonly drivePromptSection: string; + /** The agents this user can consult. */ + readonly agentAwareness: string; + /** + * Only the deferred tool CATALOG. This surface states + * `TOOL_DISCOVERY_PROMPT` up front, near the exploration rules that depend on + * it, so the catalog arrives on its own later. + */ + readonly nonCoreToolNames: string; +} + +/** + * The two assistants the product has — not two settings of one. A page agent + * may carry its owner's own prompt and its own memory; the Global Assistant + * carries exploration guidance and an awareness of the agents it can consult. + * They are kept side by side in one function so the next person to change one + * can see the other, which is the whole reason this exists: the Global + * Assistant previously held a bespoke copy of the workspace rules that drifted + * until it described a page type the product does not create. + * + * Voice is not a third surface. A call binds to a page agent or to the Global + * Assistant, so it asks for whichever of the two it is bound to and appends its + * own spoken-medium override. + */ +export type AgentSystemPromptInput = PageAgentPromptInput | GlobalAssistantPromptInput; + +/** + * Guidance that belongs only to the Global Assistant, because only it is + * reached from the dashboard with no page to anchor on and has to decide for + * itself which drive "here" means. + */ +const globalAssistantGuidance = ( + conversationType: string, + conversationContextId: string | null, +): string => ` + +You are the Global Assistant for PageSpace - accessible from both the dashboard and sidebar. + +SMART EXPLORATION RULES: +1. When in a drive context (see your current LOCATION context for the driveId) - ALWAYS explore it first: + - ALWAYS use list_pages on the current drive when: + • User asks about the drive, its contents, or what's available + • User wants to create, write, or modify ANYTHING + • User mentions something that MAY exist in the drive + • User asks general questions about content or organization + • You need to understand the workspace structure + - Start with list_pages on the current drive BEFORE other actions +2. Context-first approach: + - Default scope: current drive/location (see LOCATION context) is your primary workspace + - Only explore OTHER drives when explicitly mentioned + - When user says "here" or "this", they mean current context + - If LOCATION context shows no drive, you're in the dashboard — use list_drives when you need to work across multiple workspaces; always check existing drives before suggesting new drive creation — never fall back to the Home drive as a guess +3. Efficient exploration pattern: + - FIRST: list_pages on the current drive — omit driveId and it uses the drive in your LOCATION context + - THEN: read specific pages as needed + - ONLY IF NEEDED: explore other drives/workspaces +4. Proactive assistance: + - Don't ask "what's in your drive" - use list_pages to discover + - Suggest creating AI_CHAT and CHANNEL pages for organization + - Be autonomous within current context + +CONVERSATION TYPE: ${conversationType.toUpperCase()}${conversationContextId ? ` (Context: ${conversationContextId})` : ''}`; + +/** + * Assemble an agent's stable system prompt: which blocks, in which order. + * + * THE ORDER IS THE POINT. Every block here is already a tested builder; what + * used to live in no single place was the sequence they go in and the branch + * that skips half of them for an agent carrying its own prompt. That sequence + * was written out inline in three files, and the copies drifted. + * + * NOTHING TURN-VOLATILE MAY BE ADDED HERE. Timestamp, location, mention and + * command blocks ride the last user message (`buildVolatileTurnContext` below) + * so this string stays byte-identical across turns and provider prefix caches + * survive. A block that changes when the user walks to another page belongs + * there, not here. + * + * Pure: every section is handed in already fetched. + */ +export function buildAgentSystemPrompt(input: AgentSystemPromptInput): string { + const codeExecutionEnabled = input.codeExecutionEnabled ?? isCodeExecutionEnabled(); + + if (input.surface === 'global') { + const base = buildSystemPrompt( + input.readOnly, + input.personalization ?? undefined, + codeExecutionEnabled, + ); + + // TOOL_DISCOVERY_PROMPT explains how to reach the tools that were deferred, + // by name — so with nothing deferred it explains how to reach nothing, using + // two tools (`tool_search`, `execute_tool`) that are not registered in that + // case. Gated on the catalog it introduces, which is the same condition + // `applyToolExposureMode` uses to decide whether to register them at all. + // The text routes always defer something, so this changes nothing there. + const toolDiscovery = input.nonCoreToolNames ? '\n\n' + TOOL_DISCOVERY_PROMPT : ''; + + const persona = + base + + toolDiscovery + + globalAssistantGuidance(input.conversationType, input.conversationContextId) + + (input.includeAskUser ? `\n\n${ASK_USER_SECTION}` : '') + + input.drivePromptSection; + + return ( + persona + + '\n' + + buildGlobalAssistantInstructions(input.allowedToolNames) + + (input.agentAwareness ? '\n\n' + input.agentAwareness : '') + + input.pageTree + + (input.nonCoreToolNames ? '\n\n' + input.nonCoreToolNames : '') + + input.skillCatalog + + input.activePlan + ); + } + + let systemPrompt: string; + // Blank is not a prompt. An owner who cleared the field has no instructions, + // not empty ones — and because a custom prompt suppresses the default persona + // and the workspace knowledge, a stray space used to buy an agent a blank + // slate it never asked for. The value itself is passed through untrimmed: + // only the emptiness test trims, so a real prompt still arrives verbatim. + const customSystemPrompt = input.customSystemPrompt?.trim() + ? input.customSystemPrompt + : undefined; + if (customSystemPrompt) { + systemPrompt = input.drivePromptPrefix + customSystemPrompt; + + const personalizationPrompt = buildPersonalizationPrompt( + input.personalization ?? undefined, + ); + if (personalizationPrompt) { + systemPrompt += `\n\n${personalizationPrompt}`; + } + if (input.readOnly) { + systemPrompt += `\n\n${READ_ONLY_CONSTRAINT}`; + } + } else { + systemPrompt = buildSystemPrompt( + input.readOnly, + input.personalization ?? undefined, + codeExecutionEnabled, + ); + systemPrompt += buildInlineInstructions(input.allowedToolNames); + } + + // Cross-drive membership applies uniformly, unlike drivePromptPrefix above. + systemPrompt = input.memberDriveContextPrefix + systemPrompt; + systemPrompt += input.skillCatalog; + systemPrompt += input.activePlan; + + return systemPrompt + input.pageTree + input.agentMemory + input.toolDiscovery; +} // ─── Types ──────────────────────────────────────────────────────────────────── diff --git a/apps/web/src/lib/ai/core/system-prompt.ts b/apps/web/src/lib/ai/core/system-prompt.ts index 6ee41d57e6..c3d39e36b2 100644 --- a/apps/web/src/lib/ai/core/system-prompt.ts +++ b/apps/web/src/lib/ai/core/system-prompt.ts @@ -73,7 +73,13 @@ export function buildNonCoreToolNamesPrompt(toolNames: string[]): string { return `NON-CORE TOOLS (use execute_tool to call; use tool_search("select:tool_name") for parameter schemas):\n${lines}`; } -const READ_ONLY_CONSTRAINT = `READ-ONLY MODE: +/** + * Exported because a custom-systemPrompt agent opts out of `buildSystemPrompt` + * entirely and still has to be told it is read-only. `prompt-assembly.ts` + * appends this one; before it was exported the page route carried a hand-typed + * copy of the same four lines. + */ +export const READ_ONLY_CONSTRAINT = `READ-ONLY MODE: • You cannot modify, create, or delete any content • Focus on exploring, analyzing, and planning • Create actionable plans for the user to execute later`; diff --git a/apps/web/src/lib/ai/realtime/__tests__/binding-loader.test.ts b/apps/web/src/lib/ai/realtime/__tests__/binding-loader.test.ts index e5b66cefd3..5a0507e44e 100644 --- a/apps/web/src/lib/ai/realtime/__tests__/binding-loader.test.ts +++ b/apps/web/src/lib/ai/realtime/__tests__/binding-loader.test.ts @@ -36,6 +36,16 @@ const agentPage = (over: Partial = {}): AgentPage => ({ ...over, }); +/** + * Stands in for whatever `buildVoiceSystemContext` assembled. This module's job + * is to decide WHAT the call is bound to and hand that over; assembling the + * instructions is `system-context.ts`, tested there. + */ +const CALL_CONTEXT = { + instructions: '# PAGESPACE AI\n\n<>\n\n# THIS IS A VOICE CALL', + tools: [{ type: 'function' as const, name: 'read_page', description: '', parameters: {} }], +}; + const history = [ { role: 'user', content: 'where are my notes?', createdAt: new Date(1) }, { role: 'assistant', content: 'In your Inbox.', createdAt: new Date(2) }, @@ -48,6 +58,7 @@ function deps(over: Partial = {}) { canAccess: vi.fn(async () => true), loadMessages: vi.fn(async () => history), loadAgentPage: vi.fn(async () => agentPage()), + buildCallContext: vi.fn(async () => CALL_CONTEXT), logger: { warn }, ...over, }; @@ -228,66 +239,75 @@ describe('loadVoiceBinding — the assistant', () => { }); describe('loadVoiceBinding — the instructions', () => { - it('given any call, should say it is a spoken conversation', async () => { + it('given any call, should carry the assembled system prompt and the spoken override', async () => { const { deps: d } = deps(); const { instructions } = await loadVoiceBinding(d, { userId: 'u1', conversationId: 'conv1' }); - expect(instructions).toContain('heard, not read'); + expect(instructions).toContain('<>'); + expect(instructions).toContain('# THIS IS A VOICE CALL'); }); - it("given a page agent with its own prompt, should carry that prompt VERBATIM", async () => { - const systemPrompt = 'Answer only in limericks. Never mention the weather.'; + it('given a page agent, should describe it to the assembler as an agent', async () => { + // What the prompt says about tools, memory and persona all hangs off this: + // handing the assembler no agent would silently build the Global + // Assistant's prompt for a call the UI says is talking to a named one. + const systemPrompt = 'Answer only in limericks.'; const { deps: d } = deps({ loadConversation: vi.fn(async () => pageConversation()), - loadAgentPage: vi.fn(async () => agentPage({ systemPrompt })), + loadAgentPage: vi.fn(async () => agentPage({ systemPrompt, enabledTools: ['read_page'] })), }); - const { instructions } = await loadVoiceBinding(d, { userId: 'u1', conversationId: 'conv1' }); + await loadVoiceBinding(d, { userId: 'u1', conversationId: 'conv1' }); - expect(instructions).toContain(systemPrompt); - expect(instructions).toContain('Release Notes Bot'); + expect(d.buildCallContext).toHaveBeenCalledWith({ + userId: 'u1', + conversationId: 'conv1', + conversation: { type: 'page', contextId: 'agent1' }, + agent: { + pageId: 'agent1', + title: 'Release Notes Bot', + systemPrompt, + enabledTools: ['read_page'], + }, + }); }); - it('given a page agent with NO prompt, should still name it so it is the assistant the user picked', async () => { - const { deps: d } = deps({ - loadConversation: vi.fn(async () => pageConversation()), - loadAgentPage: vi.fn(async () => agentPage({ systemPrompt: null })), - }); + it('given a conversation with no agent, should pass its coordinates but no agent', async () => { + const { deps: d } = deps(); - const { instructions } = await loadVoiceBinding(d, { userId: 'u1', conversationId: 'conv1' }); + await loadVoiceBinding(d, { userId: 'u1', conversationId: 'conv1' }); - expect(instructions).toContain('Release Notes Bot'); + expect(d.buildCallContext).toHaveBeenCalledWith({ + userId: 'u1', + conversationId: 'conv1', + conversation: { type: 'global', contextId: null }, + }); }); - it('given a whitespace-only prompt, should treat it as no prompt rather than send blanks', async () => { - const { deps: d } = deps({ - loadConversation: vi.fn(async () => pageConversation()), - loadAgentPage: vi.fn(async () => agentPage({ systemPrompt: ' \n ' })), - }); + it('given an unbound call, should still assemble a prompt for it', async () => { + // An unbound call talks to the Global Assistant — the same one the + // dashboard talks to — so it is a real call, not a degraded one. + const { deps: d } = deps(); - const { instructions } = await loadVoiceBinding(d, { userId: 'u1', conversationId: 'conv1' }); + const { instructions } = await loadVoiceBinding(d, { userId: 'u1' }); - expect(instructions).toContain('PageSpace'); - expect(instructions).not.toMatch(/instructions follow/); + expect(d.buildCallContext).toHaveBeenCalledWith({ userId: 'u1' }); + expect(instructions).toContain('<>'); + expect(instructions).toContain('# THIS IS A VOICE CALL'); }); -}); -describe('loadVoiceBinding — a failed agent read', () => { - it('should cost the identity but KEEP the history', async () => { - // The seed was fetched successfully alongside it; throwing it away because - // a different read failed helps nobody. - const { deps: d, warn } = deps({ - loadConversation: vi.fn(async () => pageConversation()), - loadAgentPage: vi.fn(async () => { - throw new Error('db unreachable'); - }), - }); + it('given a conversation the caller cannot read, should assemble as if unbound', async () => { + // No history, and nothing derived from a thread this caller may not see. + const { deps: d } = deps({ canAccess: vi.fn(async () => false) }); - const binding = await loadVoiceBinding(d, { userId: 'u1', conversationId: 'conv1' }); + const { instructions, seed } = await loadVoiceBinding(d, { + userId: 'u1', + conversationId: 'conv1', + }); - expect(binding.seed).toHaveLength(2); - expect(binding.assistant).toBeUndefined(); - expect(warn).toHaveBeenCalled(); + expect(seed).toEqual([]); + expect(d.buildCallContext).toHaveBeenCalledWith({ userId: 'u1' }); + expect(instructions).toContain('# THIS IS A VOICE CALL'); }); }); diff --git a/apps/web/src/lib/ai/realtime/__tests__/instructions.test.ts b/apps/web/src/lib/ai/realtime/__tests__/instructions.test.ts index 14020c3a7c..1b1ee383df 100644 --- a/apps/web/src/lib/ai/realtime/__tests__/instructions.test.ts +++ b/apps/web/src/lib/ai/realtime/__tests__/instructions.test.ts @@ -1,92 +1,212 @@ /** * Instruction tests. * - * This string is the only thing that tells the model which assistant it is, so - * the cases that matter are the ones about an agent owner's own words: they - * must arrive intact, and nothing bolted on around them may contradict them. + * Two things matter here, and they used to be one. The call now carries the + * real agent system prompt — which is what made it possible to delegate to at + * all — and this module adds only what is different because the words are + * heard. So the cases are: the shared prompt arrives intact, and the spoken + * override sits after it and says what it overrides. */ import { describe, expect, it } from 'vitest'; import { buildVoiceInstructions } from '../instructions'; +/** Stands in for whatever `buildAgentSystemPrompt` produced for this binding. */ +const AGENT_PROMPT = '# PAGESPACE AI\n\nYou are PageSpace AI. <>'; + +/** An ordinary agent: the split deferred something, so discovery exists. */ +const FULL_REACH = { + exposed: ['read_page', 'tool_search', 'execute_tool'], + reachable: ['read_page', 'create_task', 'spawn_session'], +}; + +/** + * An agent allowed only core tools. `applyToolExposureMode` registers no + * scaffolding when there is nothing to defer, so `tool_search` and + * `execute_tool` genuinely are not there. + */ +const CORE_ONLY_REACH = { exposed: ['read_page'], reachable: ['read_page'] }; + +const build = ( + over: { + agentSystemPrompt?: string; + title?: string; + tools?: { exposed: string[]; reachable: string[] }; + } = {}, +) => buildVoiceInstructions({ agentSystemPrompt: AGENT_PROMPT, tools: FULL_REACH, ...over }); + describe('buildVoiceInstructions', () => { - it('given no assistant, should still say this is a spoken conversation', () => { - // The Global Assistant and an unbound call both land here, and the medium - // shapes a good answer more than the persona does. - const instructions = buildVoiceInstructions({}); + it('should carry the shared agent prompt VERBATIM', () => { + // The call is the same agent as the typed surface. Summarising or trimming + // the prompt here would make it a different one that merely sounds similar. + expect(build()).toContain(AGENT_PROMPT); + }); - expect(instructions).toContain('heard, not read'); - expect(instructions).toContain('PageSpace'); + it('should put the shared prompt FIRST and the spoken override after it', () => { + // The override names rules it reverses, so it has to come after the thing + // stating them — a contradiction resolved in the wrong order is just a + // contradiction, and gpt-realtime degrades on those specifically. + const instructions = build(); + + expect(instructions.indexOf('<>')).toBeLessThan( + instructions.indexOf('# THIS IS A VOICE CALL'), + ); }); - it('should tell the model the things a spoken turn needs and a typed one does not', () => { - const instructions = buildVoiceInstructions({}); + it('should say plainly that it overrides what came before', () => { + expect(build()).toMatch(/OVERRIDE/); + expect(build()).toContain('Everything above still applies EXCEPT'); + }); - expect(instructions).toMatch(/short/i); + it('should reverse "skip preambles" by name rather than just contradicting it', () => { + // The typed prompt says to skip preambles; a call needs a short line before + // a slow tool or the caller hears silence and assumes it dropped. Left + // unnamed, this is two rules arguing. + expect(build()).toContain('"Skip preambles" does NOT apply here'); + }); + + it('should tell the model this is spoken, and what a spoken turn costs', () => { + const instructions = build(); + + expect(instructions).toContain('heard, not read'); + expect(instructions).toMatch(/two or three sentences/i); expect(instructions).toMatch(/markdown/i); expect(instructions).toMatch(/interrupt/i); }); - it("given an agent with its own prompt, should carry it VERBATIM", () => { - // Its owner wrote exactly this, and the text surface sends it unmodified. - // Wrapping or summarising it here would be a second interpretation of a - // field that already has one. - const systemPrompt = 'Answer only in limericks.\n\nNever mention the weather.'; + it('should tell it to ACT rather than ask permission', () => { + // The model's default posture is to confirm before calling a tool, which on + // a call reads as an assistant that will not do anything. + expect(build()).toContain('DO NOT ASK PERMISSION TO USE A TOOL'); + }); + + it('should tell it not to give up after one empty result, and when to stop', () => { + const instructions = build(); - const instructions = buildVoiceInstructions({ title: 'Bard', systemPrompt }); + expect(instructions).toContain('An empty search is not an answer'); + expect(instructions).toContain('IF THE SAME TOOL FAILS TWICE ON THE SAME TASK'); + }); - expect(instructions).toContain(systemPrompt); + it('should send it to tool_search before it claims something is impossible', () => { + // Most tools are deferred. Without this the model refuses requests it is + // holding the tools for. + expect(build()).toContain('NEVER SAY SOMETHING IS IMPOSSIBLE BEFORE YOU HAVE CALLED tool_search'); }); - it('should put the medium FIRST and the assistant after it', () => { - // A prompt that establishes a persona must not be preceded by nothing and - // followed by generic guidance that argues with it. - const instructions = buildVoiceInstructions({ - title: 'Bard', - systemPrompt: 'Answer only in limericks.', - }); + it('given NO discovery tools, should not send it to a tool_search that is not there', () => { + // An agent allowed only core tools is registered no scaffolding at all, so + // this rule would order a call to a tool the session never advertised — + // spent out loud, in front of someone waiting. + const instructions = build({ tools: CORE_ONLY_REACH }); - expect(instructions.indexOf('heard, not read')).toBeLessThan( - instructions.indexOf('Answer only in limericks.'), - ); + expect(instructions).not.toContain('tool_search'); + expect(instructions).not.toContain('execute_tool'); + expect(instructions).toContain('Every tool you have is already listed for you'); }); - it("given an agent with a prompt, should NAME it alongside rather than fold it in", () => { - const instructions = buildVoiceInstructions({ - title: 'Release Notes Bot', - systemPrompt: 'Answer only in limericks.', + it('should name where long work goes, so a call is not four minutes of silence', () => { + const instructions = build(); + + expect(instructions).toContain('spawn_session'); + expect(instructions).toContain('create_task'); + }); + + it('given only one hand-off tool, should name that one and not the others', () => { + const instructions = build({ + tools: { exposed: ['read_page', 'tool_search'], reachable: ['read_page', 'create_task'] }, }); - expect(instructions).toContain('"Release Notes Bot"'); - expect(instructions).toContain('instructions follow'); + expect(instructions).toContain('create_task'); + expect(instructions).not.toContain('spawn_session'); + expect(instructions).not.toContain('a trigger or a workflow'); + }); + + it('should offer deferred work when EITHER a trigger or a workflow is reachable', () => { + // One phrase covers two tools because it names a destination, not a call. + // An agent that can set a trigger but not build a workflow can still be told + // that work may happen later. + for (const name of ['set_task_trigger', 'create_workflow']) { + const instructions = build({ + tools: { exposed: ['read_page', 'tool_search'], reachable: ['read_page', name] }, + }); + + expect(instructions, name).toContain('a trigger or a workflow for work that should happen later'); + } + }); + + it('given NO way to hand work off, should say to work through it instead of naming absent tools', () => { + const instructions = build({ tools: CORE_ONLY_REACH }); + + expect(instructions).not.toContain('spawn_session'); + expect(instructions).not.toContain('create_task'); + expect(instructions).toContain('work through it in steps'); + }); + + it('should say the page id can be omitted, rather than asking the caller for one', () => { + // The caller cannot read an id aloud and does not have one. The tools + // resolve "this page" from the live location when the id is absent. + const instructions = build(); + + expect(instructions).toContain('with NO page id'); + expect(instructions).toContain('Never ask the caller for an id'); + }); + + it('should send ask_user out loud instead of drawing a card nobody can see', () => { + expect(build()).toContain('ask_user draws a card on a screen and does not work here'); + }); + + it('should ask for clarification on unintelligible audio rather than guessing', () => { + const instructions = build(); + + expect(instructions).toContain('unintelligible'); + expect(instructions).toContain('Do not guess at what was said'); }); - it('given an agent with NO prompt, should still name it', () => { - // Being called by the name the user picked is most of what makes it feel - // like the assistant they picked. - const instructions = buildVoiceInstructions({ title: 'Release Notes Bot' }); + it('should tell it to vary its phrasing', () => { + expect(build()).toContain('Never reuse the same opener'); + }); - expect(instructions).toContain('"Release Notes Bot"'); - expect(instructions).not.toContain('instructions follow'); + it('given a bound agent, should address it by the name the user picked', () => { + // The typed surface never names the agent because the user can see which + // one they opened. On a call, being called by that name is most of what + // makes it feel like the assistant they chose. + expect(build({ title: 'Release Notes Bot' })).toContain('"Release Notes Bot"'); }); - it('given a blank or whitespace-only prompt, should treat it as no prompt', () => { - // An owner who cleared the field has no instructions, not empty ones. - const blank = buildVoiceInstructions({ title: 'Bot', systemPrompt: ' \n\t ' }); + it('given no bound agent, should still say it is speaking out loud', () => { + const instructions = build(); + + expect(instructions).toContain('speaking with someone out loud'); + expect(instructions).not.toContain('""'); + }); - expect(blank).not.toContain('instructions follow'); - expect(blank).toBe(buildVoiceInstructions({ title: 'Bot' })); + it('should say its own tool results outrank the standing instructions it was given', () => { + // The instructions are assembled at socket open and never re-sent, so a + // block describing mutable state goes stale the moment the model mutates + // it. The ACTIVE PLAN pointer is the sharp case: it is a directive ("keep + // working against this page, re-read it before continuing"), so after a + // clear_plan on the call the model would otherwise go on being told to + // resume a plan the caller just ended. Its own tool result is the one + // channel that CAN correct the record mid-call, so it is named as + // authoritative. + const instructions = build(); + + expect(instructions).toContain('YOUR OWN TOOL RESULT IS WHAT IS CURRENT'); + expect(instructions).toContain('Never re-follow a standing instruction about something you have since changed'); }); - it('given a prompt but no title, should still deliver the prompt', () => { - const instructions = buildVoiceInstructions({ systemPrompt: 'Be terse.' }); + it('should carry NOTHING turn-volatile — instructions are sent once, at socket open', () => { + // There is no path that sends a second session.update, so a location baked + // in here becomes a lie the moment the caller walks to another page. The + // tools read the live location instead. + const instructions = build(); - expect(instructions).toContain('Be terse.'); - expect(instructions).toContain('an assistant in PageSpace'); + expect(instructions).not.toContain('LOCATION CONTEXT'); + expect(instructions).not.toMatch(/CURRENT (DATE|TIME)/i); }); it('should be pure — same input, same output, nothing accumulated between calls', () => { - const input = { title: 'Bot', systemPrompt: 'Be terse.' }; + const input = { agentSystemPrompt: AGENT_PROMPT, title: 'Bot' }; expect(buildVoiceInstructions(input)).toBe(buildVoiceInstructions(input)); }); @@ -94,8 +214,8 @@ describe('buildVoiceInstructions', () => { it('should never come back empty', () => { // An empty `instructions` on a session.update REPLACES the session's, so a // builder that can return '' is a builder that can erase a persona. - for (const input of [{}, { title: 'Bot' }, { systemPrompt: 'x' }, { title: '', systemPrompt: '' }]) { - expect(buildVoiceInstructions(input).trim().length).toBeGreaterThan(0); + for (const input of [{}, { title: 'Bot' }, { agentSystemPrompt: '' }]) { + expect(build(input).trim().length).toBeGreaterThan(0); } }); }); diff --git a/apps/web/src/lib/ai/realtime/__tests__/system-context.test.ts b/apps/web/src/lib/ai/realtime/__tests__/system-context.test.ts new file mode 100644 index 0000000000..a193cd0b2c --- /dev/null +++ b/apps/web/src/lib/ai/realtime/__tests__/system-context.test.ts @@ -0,0 +1,448 @@ +/** + * What a call actually knows. + * + * The bug this module exists to fix was invisible: `tool_search` and + * `execute_tool` rode every session while nothing in the prompt named them, so + * every deferred tool — the calendar family, `spawn_session`, `create_task`, + * the workflow tools — was loaded and undiscoverable, and the model answered + * "I can't do that" about tools it was holding. So the cases that matter most + * are the ones about the discovery block and the tool names in it. + * + * The rest are about not letting one failed read cost the call. + */ + +import { describe, expect, it, vi } from 'vitest'; +import type { ToolSet } from 'ai'; +import { z } from 'zod'; +import type { Tool } from 'ai'; +import { + buildVoiceCallContext, + type VoiceSystemContextDeps, + type BoundAgent, +} from '../system-context'; + +/** The assembled prompt alone — most cases here are about what it says. */ +const promptOf = async ( + deps: VoiceSystemContextDeps, + request: Parameters[1], +): Promise => (await buildVoiceCallContext(deps, request)).instructions; +import { buildPageSpaceTools } from '../../core/ai-tools'; +import { buildRealtimeToolExposure } from '../tools'; + +const fakeTool = (description = 'A tool.'): Tool => + ({ + description, + inputSchema: z.object({ pageId: z.string() }), + execute: async () => ({}), + }) as Tool; + +/** One core tool and two deferred ones, so there is something to discover. */ +const smallRegistry = (): ToolSet => ({ + read_page: fakeTool('Read a page.'), + create_task: fakeTool('Create a task.'), + spawn_session: fakeTool('Spawn a worker session.'), +}); + +const agent = (over: Partial = {}): BoundAgent => ({ + pageId: 'agent1', + title: 'Release Notes Bot', + systemPrompt: null, + enabledTools: null, + ...over, +}); + +function deps(over: Partial = {}) { + const warn = vi.fn(); + const base: VoiceSystemContextDeps = { + buildTools: () => smallRegistry(), + loadAgentMemory: vi.fn(async () => '\n\n<>'), + loadActivePlan: vi.fn(async () => '\n\n<>'), + loadPersonalization: vi.fn(async () => ({ enabled: true, bio: 'Ships release notes.' })), + logger: { warn }, + ...over, + }; + return { deps: base, warn }; +} + +/** + * The catalog block, sliced out of the prompt. + * + * Asserted against the SECTION rather than the whole string: several of these + * tool names also appear in the workspace-knowledge prose, so a whole-string + * `toContain('spawn_session')` passes with the catalog entirely absent — which + * is precisely the bug, and a mutation check caught these two tests passing + * that way. + */ +const catalog = (prompt: string): string => { + const at = prompt.indexOf('NON-CORE TOOLS'); + return at === -1 ? '' : prompt.slice(at); +}; + +describe('buildVoiceSystemContext — reaching the tools it is holding', () => { + it('should tell the model how to reach the tools that are not advertised', async () => { + const { deps: d } = deps(); + + const prompt = await promptOf(d, { userId: 'u1' }); + + expect(prompt).toContain('call execute_tool'); + expect(prompt).toContain('tool_search("select:tool_name")'); + }); + + it('should NAME the deferred tools, which is the whole delegation surface', async () => { + const { deps: d } = deps(); + + const listed = catalog(await promptOf(d, { userId: 'u1' })); + + expect(listed).toContain('create_task'); + expect(listed).toContain('spawn_session'); + }); + + it('given the real registry, should name tools a call could never reach before', async () => { + // Against the actual product registry, not a fixture: the failure this + // guards is "the model refuses a request it has the tool for", and the + // tools in question are real ones. + const { deps: d } = deps({ buildTools: () => buildPageSpaceTools() }); + + const listed = catalog(await promptOf(d, { userId: 'u1' })); + + for (const name of ['create_task', 'spawn_session', 'list_calendar_events', 'create_workflow']) { + expect(listed, `${name} is not discoverable on a call`).toContain(name); + } + }); + + it("should describe only the tools the agent's owner left switched on", async () => { + // A name in the prompt is an invitation to call it. Naming a blocked tool + // both leaks the agent's configuration and spends a turn on a refusal. + const { deps: d } = deps(); + + const listed = catalog( + await promptOf(d, { + userId: 'u1', + agent: agent({ enabledTools: ['read_page', 'create_task'] }), + }), + ); + + expect(listed).toContain('create_task'); + expect(listed).not.toContain('spawn_session'); + }); + + it('should carry the workspace knowledge, gated on those same tools', async () => { + const { deps: d } = deps(); + + const prompt = await promptOf(d, { userId: 'u1' }); + + expect(prompt).toContain('# PAGESPACE AI'); + expect(prompt).toMatch(/TASK/i); + }); + + it('should explain the DEFERRED capabilities, not just list their names', async () => { + // THE TRAP. After the split, the tool set holds the core tools and the two + // scaffolding tools and nothing else. Gate the capability sections on ITS + // keys and the catalog goes on advertising create_task and spawn_session as + // callable while every word of guidance about tasks, delegation and + // automation is silently dropped — the model is told the tools exist and + // nothing about when to use them. `page-chat-turn.ts:1306` captures the + // pre-split list at the same point for the same reason. + // + // Asserted on the PAGE branch specifically: it is `buildInlineInstructions` + // that gates these sections on the names, whereas the Global Assistant's + // builder states them unconditionally and would pass either way. + const { deps: d } = deps({ buildTools: () => buildPageSpaceTools() }); + + const prompt = await promptOf(d, { userId: 'u1', agent: agent() }); + const guidance = prompt.slice(0, prompt.indexOf('NON-CORE TOOLS')); + + expect(guidance).toContain('TASK MANAGEMENT:'); + expect(guidance).toContain('AGENTS:'); + expect(guidance).toContain('AUTOMATION:'); + }); + + it('should offer the skills whose tools are DEFERRED, not only the core-tool ones', async () => { + // `task-management` needs create_task/update_task and `spreadsheets` needs + // edit_sheet_cells — all deferred. Gated on the post-split names they + // vanish, while `writing-documents` (core tools only) survives and keeps the + // SKILLS section looking populated, which is what makes the loss easy to + // miss. + const { deps: d } = deps({ buildTools: () => buildPageSpaceTools() }); + + const prompt = await promptOf(d, { userId: 'u1', agent: agent() }); + + expect(prompt).toContain('task-management'); + expect(prompt).toContain('spreadsheets'); + }); + + it('should advertise exactly the tools its own prompt describes', async () => { + // The two used to be built in different places from the same inputs, which + // is the arrangement where they agree until someone edits one call site. + // Now they are two projections of one exposure, and this is the property + // that buys: every advertised name is either explained upfront or named in + // the deferred catalog, and nothing is advertised that the prompt is silent + // about. + const { deps: d } = deps({ buildTools: () => buildPageSpaceTools() }); + + const { instructions, tools } = await buildVoiceCallContext(d, { + userId: 'u1', + agent: agent({ enabledTools: ['read_page', 'create_task', 'list_calendar_events'] }), + }); + + expect(tools.length).toBeGreaterThan(0); + for (const tool of tools) { + expect(instructions, `${tool.name} is advertised but never mentioned`).toContain(tool.name); + } + }); + + it('given a core-only agent, should advertise no scaffolding AND promise none', async () => { + const { deps: d } = deps({ buildTools: () => buildPageSpaceTools() }); + + const { instructions, tools } = await buildVoiceCallContext(d, { + userId: 'u1', + agent: agent({ enabledTools: ['read_page', 'list_pages'] }), + }); + + expect(tools.map((t) => t.name).sort()).toEqual(['list_pages', 'read_page']); + expect(instructions).not.toContain('tool_search'); + expect(instructions).not.toContain('execute_tool'); + }); + + it('should let tool_search resolve a skill the prompt advertises', async () => { + // The catalog says a skill is loadable and tool_search is where the model + // goes to find it. Advertising one the search cannot resolve sends it + // looking for something that, as far as the search is concerned, is not + // there — `searchableSkills` went unpassed and the corpus was tools-only. + const search = buildRealtimeToolExposure(buildPageSpaceTools()).tools.tool_search; + + const found = JSON.stringify( + await (search.execute as (a: unknown, o: unknown) => unknown)( + { query: 'task-management' }, + { experimental_context: {}, toolCallId: 't1', messages: [] }, + ), + ); + + expect(found).toContain('task-management'); + }); +}); + +describe('buildVoiceSystemContext — which assistant it assembles', () => { + it("given a bound agent with its own prompt, should carry it VERBATIM and skip our persona", async () => { + const systemPrompt = 'Answer only in limericks. Never mention the weather.'; + const { deps: d } = deps(); + + const prompt = await promptOf(d, { + userId: 'u1', + agent: agent({ systemPrompt }), + }); + + expect(prompt).toContain(systemPrompt); + expect(prompt).not.toContain('# PAGESPACE AI'); + }); + + it('given a bound agent, should address it by the name the user picked', async () => { + // The typed surface never names the agent because the user can see which + // one they opened. On a call, being called by that name is most of what + // makes it feel like the assistant they chose. + const { deps: d } = deps(); + + const prompt = await promptOf(d, { userId: 'u1', agent: agent() }); + + expect(prompt).toContain('"Release Notes Bot"'); + }); + + it('should cap the assembly with the spoken override, after the shared prompt', async () => { + const { deps: d } = deps(); + + const prompt = await promptOf(d, { userId: 'u1' }); + + expect(prompt).toContain('# THIS IS A VOICE CALL'); + expect(prompt.indexOf('You are the Global Assistant')).toBeLessThan( + prompt.indexOf('# THIS IS A VOICE CALL'), + ); + }); + + it('given an agent allowed only core tools, should not name discovery tools anywhere', async () => { + // Nothing is deferred, so no scaffolding is registered. Every mention of + // tool_search or execute_tool — in the catalog or in the spoken rules — + // would name a tool the session never advertised. + const { deps: d } = deps({ buildTools: () => buildPageSpaceTools() }); + + const prompt = await promptOf(d, { + userId: 'u1', + agent: agent({ enabledTools: ['read_page', 'list_pages'] }), + }); + + expect(prompt).not.toContain('tool_search'); + expect(prompt).not.toContain('execute_tool'); + }); + + it('given a bound agent, should carry its memory', async () => { + const { deps: d } = deps(); + + const prompt = await promptOf(d, { userId: 'u1', agent: agent() }); + + expect(prompt).toContain('<>'); + expect(d.loadAgentMemory).toHaveBeenCalledWith('agent1', 'u1'); + }); + + it('given NO bound agent, should assemble the Global Assistant', async () => { + const { deps: d } = deps(); + + const prompt = await promptOf(d, { userId: 'u1' }); + + expect(prompt).toContain('You are the Global Assistant for PageSpace'); + }); + + it('should NOT scan the deployment for agents before the caller can be heard', async () => { + // `buildAgentAwarenessPrompt` selects every non-trashed drive and then + // awaits a check per drive and per agent, serially — all of it before the + // SDP exchange, with the caller on a dead line. The guidance about agents + // stays; the list is fetched by `list_agents` on the turn that needs it. + const { deps: d } = deps({ buildTools: () => buildPageSpaceTools() }); + + const prompt = await promptOf(d, { userId: 'u1' }); + + expect(prompt).not.toContain('## Available AI Agents'); + expect(prompt).toContain('list_agents'); + }); + + it('should carry the caller\'s own personalization, same as the typed surface', async () => { + const { deps: d } = deps(); + + const prompt = await promptOf(d, { userId: 'u1' }); + + expect(prompt).toContain('ABOUT THE USER'); + expect(prompt).toContain('Ships release notes.'); + }); + + it('given a bound conversation, should carry its plan pointer', async () => { + const { deps: d } = deps(); + + const prompt = await promptOf(d, { userId: 'u1', conversationId: 'conv1' }); + + expect(prompt).toContain('<>'); + expect(d.loadActivePlan).toHaveBeenCalledWith('conv1', 'u1'); + }); + + it('given an UNBOUND call, should not go looking for a plan there is no thread for', async () => { + const { deps: d } = deps(); + + await promptOf(d, { userId: 'u1' }); + + expect(d.loadActivePlan).not.toHaveBeenCalled(); + }); + + it('should never describe ask_user, which draws a card nobody on a call can see', async () => { + const { deps: d } = deps(); + + expect(await promptOf(d, { userId: 'u1' })).not.toContain('ASKING THE USER:'); + }); + + it('should carry NOTHING turn-volatile — instructions are sent once, at socket open', async () => { + // The caller can walk to another page mid-call without the session + // rebinding, so a location or a clock frozen here goes quietly wrong. The + // tools read the live location instead. + const { deps: d } = deps(); + + const prompt = await promptOf(d, { userId: 'u1', agent: agent() }); + + expect(prompt).not.toContain('LOCATION CONTEXT'); + expect(prompt).not.toMatch(/CURRENT (DATE|TIME)/i); + }); +}); + +describe('buildVoiceSystemContext — what it costs the session', () => { + /** + * A call's context window is shared with the audio flowing through it for as + * long as the call lasts, and the seed already reserves 4k on top of this. At + * the time of writing the real registry assembles to roughly 9k characters + * (~2.3k tokens), which is comfortable; this ceiling is set well above that so + * it fails on a block that doubled rather than on ordinary drift. + * + * If it fails, the order to cut in is: page tree (already omitted) → + * personalization → agent memory → skill catalog. The tool catalog is the LAST + * thing to cut — it is what makes a call something you can delegate to. + */ + const CEILING_CHARS = 20_000; + + it('should assemble well inside the session context it shares with the audio', async () => { + const { deps: d } = deps({ buildTools: () => buildPageSpaceTools() }); + + const prompt = await promptOf(d, { userId: 'u1' }); + + expect(prompt.length, `assembled prompt is ${prompt.length} characters`).toBeLessThan( + CEILING_CHARS, + ); + }); + + it('should stay inside it in the WORST case, not just the empty one', async () => { + // The ceiling above is measured with nothing loaded, which is the case + // least likely to breach it. This is the real shape of a heavy call: a + // bound agent carrying a memory page at its own ~2k-token cap + // (`agent-memory.ts`), a plan pointer, and personalization the user wrote. + // + // The budget it has to fit inside: a 32k session, ~4k of which the seed + // reserves (`seed.ts`), and the rest shared with the audio for the whole + // length of the call. + const memory = 'Remembered: '.repeat(700); // ~8.4k chars, past the cap's own limit + const { deps: d } = deps({ + buildTools: () => buildPageSpaceTools(), + loadAgentMemory: async () => memory, + loadActivePlan: async () => + '\n\nACTIVE PLAN:\nThis conversation is working against the plan page "Q3 Migration".', + loadPersonalization: async () => ({ + enabled: true, + bio: 'Runs release engineering.', + writingStyle: 'Terse, no hedging.', + rules: 'Never guess at version numbers.', + }), + }); + + const prompt = await promptOf(d, { + userId: 'u1', + conversationId: 'conv1', + agent: agent(), + }); + + expect(prompt).toContain('Remembered:'); + expect(prompt.length, `worst-case prompt is ${prompt.length} characters`).toBeLessThan( + CEILING_CHARS + memory.length, + ); + }); +}); + +describe('buildVoiceSystemContext — no block is worth the call', () => { + const failing = () => async () => { + throw new Error('database is down'); + }; + + it.each([ + ['loadActivePlan', '<>'], + ['loadAgentMemory', '<>'], + ['loadPersonalization', 'ABOUT THE USER'], + ] as const)('given %s fails, should drop only that block', async (dep, marker) => { + const { deps: d, warn } = deps({ [dep]: failing() }); + + const prompt = await promptOf(d, { + userId: 'u1', + conversationId: 'conv1', + agent: agent(), + }); + + expect(prompt).not.toContain(marker); + // Everything the call actually needs is still there. + expect(prompt).toContain('execute_tool'); + expect(warn).toHaveBeenCalled(); + }); + + it('given a failed read, should name which block gave up', async () => { + // A dropped block shows up as a model that has quietly stopped knowing + // something — invisible in a transcript, impossible to bisect without this. + const { deps: d, warn } = deps({ loadActivePlan: failing() }); + + await promptOf(d, { userId: 'u1', conversationId: 'conv1' }); + + expect(warn).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ block: 'activePlan' }), + ); + }); + +}); diff --git a/apps/web/src/lib/ai/realtime/__tests__/tools.test.ts b/apps/web/src/lib/ai/realtime/__tests__/tools.test.ts index 9da055ae5e..55fdebaca6 100644 --- a/apps/web/src/lib/ai/realtime/__tests__/tools.test.ts +++ b/apps/web/src/lib/ai/realtime/__tests__/tools.test.ts @@ -1,7 +1,13 @@ import { describe, expect, it } from 'vitest'; import { z } from 'zod'; import type { Tool, ToolSet } from 'ai'; -import { buildRealtimeTools, buildRealtimeToolSet, toRealtimeTool } from '../tools'; +import { + buildRealtimeToolExposure, + buildRealtimeToolSet, + toRealtimeTool, + toRealtimeTools, + type ToolAllowlist, +} from '../tools'; import { CORE_TOOL_NAMES } from '../../core/stub-tools'; import { createToolSearchTool } from '../../tools/tool-search-tool'; import { createExecuteTool } from '../../tools/execute-tool'; @@ -24,6 +30,13 @@ const smallSet = (): ToolSet => ({ const names = (tools: readonly { name: string }[]) => tools.map((t) => t.name); +/** + * What the session would actually advertise: the exposure, projected. The two + * steps are one call here because every case below is about the result of both. + */ +const advertised = (tools: ToolSet, allowlist: ToolAllowlist = null) => + toRealtimeTools(buildRealtimeToolExposure(tools, allowlist).tools); + /** Every key anywhere in a JSON value, at any depth. */ function deepKeys(value: unknown): string[] { if (Array.isArray(value)) return value.flatMap(deepKeys); @@ -34,10 +47,10 @@ function deepKeys(value: unknown): string[] { ]); } -describe('buildRealtimeTools', () => { +describe('the advertised tool definitions', () => { it('given the full PageSpace ToolSet, should emit exactly the core tools plus the two scaffolding tools', () => { const registry = buildPageSpaceTools({ codeExecutionEnabled: true }); - const emitted = names(buildRealtimeTools(registry)); + const emitted = names(advertised(registry)); const registryCoreNames = Object.keys(registry).filter((n) => CORE_TOOL_NAMES.has(n)); expect(new Set(emitted)).toEqual( @@ -58,7 +71,7 @@ describe('buildRealtimeTools', () => { it('given the full PageSpace ToolSet, should defer every non-core tool rather than front-loading it', () => { const registry = buildPageSpaceTools({ codeExecutionEnabled: true }); - const emitted = new Set(names(buildRealtimeTools(registry))); + const emitted = new Set(names(advertised(registry))); const nonCore = Object.keys(registry).filter((n) => !CORE_TOOL_NAMES.has(n)); // The registry is big enough for deferral to be the point of the split. @@ -73,11 +86,11 @@ describe('buildRealtimeTools', () => { // any tool input is unrepresentable in JSON Schema and makes z.toJSONSchema // throw. This case fails the moment that lands, in whichever module it lands. // - // It drives `toRealtimeTool` directly, NOT buildRealtimeTools: the latter + // It drives `toRealtimeTool` directly, NOT the advertised set: an exposure // converts only the upfront half, so a deferred tool — which is most of the // registry — would never have its schema touched and the guard would pass // vacuously. (Mutation-checked: adding `z.date()` to a deferred tool goes red - // here and stays green through buildRealtimeTools.) + // here and stays green through the advertised set.) const registry = buildPageSpaceTools({ codeExecutionEnabled: true }); expect(Object.keys(registry).length).toBeGreaterThan(50); @@ -87,7 +100,7 @@ describe('buildRealtimeTools', () => { `tool "${name}" is not representable as realtime parameters`, ).not.toThrow(); } - expect(() => buildRealtimeTools(registry)).not.toThrow(); + expect(() => advertised(registry)).not.toThrow(); }); it('given an unrepresentable input schema, should throw rather than emit a lie', () => { @@ -99,7 +112,7 @@ describe('buildRealtimeTools', () => { }); it('given any tool, should emit the FLAT realtime function shape', () => { - for (const tool of buildRealtimeTools(smallSet())) { + for (const tool of advertised(smallSet())) { expect(tool.type).toBe('function'); expect(typeof tool.name).toBe('string'); expect(typeof tool.description).toBe('string'); @@ -116,7 +129,7 @@ describe('buildRealtimeTools', () => { }); it('given a zod-object inputSchema, should emit inlined JSON Schema parameters', () => { - const [readPage] = buildRealtimeTools({ + const [readPage] = advertised({ read_page: fakeTool({ description: 'Read a page.', inputSchema: z.object({ @@ -139,7 +152,7 @@ describe('buildRealtimeTools', () => { it('given a schema that reuses a sub-schema, should inline it with no $ref/$defs', () => { const shared = z.object({ id: z.string() }); - const [tool] = buildRealtimeTools({ + const [tool] = advertised({ read_page: fakeTool({ inputSchema: z.object({ from: shared, to: shared }), }), @@ -164,12 +177,12 @@ describe('buildRealtimeTools', () => { const raw = z.toJSONSchema(z.object({ pageId: z.string() })) as Record; expect(raw.$schema).toBeDefined(); - const [tool] = buildRealtimeTools({ read_page: fakeTool() }); + const [tool] = advertised({ read_page: fakeTool() }); expect(tool.parameters).not.toHaveProperty('$schema'); }); it('given a tool with no description, should still emit a valid definition', () => { - const [tool] = buildRealtimeTools({ + const [tool] = advertised({ read_page: fakeTool({ description: undefined }), }); @@ -187,7 +200,7 @@ describe('buildRealtimeTools', () => { }); it('given a tool with a description, should carry it through verbatim', () => { - const [tool] = buildRealtimeTools({ + const [tool] = advertised({ read_page: fakeTool({ description: 'Read a page aloud.' }), }); expect(tool.description).toBe('Read a page aloud.'); @@ -197,7 +210,7 @@ describe('buildRealtimeTools', () => { // Built from the same factories, so voice and text cannot describe the two // discovery tools differently. const set = smallSet(); - const emitted = buildRealtimeTools(set); + const emitted = advertised(set); const search = emitted.find((t) => t.name === 'tool_search'); const execute = emitted.find((t) => t.name === 'execute_tool'); @@ -220,15 +233,23 @@ describe('buildRealtimeTools', () => { }); }); - it('given an empty tool set, should still emit the discovery scaffolding', () => { - expect(names(buildRealtimeTools({}))).toEqual(['tool_search', 'execute_tool']); + it('given an empty tool set, should emit NOTHING — not scaffolding over an empty catalog', () => { + // Was: scaffolding regardless. `tool_search` over nothing and an + // `execute_tool` that can only refuse are two tools whose every call fails, + // which is worse than two tools absent. This is applyToolExposureMode's own + // rule (tool-exposure.ts:130-132), now shared rather than re-decided. + expect(names(advertised({}))).toEqual([]); + }); + + it('given only core tools, should skip the scaffolding — there is nothing to discover', () => { + expect(names(advertised({ read_page: fakeTool() }))).toEqual(['read_page']); }); it('given composer-toggled tools, should defer them like any other non-core tool', () => { // A voice call has no composer toggles, so `web_search`/`generate_image` get // no always-upfront rescue — they are reachable through execute_tool. const emitted = names( - buildRealtimeTools({ + advertised({ read_page: fakeTool(), web_search: fakeTool(), generate_image: fakeTool(), @@ -240,42 +261,109 @@ describe('buildRealtimeTools', () => { it('given the same tool set twice, should be pure — equal output, input untouched', () => { const set = smallSet(); const snapshot = Object.keys(set); - expect(buildRealtimeTools(set)).toEqual(buildRealtimeTools(set)); + expect(advertised(set)).toEqual(advertised(set)); expect(Object.keys(set)).toEqual(snapshot); expect(set).not.toHaveProperty('tool_search'); }); }); +/** + * The half voice used to throw away. + * + * `tool_search` and `execute_tool` rode every session while nothing in the + * instructions named them, so every deferred tool — the calendar family, + * spawn_session, create_task, the workflow tools — was loaded and undiscoverable, + * and the model answered "I can't do that" about tools it was holding. The + * exposure now carries the text describing itself. + */ +describe('buildRealtimeToolExposure — the discovery prompt', () => { + it('given deferred tools, should return the prompt that tells the model how to reach them', () => { + const { toolDiscoveryPrompt } = buildRealtimeToolExposure(smallSet()); + + expect(toolDiscoveryPrompt).toContain('execute_tool'); + expect(toolDiscoveryPrompt).toContain('tool_search'); + }); + + it('should name the deferred tools, so the model knows what exists before it searches', () => { + const { toolDiscoveryPrompt } = buildRealtimeToolExposure({ + read_page: fakeTool(), + create_task: fakeTool(), + rename_drive: fakeTool(), + }); + + expect(toolDiscoveryPrompt).toContain('create_task'); + expect(toolDiscoveryPrompt).toContain('rename_drive'); + }); + + it('should describe only tools the allowlist permits', () => { + // A name in the prompt is an invitation to call it. Naming a blocked tool + // both leaks the agent's configuration and spends a turn on a refusal. + const { toolDiscoveryPrompt } = buildRealtimeToolExposure( + { read_page: fakeTool(), create_task: fakeTool(), rename_drive: fakeTool() }, + ['read_page', 'create_task'], + ); + + expect(toolDiscoveryPrompt).toContain('create_task'); + expect(toolDiscoveryPrompt).not.toContain('rename_drive'); + }); + + it('given nothing to defer, should return no prompt rather than an empty instruction', () => { + expect(buildRealtimeToolExposure({ read_page: fakeTool() }).toolDiscoveryPrompt).toBe(''); + expect(buildRealtimeToolExposure({}).toolDiscoveryPrompt).toBe(''); + }); + + it('should describe the SAME tools it advertises, never a wider or narrower set', () => { + // The prompt and the tool set are one decision. If they can disagree, the + // model is either told about a tool nothing answers, or holds one it was + // never told it had — which is the bug this whole seam exists to prevent. + const registry = buildPageSpaceTools({ codeExecutionEnabled: true }); + const { tools, toolDiscoveryPrompt } = buildRealtimeToolExposure(registry); + + const advertised = new Set(Object.keys(tools)); + const deferred = Object.keys(registry).filter((n) => !advertised.has(n)); + + expect(deferred.length).toBeGreaterThan(0); + for (const name of deferred) { + expect(toolDiscoveryPrompt, `deferred tool "${name}" is not named in the prompt`).toContain( + name, + ); + } + }); +}); + /** * The agent's allowlist is what its owner switched off. Advertising past it * told the model it could call write and delete tools an owner had disabled — * and because `execute_tool` reaches everything the split deferred, filtering * only the upfront half would have left them all callable anyway. */ -describe('buildRealtimeTools — the bound agent allowlist', () => { +describe('the advertised set — the bound agent allowlist', () => { it('given an allowlist, should advertise only what it names', () => { const emitted = names( - buildRealtimeTools( + advertised( { read_page: fakeTool(), delete_page: fakeTool(), rename_drive: fakeTool() }, - ['read_page'], + ['read_page', 'rename_drive'], ), ); + // rename_drive is allowed but non-core, so it defers behind the scaffolding + // rather than being front-loaded. delete_page is not allowed and is nowhere. expect(emitted).toEqual(['read_page', 'tool_search', 'execute_tool']); }); it('given null, should treat the agent as unrestricted', () => { const set = { read_page: fakeTool(), rename_drive: fakeTool() }; - expect(names(buildRealtimeTools(set, null))).toEqual(names(buildRealtimeTools(set))); + expect(names(advertised(set, null))).toEqual(names(advertised(set))); }); - it('given an EMPTY allowlist, should advertise nothing but the scaffolding', () => { - // [] is "every PageSpace tool off", not "unconfigured". + it('given an EMPTY allowlist, should advertise nothing at all', () => { + // [] is "every PageSpace tool off", not "unconfigured" — and with every tool + // off there is nothing for the scaffolding to reach either. const emitted = names( - buildRealtimeTools({ read_page: fakeTool(), rename_drive: fakeTool() }, []), + advertised({ read_page: fakeTool(), rename_drive: fakeTool() }, []), ); - expect(emitted).toEqual(['tool_search', 'execute_tool']); + expect(emitted).toEqual([]); }); it('should keep a blocked tool out of the EXECUTABLE set as well, not just the advertised one', () => { @@ -283,8 +371,8 @@ describe('buildRealtimeTools — the bound agent allowlist', () => { // it reaches everything else. A filter applied to only one of them is not a // filter. const executable = buildRealtimeToolSet( - { read_page: fakeTool(), rename_drive: fakeTool() }, - ['read_page'], + { read_page: fakeTool(), create_task: fakeTool(), rename_drive: fakeTool() }, + ['read_page', 'create_task'], ); expect(Object.keys(executable).sort()).toEqual( @@ -297,8 +385,8 @@ describe('buildRealtimeTools — the bound agent allowlist', () => { // refused — which is both a leak of the agent's configuration and an // invitation to spend a turn failing. const search = buildRealtimeToolSet( - { read_page: fakeTool(), rename_drive: fakeTool() }, - ['read_page'], + { read_page: fakeTool(), create_task: fakeTool(), rename_drive: fakeTool() }, + ['read_page', 'create_task'], ).tool_search; const described = JSON.stringify( await (search.execute as (a: unknown, o: unknown) => unknown)( @@ -312,8 +400,11 @@ describe('buildRealtimeTools — the bound agent allowlist', () => { it('should never filter away the scaffolding itself', () => { // tool_search and execute_tool are how an allowlist is reached at all, not - // capabilities an owner grants. - const emitted = names(buildRealtimeTools({ read_page: fakeTool() }, ['nothing_matches'])); + // capabilities an owner grants — so they appear even when the allowlist + // names neither of them, as no allowlist ever does. + const emitted = names( + advertised({ read_page: fakeTool(), rename_drive: fakeTool() }, ['rename_drive']), + ); expect(emitted).toEqual(['tool_search', 'execute_tool']); }); }); diff --git a/apps/web/src/lib/ai/realtime/__tests__/voice-bridge-contract-drift.test.ts b/apps/web/src/lib/ai/realtime/__tests__/voice-bridge-contract-drift.test.ts index 455b110218..8524f148c7 100644 --- a/apps/web/src/lib/ai/realtime/__tests__/voice-bridge-contract-drift.test.ts +++ b/apps/web/src/lib/ai/realtime/__tests__/voice-bridge-contract-drift.test.ts @@ -1,7 +1,7 @@ /** * Drift guard between the two declarations of one wire shape. * - * `RealtimeTool` (this app) is what `buildRealtimeTools` produces; the shared + * `RealtimeTool` (this app) is what `toRealtimeTools` produces; the shared * `realtimeToolSchema` is what `apps/realtime` validates on arrival. They are * deliberately separate declarations — the realtime server must not import from * the web app — so the only thing keeping them honest is this file. @@ -19,14 +19,14 @@ import { realtimeSeedEventSchema, type RealtimeSeedEventWire, } from '@pagespace/lib/realtime/voice-bridge-contract'; -import { buildRealtimeTools } from '../tools'; +import { buildRealtimeToolExposure, toRealtimeTools } from '../tools'; import { buildRealtimeSeed, type SeedEvent } from '../seed'; import { buildPageSpaceTools } from '../../core/ai-tools'; import type { RealtimeTool } from '../session'; describe('realtime tool wire shape', () => { it('given the real registry, every emitted tool should satisfy the shared schema', () => { - const tools = buildRealtimeTools(buildPageSpaceTools()); + const tools = toRealtimeTools(buildRealtimeToolExposure(buildPageSpaceTools()).tools); expect(tools.length).toBeGreaterThan(0); for (const tool of tools) { @@ -36,7 +36,7 @@ describe('realtime tool wire shape', () => { }); it('should carry the whole registry-built tool set through the attach payload intact', () => { - const tools = buildRealtimeTools(buildPageSpaceTools()); + const tools = toRealtimeTools(buildRealtimeToolExposure(buildPageSpaceTools()).tools); const parsed = realtimeAttachPayloadSchema.safeParse({ callId: 'rtc_u0_abc', diff --git a/apps/web/src/lib/ai/realtime/binding-loader.ts b/apps/web/src/lib/ai/realtime/binding-loader.ts index a75ef8a1ea..8903576f5c 100644 --- a/apps/web/src/lib/ai/realtime/binding-loader.ts +++ b/apps/web/src/lib/ai/realtime/binding-loader.ts @@ -26,7 +26,8 @@ */ import { buildRealtimeSeed, type SeedEvent, type SeedMessage } from './seed'; -import { buildVoiceInstructions } from './instructions'; +import type { RealtimeTool } from './session'; +import type { VoiceCallContext, VoiceSystemContextRequest } from './system-context'; import type { VoiceAssistant } from '@pagespace/lib/realtime/voice-bridge-contract'; /** The conversation facts the access check needs. A `conversations` row satisfies it. */ @@ -60,6 +61,16 @@ export type BindingLoaderDeps = { readonly canAccess: (userId: string, conversation: SeedConversation) => Promise; readonly loadMessages: (conversationId: string) => Promise; readonly loadAgentPage: (pageId: string) => Promise; + /** + * Everything the session is opened with, for whatever this call turned out to + * be bound to: the same system prompt the typed surface builds capped with the + * spoken-medium override, and the tools to advertise — from one exposure, so + * the two cannot describe different capabilities. Injected rather than called + * directly so this module stays exercisable without a database, and so the one + * place that answers "what is this bound to?" is not also the place that does + * half a dozen reads to describe it. See `system-context.ts`. + */ + readonly buildCallContext: (request: VoiceSystemContextRequest) => Promise; readonly logger: { readonly warn: (message: string, meta?: Record) => void; }; @@ -83,13 +94,27 @@ export type BindingLoaderRequest = { export type VoiceBinding = { readonly seed: SeedEvent[]; readonly instructions: string; + /** + * The tools to advertise on this session. Carried on the binding rather than + * rebuilt by the route because they and `instructions` come from one exposure: + * a session that advertises `tool_search` while its prompt never names it is + * the exact failure this whole change is about. + */ + readonly tools: readonly RealtimeTool[]; readonly assistant?: VoiceAssistant; }; -/** The binding for a call with nothing to bind to: no history, default persona. */ -const unbound = (): VoiceBinding => ({ +/** + * The binding for a call with nothing to bind to: no history, and the Global + * Assistant's own prompt — which is the same assistant the dashboard talks to, + * so an unbound call is a real call, not a degraded one. + */ +const unbound = async ( + deps: BindingLoaderDeps, + userId: string, +): Promise => ({ seed: [], - instructions: buildVoiceInstructions({}), + ...(await deps.buildCallContext({ userId })), }); /** @@ -130,20 +155,20 @@ export const loadVoiceBinding = async ( deps: BindingLoaderDeps, request: BindingLoaderRequest, ): Promise => { - if (!request.conversationId) return unbound(); + if (!request.conversationId) return unbound(deps, request.userId); try { const conversation = await deps.loadConversation(request.conversationId); // No row is the ORDINARY case, not an error: a thread the user just opened // has a client-minted id and no row until its first message lands. - if (!conversation || !conversation.isActive) return unbound(); + if (!conversation || !conversation.isActive) return unbound(deps, request.userId); if (!(await deps.canAccess(request.userId, conversation))) { deps.logger.warn('Realtime voice binding refused: caller cannot access the conversation', { userId: request.userId, conversationId: request.conversationId, }); - return unbound(); + return unbound(deps, request.userId); } const [messages, agent] = await Promise.all([ @@ -156,14 +181,32 @@ export const loadVoiceBinding = async ( ...(request.maxTokens === undefined ? {} : { maxTokens: request.maxTokens }), }); - if (!agent) return { seed, instructions: buildVoiceInstructions({}) }; + // A conversation the caller may read but that has no agent behind it is + // still the Global Assistant's — and it still has a conversation, so the + // prompt gets its coordinates and its plan pointer. + const contextOf = (): VoiceSystemContextRequest => ({ + userId: request.userId, + conversationId: request.conversationId as string, + conversation: { type: conversation.type, contextId: conversation.contextId }, + ...(agent === undefined + ? {} + : { + agent: { + pageId: agent.id, + title: agent.title, + systemPrompt: agent.systemPrompt, + enabledTools: agent.enabledTools, + }, + }), + }); + + const callContext = await deps.buildCallContext(contextOf()); + + if (!agent) return { seed, ...callContext }; return { seed, - instructions: buildVoiceInstructions({ - title: agent.title, - ...(agent.systemPrompt === null ? {} : { systemPrompt: agent.systemPrompt }), - }), + ...callContext, assistant: { agentPageId: agent.id, agentTitle: agent.title, @@ -178,6 +221,6 @@ export const loadVoiceBinding = async ( conversationId: request.conversationId, error: error instanceof Error ? error.message : 'unknown', }); - return unbound(); + return unbound(deps, request.userId); } }; diff --git a/apps/web/src/lib/ai/realtime/instructions.ts b/apps/web/src/lib/ai/realtime/instructions.ts index 14574f489a..844d2b8ff9 100644 --- a/apps/web/src/lib/ai/realtime/instructions.ts +++ b/apps/web/src/lib/ai/realtime/instructions.ts @@ -1,99 +1,164 @@ /** * What the model is told it is, on a spoken call. * - * Every voice session used to be minted with a model and nothing else, so the - * call ran on the realtime model's generic default persona no matter which - * assistant the user had selected — while the UI, and the changelog, presented - * it as talking to that assistant. This is the fix for the half of that gap - * that is text: who you are, and what a spoken turn is. + * A CALL GETS THE REAL SYSTEM PROMPT. Voice is a transport onto a conversation + * PageSpace already has, so it is handed the same assembly the typed surface + * builds — `buildAgentSystemPrompt`, with the same workspace knowledge, the + * same skill catalog, the same tool-discovery block naming every deferred tool. + * That block is the whole reason this file changed: `tool_search` and + * `execute_tool` rode every session while nothing ever named them, so the + * calendar family, `spawn_session`, `create_task` and the workflow tools were + * loaded and undiscoverable, and a call could only ever reach the ten core + * tools. It was a conversation you could have, not an agent you could delegate + * to. * - * TWO PARTS, IN THIS ORDER: - * 1. THE MEDIUM. A spoken answer cannot be skimmed, scrolled back through, or - * skipped over — and the caller can interrupt, which a text surface has no - * equivalent of. That changes what a good answer looks like far more than - * which assistant is speaking does, so it comes first and applies to every - * call. - * 2. THE ASSISTANT. A page agent's own `systemPrompt` verbatim, because that - * is exactly the string its owner wrote and the text surface sends it - * unmodified too. Wrapping, summarising or "adapting" it here would be a - * second interpretation of a field that already has one. + * This module adds exactly one thing to that prompt: what is different because + * the words are HEARD. * - * WHAT IS DELIBERATELY NOT HERE. The text pipeline's system prompt also carries - * a page tree, agent memory, a skill catalog, tool-discovery guidance and the - * turn-volatile location block. None of it is copied in: - * - most of it is turn-volatile, and a realtime session's instructions are - * set once at attach and are not re-sent per turn, so a copy would be stale - * for the rest of the call; - * - the tool guidance is written for a surface where the model can run a long - * tool loop while the reader waits silently, which is the opposite of what - * a caller experiences; - * - and a second copy of any of it is a copy that drifts from - * `page-chat-turn.ts` the first time either changes. - * Where the model is standing still reaches the tools, through - * `locationContext` on the execution context — which is the part that has to be - * live, and is. + * IT GOES LAST, AND IT SAYS SO. A spoken turn contradicts the typed one in + * places — the typed prompt says to skip preambles, while a call needs a short + * line before a slow tool or the caller hears silence and assumes the line + * dropped. `gpt-realtime` degrades on conflicting instructions specifically, so + * the conflicts are named and resolved here rather than left for the model to + * arbitrate, and this section sits after the thing it overrides. + * + * WHAT IS STILL DELIBERATELY NOT HERE. A realtime session's instructions are + * sent once, in the `session.update` at socket open, and there is no path that + * sends a second one — so nothing turn-volatile can live in this string. The + * caller's LOCATION is the live example: it reaches the tools instead, through + * `locationContext` on the execution context, and navigating mid-call updates + * that rather than rebinding the session. Baking a location in would make it a + * lie the moment the caller walked to another page; the override block instead + * tells the model the ids are omittable, which is true for the whole call. * * Pure: no I/O, no clock, no randomness, no module-level mutable state. */ /** - * The assistant a call is bound to, as far as its instructions are concerned. - * An empty object is the Global Assistant — or an unbound call, which gets the - * same persona because it is the same one the dashboard talks to. + * What the model can actually call, as the override block needs to know it. + * + * A RULE THAT NAMES A TOOL THE MODEL DOES NOT HAVE IS WORSE THAN NO RULE. It + * spends a turn on a call that is rejected, and spends it out loud, in front of + * someone waiting. Both halves are needed because they answer different + * questions: `tool_search` only ever exists in the exposed set, and + * `spawn_session` only ever exists in the reachable one. */ +export type VoiceToolReach = { + /** Advertised with full schemas — the core tools, plus the scaffolding when there is any. */ + readonly exposed: readonly string[]; + /** Everything callable at all, including the half deferred behind `execute_tool`. */ + readonly reachable: readonly string[]; +}; + +/** What the call is bound to, as far as its instructions are concerned. */ export type VoiceInstructionsInput = { - /** The agent page's title. Absent for the Global Assistant. */ + /** + * The shared agent system prompt for whichever surface this call is bound to + * — a page agent's, or the Global Assistant's. Already assembled by the + * caller, because building it reads the database and this module does not. + */ + readonly agentSystemPrompt: string; + /** + * The bound agent's title, when there is one. Named on the call even though + * the typed surface does not name it: on screen the user can see which + * assistant they opened, and on a call being addressed by the name they + * picked is most of what makes it feel like the one they picked. + */ readonly title?: string; - /** The agent's own configured prompt, when its owner wrote one. */ - readonly systemPrompt?: string; + /** Omitted means "assume nothing is reachable" — the safe direction. */ + readonly tools?: VoiceToolReach; }; /** - * How to behave because this is speech rather than text. + * Where long work goes, each paired with the tools that have to be reachable + * before it can be offered. Any one of them is enough — the phrase describes a + * destination, and a caller who can set a trigger but not a workflow can still + * be told work can happen later. + */ +const HAND_OFFS: readonly (readonly [readonly string[], string])[] = [ + [['spawn_session'], 'spawn_session for work an agent should carry out'], + [['create_task'], 'create_task for work a person should'], + [ + ['set_task_trigger', 'create_workflow'], + 'a trigger or a workflow for work that should happen later', + ], +]; + +/** + * How to behave because this is speech rather than text, and because the caller + * is talking in order to get something DONE rather than to have a conversation. * - * Every line here is about the medium, not about PageSpace: brevity because a - * listener cannot skim, plain prose because markdown is read aloud as noise, - * and a short acknowledgement before a slow tool because the alternative is - * silence the caller reads as a dropped connection. + * Written as labeled sections of short bullets, with the load-bearing rules + * capitalized, because that is the shape `gpt-realtime` follows most reliably. */ -const SPOKEN_TURN_PREAMBLE = [ - 'You are speaking with someone out loud, in real time. Your replies are heard, not read.', - '', - '- Keep answers short. Two or three sentences is usually right; offer detail rather than delivering it.', - '- Speak in plain prose. Never read out markdown, bullet characters, code fences, URLs or raw ids.', - '- If a tool will take a moment, say so in a few words first, so the pause is not silence.', - '- Expect to be interrupted, and stop cleanly when you are. Do not restart an answer from the top.', - '- If you did not catch something, ask — do not guess at what was said.', -].join('\n'); - -/** What the model is, when nothing more specific is bound. */ -const PAGESPACE_ASSISTANT = [ - 'You are the PageSpace assistant. PageSpace is the workspace this person keeps their', - 'pages, drives, tasks and agents in, and your tools act on that workspace on their behalf.', -].join(' '); - -export const buildVoiceInstructions = (assistant: VoiceInstructionsInput): string => { - const parts = [SPOKEN_TURN_PREAMBLE]; - - const systemPrompt = assistant.systemPrompt?.trim(); - if (systemPrompt) { - // The agent's own words, verbatim. Its title is named alongside rather than - // folded in, because a prompt that already establishes a persona must not - // be contradicted by a second one bolted on top. - parts.push( - assistant.title - ? `You are "${assistant.title}", an assistant in PageSpace. Your instructions follow.` - : 'You are an assistant in PageSpace. Your instructions follow.', - systemPrompt, - ); - } else if (assistant.title) { - // A page agent whose owner configured no prompt: it is still a distinct - // assistant with a name, and being called by that name is most of what - // makes it feel like the one the user picked. - parts.push(`You are "${assistant.title}", an assistant in PageSpace.`, PAGESPACE_ASSISTANT); - } else { - parts.push(PAGESPACE_ASSISTANT); - } - - return parts.join('\n\n'); +const voiceOverride = (title: string | undefined, tools: VoiceToolReach): string => { + const identity = title + ? `You are "${title}", speaking with someone out loud, in real time.` + : 'You are speaking with someone out loud, in real time.'; + + // An agent allowed only core tools gets no `tool_search` and no + // `execute_tool` — there is nothing deferred for them to reach — so the rule + // sending the model to search before refusing would name a tool that is not + // there. In that case everything it can do is already in front of it, and + // saying so is the honest replacement. + const canSearch = tools.exposed.includes('tool_search'); + const beforeRefusing = canSearch + ? '- NEVER SAY SOMETHING IS IMPOSSIBLE BEFORE YOU HAVE CALLED tool_search.' + : '- Every tool you have is already listed for you. If none of them fits, say so plainly rather than guessing at one.'; + + // Same rule for the hand-off targets: each is offered only when the agent can + // actually reach it, and an agent granted none of them cannot delegate at all. + // Naming one anyway would spend a turn on a call `execute_tool` rejects — out + // loud, in front of someone waiting. + const handOffs = HAND_OFFS.filter(([names]) => + names.some((name) => tools.reachable.includes(name)), + ).map(([, phrase]) => phrase); + + const delegating = + handOffs.length === 0 + ? `## Long work +- Work that takes minutes does not belong inline on a call — the caller would wait in silence. Say what you are starting, work through it in steps, and keep saying where you have got to.` + : `## Delegating +- Work that takes minutes does not belong inline on a call — the caller would wait in silence. Hand it off and say that you have: ${handOffs.join(', ')}. +- Say what you handed off and where it will land, in one sentence. If the caller asks later in the call how it is going, check then.`; + + return `# THIS IS A VOICE CALL + +${identity} Everything above still applies EXCEPT where this section overrides it. This is a delegation surface: the caller is talking so they do not have to type. Success is the request DONE by the end of the call — not described, not offered. + +## Speaking +- Your replies are heard, not read. Two or three sentences per turn. Offer detail rather than delivering it. +- Never read out markdown, bullet characters, code fences, URLs or raw ids. Name pages by their title. +- Read a code or a number one character at a time, separated by hyphens. +- Never reuse the same opener or acknowledgement twice in a row. +- Reply in the language the caller is speaking. No sound effects or onomatopoeia. +- Expect to be interrupted, and stop cleanly when you are. Do not restart an answer from the top. + +## Acting — these OVERRIDE the guidance above +- "Skip preambles" does NOT apply here. Say one short line AS you call a tool, then call it immediately, so the pause is not silence. Vary these: "One moment." "Let me check." "Pulling that up." "Adding that now." +- A filler must NOT imply success or failure. Never say you found or changed something before the tool has returned. +- DO NOT ASK PERMISSION TO USE A TOOL. When you know what the caller wants, do it, then say what you did. +- Chain tools. A request that needs four calls gets four calls, not a question after the first. +- "This page", "here" and "this drive" resolve on their own — call read_page, insert_content or replace_lines with NO page id and they act on wherever the caller is standing. Never ask the caller for an id. +- Keep going until the request is resolved. An empty search is not an answer: try different wording, or another drive, before reporting nothing. +- IF THE SAME TOOL FAILS TWICE ON THE SAME TASK, stop retrying, say plainly what failed, and offer the next best thing. +${beforeRefusing} +- Close every stretch of tool calls with one spoken sentence saying what changed. Silence after a run of tool calls sounds like a dropped call. + +${delegating} + +## What you were told, and when +- Everything above was assembled when this call started and is never re-sent. If you change any of it during the call — bind or clear a plan, edit your memory page, move or rename something — YOUR OWN TOOL RESULT IS WHAT IS CURRENT, and the description above is out of date from that moment on. +- Never re-follow a standing instruction about something you have since changed. Say what the change was, then work from it. + +## Asking +- ask_user draws a card on a screen and does not work here. Ask out loud, in one sentence. +- Only respond to clear audio or text. If the audio is unintelligible — background noise, partial words, silence — ask for clarification in the language the caller is speaking. Do not guess at what was said. +- Never ask for something you could find out yourself by searching or reading first. +- If the caller asks you to stop, stop immediately, mid-action.`; }; + +const NOTHING_REACHABLE: VoiceToolReach = { exposed: [], reachable: [] }; + +export const buildVoiceInstructions = (input: VoiceInstructionsInput): string => + `${input.agentSystemPrompt}\n\n${voiceOverride(input.title, input.tools ?? NOTHING_REACHABLE)}`; diff --git a/apps/web/src/lib/ai/realtime/system-context.ts b/apps/web/src/lib/ai/realtime/system-context.ts new file mode 100644 index 0000000000..6f8a59dddf --- /dev/null +++ b/apps/web/src/lib/ai/realtime/system-context.ts @@ -0,0 +1,254 @@ +/** + * What a call is opened with: the tools it may use, and the instructions + * describing them — the SAME system prompt the typed surface builds, capped + * with what changes because the words are heard. + * + * BOTH COME FROM ONE EXPOSURE, which is the point of assembling them together. + * The tool set and the text describing it are two projections of a single + * decision — what this agent may reach — and the bug this module exists to fix + * was those two disagreeing: sessions carried `tool_search` and `execute_tool` + * while nothing in the prompt ever named them, so every deferred tool was + * loaded and undiscoverable. + * + * A voice call binds to a conversation that is either a page agent's or the + * Global Assistant's. Those are exactly the two surfaces `buildAgentSystemPrompt` + * assembles, so this module gathers that assembly's inputs for whichever one + * the call is bound to and asks for it. It does not compose a prompt of its own: + * a third assembly is a third thing to drift, and the Global Assistant's + * bespoke copy already drifted far enough to describe a page type the product + * does not create. + * + * Split out of `binding-loader.ts` rather than added to it. That module answers + * one question — what is this call bound to? — behind one read and one access + * check, and the answer is small. Assembling a prompt is several independent + * reads with different failure modes, and putting them there would have buried + * the access decision in the middle of them. + * + * NO BLOCK IS WORTH A CALL. Every read here is individually best-effort: a + * block that throws is omitted and logged, and the call still connects with + * everything else intact. That is `binding-loader.ts`'s rule ("a binding is + * never a reason to fail a call") applied one level down — a missing plan + * pointer should cost the plan pointer, not the conversation. + */ + +import type { ToolSet } from 'ai'; +import { buildAgentSystemPrompt } from '../core/prompt-assembly'; +import type { RealtimeTool } from './session'; +import { buildVoiceInstructions } from './instructions'; +import type { PersonalizationInfo } from '../core/system-prompt'; +import { buildBuiltinSkillCatalog } from '../core/skill-catalog'; +import { + buildRealtimeToolExposure, + toRealtimeTools, + type RealtimeToolExposure, + type ToolAllowlist, +} from './tools'; + +/** + * What the session is told, and what it is given — from ONE exposure. + * + * They ship together because they are two projections of a single decision, and + * the whole bug this module exists to fix was those two disagreeing: the + * session carried `tool_search` while the prompt never named it. Computing the + * exposure once also keeps a second full registry build off the handshake, + * which the caller is waiting on. + */ +export type VoiceCallContext = { + /** The system prompt for the bound surface, capped with the spoken override. */ + readonly instructions: string; + /** The tool definitions to advertise, already on the realtime wire shape. */ + readonly tools: readonly RealtimeTool[]; +}; + +/** The agent a call is bound to, when it is bound to one rather than to the Global Assistant. */ +export type BoundAgent = { + readonly pageId: string; + readonly title: string; + readonly systemPrompt: string | null; + readonly enabledTools: string[] | null; +}; + +/** The conversation's own coordinates, which the Global Assistant reports back to the model. */ +export type BoundConversation = { + readonly type: string; + readonly contextId: string | null; +}; + +export type VoiceSystemContextDeps = { + /** The registry to expose. A parameter because building it reads the code-execution kill switch. */ + readonly buildTools: () => ToolSet; + /** The agent's own memory page, already rendered as a prompt section. */ + readonly loadAgentMemory: (pageId: string, userId: string) => Promise; + /** The active plan pointer, already rendered. Empty string when there is none. */ + readonly loadActivePlan: (conversationId: string, userId: string) => Promise; + /** The caller's own bio, style and rules, when they enabled them. */ + readonly loadPersonalization: (userId: string) => Promise; + readonly logger: { + readonly warn: (message: string, meta?: Record) => void; + }; +}; + +export type VoiceSystemContextRequest = { + readonly userId: string; + readonly conversationId?: string; + readonly agent?: BoundAgent; + readonly conversation?: BoundConversation; +}; + +/** + * Run one prompt block's read, and let it fail alone. + * + * The name of the block is logged rather than inferred from a stack, because + * the symptom of a silently-dropped block is a model that has simply stopped + * knowing something — which is invisible in a transcript and impossible to + * bisect without knowing which read gave up. + */ +const softly = async ( + deps: VoiceSystemContextDeps, + block: string, + read: () => Promise, + whenMissing: T, +): Promise => { + try { + return await read(); + } catch (error) { + deps.logger.warn('Realtime voice prompt block could not be loaded; continuing without it', { + block, + error: error instanceof Error ? error.message : 'unknown', + }); + return whenMissing; + } +}; + +/** + * Cap the assembled prompt with the spoken-medium override. + * + * Done here rather than a layer up because the override has to know what the + * model can actually call: an agent allowed only core tools is given no + * `tool_search`, so the rule sending it to search before refusing would name a + * tool that is not there. This is the only place holding both halves of that — + * the exposed set and the reachable one — so it is the only place that can + * decide. + */ +const withVoiceOverride = ( + exposure: RealtimeToolExposure, + agentSystemPrompt: string, + title?: string, +): string => + buildVoiceInstructions({ + agentSystemPrompt, + ...(title === undefined ? {} : { title }), + tools: { + exposed: Object.keys(exposure.tools), + reachable: exposure.allowedToolNames, + }, + }); + +/** + * Assemble everything the session is opened with. + * + * THREE BLOCKS THE TYPED SURFACE CARRIES ARE DELIBERATELY OMITTED, and the + * reason is the same for all three — a realtime session's instructions are sent + * once, at socket open, and there is no path that sends a second one: + * + * - the PAGE TREE, because it is the largest block by far and a call's context + * window is shared with the audio flowing through it for the length of the + * call. It is also the first thing a token budget would cut, so it is cut. + * - the DRIVE prompt and the CROSS-DRIVE MEMBER context, because both are keyed + * to the drive the caller is standing in, and on a call they can walk to + * another one without the session rebinding. A drive's instructions frozen at + * the moment the call connected would go quietly wrong the first time that + * happened; the tools read the live location instead. + * + * THE EAGER AGENT LIST IS OMITTED FOR A DIFFERENT REASON: WHAT IT COSTS TO GET. + * `buildAgentAwarenessPrompt` selects every non-trashed drive in the deployment + * and then awaits an access check per drive and a view check per agent, one + * after another. Everything here runs BEFORE the SDP exchange, with the caller + * holding a dead line waiting to be heard, so that work would put a + * whole-deployment scan on the most latency-sensitive path in the product. The + * capability is not lost: the AGENTS guidance is still in the prompt, and + * `list_agents` fetches the list on the one turn that actually needs it rather + * than on every call that might. + * + * Read-only mode is `false`: it is a property of a typed session's toggles, and + * a call has none. An agent whose owner restricted its tools is still restricted + * — that rides `enabledTools` through the exposure, which is a different + * mechanism and is applied. + */ +export const buildVoiceCallContext = async ( + deps: VoiceSystemContextDeps, + request: VoiceSystemContextRequest, +): Promise => { + const allowlist: ToolAllowlist = request.agent?.enabledTools ?? null; + const exposure = buildRealtimeToolExposure(deps.buildTools(), allowlist); + // The PRE-split names, not `Object.keys(exposure.tools)`. After the split that + // object holds the core tools and the two scaffolding tools, so gating on its + // keys would drop the task-management, delegation and automation guidance for + // exactly the capabilities the discovery catalog goes on to advertise. + const { allowedToolNames } = exposure; + const skillCatalog = buildBuiltinSkillCatalog(allowedToolNames); + + const [activePlan, personalization] = await Promise.all([ + request.conversationId + ? softly( + deps, + 'activePlan', + () => deps.loadActivePlan(request.conversationId as string, request.userId), + '', + ) + : Promise.resolve(''), + softly(deps, 'personalization', () => deps.loadPersonalization(request.userId), null), + ]); + + if (request.agent) { + const agentMemory = await softly( + deps, + 'agentMemory', + () => deps.loadAgentMemory((request.agent as BoundAgent).pageId, request.userId), + '', + ); + + const systemPrompt = buildAgentSystemPrompt({ + surface: 'page', + readOnly: false, + personalization, + allowedToolNames, + skillCatalog, + activePlan, + pageTree: '', + customSystemPrompt: request.agent.systemPrompt, + drivePromptPrefix: '', + memberDriveContextPrefix: '', + agentMemory, + toolDiscovery: exposure.toolDiscoveryPrompt, + }); + + return { + instructions: withVoiceOverride(exposure, systemPrompt, request.agent.title), + tools: toRealtimeTools(exposure.tools), + }; + } + + const systemPrompt = buildAgentSystemPrompt({ + surface: 'global', + readOnly: false, + personalization, + allowedToolNames, + skillCatalog, + activePlan, + pageTree: '', + // An unbound call has no conversation row yet — the same state a brand-new + // typed thread is in before its first message lands. + conversationType: request.conversation?.type ?? 'global', + conversationContextId: request.conversation?.contextId ?? null, + // `ask_user` draws a card on a screen. The override block tells the model to + // ask out loud instead, so describing the tool here would only argue with it. + includeAskUser: false, + drivePromptSection: '', + // Not the eager agent list the typed surface renders — see the note above. + agentAwareness: '', + nonCoreToolNames: exposure.nonCoreToolNames, + }); + + return { instructions: withVoiceOverride(exposure, systemPrompt), tools: toRealtimeTools(exposure.tools) }; +}; diff --git a/apps/web/src/lib/ai/realtime/tools.ts b/apps/web/src/lib/ai/realtime/tools.ts index 5855a637d8..d84153ccb0 100644 --- a/apps/web/src/lib/ai/realtime/tools.ts +++ b/apps/web/src/lib/ai/realtime/tools.ts @@ -2,9 +2,9 @@ * PageSpace's AI SDK tool registry, projected onto the realtime wire shape. * * Voice is a second transport onto the conversations PageSpace already has, not - * a second capability surface — so the realtime session gets the SAME exposure - * split the text stack uses (`splitToolsForExposure`), and the SAME - * tool_search / execute_tool scaffolding built by the SAME factories. Nothing + * a second capability surface — so the realtime session gets its exposure from + * the SAME function the text routes use (`applyToolExposureMode`), which yields + * the tool set and the discovery prompt describing it as one result. Nothing * here curates a "voice subset": a curated list would be a second tool surface * that silently drifts from the text one every time a tool is added. * @@ -16,10 +16,9 @@ import { z } from 'zod'; import type { Tool, ToolSet } from 'ai'; -import { splitToolsForExposure } from '../tools/tool-exposure'; -import { createToolSearchTool } from '../tools/tool-search-tool'; -import { createExecuteTool } from '../tools/execute-tool'; +import { applyToolExposureMode } from '../tools/tool-exposure'; import { filterToolsForAgentAllowlist } from '../core/tool-filtering'; +import { listEligibleSkills } from '../core/skill-catalog'; import type { RealtimeTool } from './session'; /** @@ -73,8 +72,8 @@ function toRealtimeParameters(inputSchema: Tool['inputSchema']): Record = new Set(); + +/** + * How a call's tools are exposed to the model: the set it may call, and the + * prompt text that tells it how to reach everything else. + * + * THE TWO TRAVEL TOGETHER BECAUSE THEY ARE ONE DECISION. Voice used to build + * the tool half by hand and simply never build the prompt half — so `tool_search` + * and `execute_tool` rode every session while nothing in the instructions ever + * named them, and every non-core tool (the calendar family, spawn_session, + * create_task, the workflow tools) was loaded and undiscoverable. The model + * answered "I can't do that" about tools it was holding. + */ +export type RealtimeToolExposure = { + readonly tools: ToolSet; + /** Both halves as one block: how to reach deferred tools, then their names. */ + readonly toolDiscoveryPrompt: string; + /** The catalog alone, for the surface that states the "how" earlier. */ + readonly nonCoreToolNames: string; + /** + * Every tool the agent may reach, named BEFORE the split moved most of them + * behind `execute_tool` — which is what the prompt's capability sections have + * to be gated on. + * + * Returned rather than left for the caller to derive because deriving it from + * `tools` is wrong in a way nothing catches: after the split that object holds + * the core tools and the two scaffolding tools and nothing else, so gating on + * its keys silently drops the task-management, delegation and automation + * guidance for every capability the split deferred — the exact capabilities + * the discovery catalog then advertises as callable. `page-chat-turn.ts:1306` + * captures the same list at the same point, for the same reason. + */ + readonly allowedToolNames: string[]; +}; + +/** + * Decide the exposure for a call: allowlist first, then the SAME search-mode + * split the text routes use. + * + * `applyToolExposureMode(_, 'search')` is called rather than reproduced. It + * already emits the core tools with full schemas, defers the rest behind + * `tool_search` / `execute_tool` built by the real factories, and returns the + * discovery prompt naming what was deferred — one expression, so the advertised + * tools and the text describing them cannot disagree. Reproducing its body here + * is what dropped the prompt in the first place. + * + * THE AGENT'S ALLOWLIST IS APPLIED FIRST, before the split and before + * `tool_search` is handed its catalog — the same order `page-chat-turn.ts` uses, + * and for the same reason: filtering afterwards would leave blocked tools + * discoverable through `tool_search` and callable through `execute_tool`, which + * is every tool the agent's owner switched off. + * + * WITH NOTHING TO DEFER THERE IS NO SCAFFOLDING. An agent allowed only core + * tools (or none at all) gets its tools and an empty discovery prompt, rather + * than a `tool_search` over an empty catalog and an `execute_tool` that can only + * refuse. That is `applyToolExposureMode`'s own rule and it is the honest one: + * two tools whose every call fails are worse than two tools absent. + * + * The skill catalog handed to `tool_search` is the BUILT-IN one only. The text + * routes also merge in per-viewer user/drive commands, which are volatile by + * nature — they change whenever anyone edits a command — and a call's + * instructions are sent once at socket open, so a snapshot of them would go + * stale mid-call with no way to correct it. */ -export function buildRealtimeTools( +export function buildRealtimeToolExposure( tools: ToolSet, allowlist: ToolAllowlist = null, -): readonly RealtimeTool[] { - return Object.entries(buildRealtimeToolSet(tools, allowlist)).map(([name, tool]) => - toRealtimeTool(name, tool), - ); +): RealtimeToolExposure { + const allowed = filterToolsForAgentAllowlist( + tools, + allowlist === null ? null : [...allowlist], + ) as ToolSet; + + const allowedToolNames = Object.keys(allowed); + + // The searchable skills are derived from the same pre-split names and handed + // straight to the exposure. Computed here rather than accepted as a parameter: + // it is a pure function of those names, and a parameter no caller passed is + // exactly how `tool_search` ended up with a tools-only corpus while the prompt + // advertised a skill catalog. + return { + ...applyToolExposureMode( + allowed, + 'search', + NO_COMPOSER_OVERRIDES, + listEligibleSkills(allowedToolNames), + ), + allowedToolNames, + }; +} + +/** + * Project an exposure's tool set onto the definitions sent with the session. + * + * Takes the ALREADY-EXPOSED set rather than a registry and an allowlist, + * because its one caller has the exposure in hand and the prompt describing + * those same tools comes out of it: re-deriving the exposure here would put the + * advertised list and the text describing it back on separate computations. + * + * Only each tool's name/description/parameters are read; wiring their `execute` + * to the live call is the caller's job. + */ +export function toRealtimeTools(tools: ToolSet): readonly RealtimeTool[] { + return Object.entries(tools).map(([name, tool]) => toRealtimeTool(name, tool)); } /** * The EXECUTABLE set behind those definitions — the same objects, before the * projection onto the wire shape. * - * Split out from `buildRealtimeTools` so that what the session ADVERTISES and - * what the dispatcher RUNS are one expression evaluated twice, not two lists - * that agree today. They are built in different processes (definitions ride the + * Split out from the exposure's own projection so that what the session + * ADVERTISES and what the dispatcher RUNS are one expression evaluated twice, + * not two lists that agree today. They are built in different processes (definitions ride the * attach payload to `apps/realtime`; execution happens back here on the bridge), * which is exactly the arrangement where two hand-kept lists drift — the model * would call a name nothing answers, and the call would hang on a tool result @@ -136,29 +217,10 @@ export function buildRealtimeTools( * reaches a non-core tool goes through the SAME allowlist re-check and the SAME * `safeParse` the text stack uses. * - * THE AGENT'S ALLOWLIST IS APPLIED FIRST, before the exposure split and before - * `tool_search` is handed its catalog — the same order `page-chat-turn.ts` - * uses, and for the same reason: filtering afterwards would leave blocked tools - * discoverable through `tool_search` and callable through `execute_tool`, which - * is every tool the agent's owner switched off. `tool_search` therefore also - * gets the FILTERED set here rather than the whole registry, so it cannot - * describe a tool the model is not allowed to reach. - * - * `tool_search` and `execute_tool` are added AFTER the filter and are never - * subject to it. They are the scaffolding that makes the allowlist reachable at - * all, not capabilities in their own right — and `execute_tool` re-checks the - * allowlist internally anyway, over the already-filtered deferred half. + * The discovery prompt half of the exposure is dropped here on purpose — the + * dispatcher runs tools, it does not prompt. `binding-loader.ts` takes the + * exposure whole. */ export function buildRealtimeToolSet(tools: ToolSet, allowlist: ToolAllowlist = null): ToolSet { - const allowed = filterToolsForAgentAllowlist( - tools, - allowlist === null ? null : [...allowlist], - ) as ToolSet; - const { coreTools, nonCoreTools } = splitToolsForExposure(allowed); - - return { - ...coreTools, - tool_search: createToolSearchTool(allowed), - execute_tool: createExecuteTool(nonCoreTools), - }; + return buildRealtimeToolExposure(tools, allowlist).tools; } diff --git a/apps/web/src/lib/ai/realtime/voice-runtime-deps.ts b/apps/web/src/lib/ai/realtime/voice-runtime-deps.ts index 97f82187d7..8b3120cba4 100644 --- a/apps/web/src/lib/ai/realtime/voice-runtime-deps.ts +++ b/apps/web/src/lib/ai/realtime/voice-runtime-deps.ts @@ -28,7 +28,12 @@ import { eq } from '@pagespace/db/operators'; import { pages } from '@pagespace/db/schema/core'; import { conversations } from '@pagespace/db/schema/conversations'; import { buildPageSpaceTools } from '@/lib/ai/core/ai-tools'; +import { getAgentMemoryContext, buildAgentMemorySection } from '@/lib/ai/core/agent-memory'; +import { buildActivePlanPrompt, getActivePlan } from '@/lib/ai/core/plan-binding'; +import { getUserPersonalization } from '@/lib/ai/core/personalization-utils'; +import { canPrincipalViewPage, type AuthResult } from '@/lib/auth'; import { buildRealtimeToolSet, type ToolAllowlist } from './tools'; +import { buildVoiceCallContext, type VoiceSystemContextDeps } from './system-context'; import type { BindingLoaderDeps, SeedConversation, AgentPage } from './binding-loader'; import type { TranscriptPersistenceDeps, @@ -118,7 +123,33 @@ export const activeConversationGuard = return row?.isActive === true; }; -export const voiceBindingDeps: BindingLoaderDeps = { +/** + * The live reads behind the call's system prompt. + * + * A FACTORY, taking the caller's auth principal, because the active-plan lookup + * needs a principal-aware page check: a plan can be bound to a page in ANOTHER + * drive, and a user-level check would leak that page's title and id to a token + * that may not reach it. The typed surface makes the same distinction at + * `page-chat-turn.ts`, for the same reason. + */ +export const voiceSystemContextDeps = (auth: AuthResult): VoiceSystemContextDeps => ({ + // Built per request, not at module load: `buildPageSpaceTools` branches on the + // code-execution kill switch, so a module-level constant would pin whatever + // the environment looked like at import time. + buildTools: () => buildPageSpaceTools(), + loadAgentMemory: async (pageId, userId) => + buildAgentMemorySection(await getAgentMemoryContext(pageId, userId)), + loadActivePlan: async (conversationId, userId) => + buildActivePlanPrompt( + await getActivePlan(conversationId, userId, (pageId) => + canPrincipalViewPage(auth, pageId), + ), + ), + loadPersonalization: (userId) => getUserPersonalization(userId), + logger: loggers.ai, +}); + +export const voiceBindingDeps = (auth: AuthResult): BindingLoaderDeps => ({ loadConversation, canAccess: (userId, conversation) => canAccessConversation(userId, conversation), // Streaming placeholders stay excluded (the default): an empty mid-flight row @@ -126,8 +157,10 @@ export const voiceBindingDeps: BindingLoaderDeps = { loadMessages: (conversationId) => messageRepository.getMessagesByConversationId(conversationId), loadAgentPage, + buildCallContext: (request) => + buildVoiceCallContext(voiceSystemContextDeps(auth), request), logger: loggers.ai, -}; +}); export const voiceTranscriptDeps: TranscriptPersistenceDeps = { loadConversation, diff --git a/apps/web/src/lib/ai/tools/tool-exposure.ts b/apps/web/src/lib/ai/tools/tool-exposure.ts index 7e27722d35..f7a882becc 100644 --- a/apps/web/src/lib/ai/tools/tool-exposure.ts +++ b/apps/web/src/lib/ai/tools/tool-exposure.ts @@ -114,9 +114,9 @@ export function applyToolExposureMode( mode: ToolExposureMode, alwaysUpfront: ReadonlySet = new Set(), searchableSkills: readonly SkillSearchEntry[] = [], -): { tools: ToolSet; toolDiscoveryPrompt: string } { +): { tools: ToolSet; toolDiscoveryPrompt: string; nonCoreToolNames: string } { if (mode !== 'search') { - return { tools, toolDiscoveryPrompt: '' }; + return { tools, toolDiscoveryPrompt: '', nonCoreToolNames: '' }; } // Tools forced upfront (e.g. the web_search runtime override) are excluded from @@ -128,7 +128,7 @@ export function applyToolExposureMode( const { nonCoreTools } = splitToolsForExposure(tools, alwaysUpfront); if (Object.keys(nonCoreTools).length === 0) { - return { tools, toolDiscoveryPrompt: '' }; + return { tools, toolDiscoveryPrompt: '', nonCoreToolNames: '' }; } const upfrontOverrides = Object.fromEntries( @@ -145,9 +145,14 @@ export function applyToolExposureMode( execute_tool: createExecuteTool(nonCoreTools), }; + // Returned as one block AND as its two halves. Most callers want the block; + // the Global Assistant states TOOL_DISCOVERY_PROMPT early, beside the + // exploration rules that depend on it, and takes the catalog on its own + // later. Both come from this one computation so a caller assembling them + // separately still describes exactly the tools that were deferred here. const nonCoreNamesPrompt = buildNonCoreToolNamesPrompt(Object.keys(nonCoreTools)); const toolDiscoveryPrompt = '\n\n' + TOOL_DISCOVERY_PROMPT + (nonCoreNamesPrompt ? '\n\n' + nonCoreNamesPrompt : ''); - return { tools: searchTools, toolDiscoveryPrompt }; + return { tools: searchTools, toolDiscoveryPrompt, nonCoreToolNames: nonCoreNamesPrompt }; } diff --git a/docs/2.0-architecture/agent-sessions.md b/docs/2.0-architecture/agent-sessions.md index bf440a89a2..ab70fcad12 100644 --- a/docs/2.0-architecture/agent-sessions.md +++ b/docs/2.0-architecture/agent-sessions.md @@ -354,7 +354,7 @@ that drifts into double generation and double billing. **What "one pipeline" does NOT mean, and this section used to imply.** It names the ENTRY. It says nothing about the two strategy functions, and they are neither small nor DRY: `runPageChatTurn` is ~2,080 lines in one function and `runGlobalChatTurn` ~1,460, with -**165 substantive lines of 40+ characters byte-identical between them** — measured, and +**164 substantive lines of 40+ characters byte-identical between them** — measured, and clustered rather than scattered, in the epilogue (stream construction, `onFinish`, terminal persist, hold settle, telemetry), which is also where the billing settle, `releaseHold` and the exactly-once mention latch live. Two copies of the money path. That diff --git a/packages/lib/src/realtime/voice-bridge-contract.ts b/packages/lib/src/realtime/voice-bridge-contract.ts index d1b9ff660e..1c41877ca5 100644 --- a/packages/lib/src/realtime/voice-bridge-contract.ts +++ b/packages/lib/src/realtime/voice-bridge-contract.ts @@ -7,7 +7,7 @@ * validates it after the HMAC check and before touching a socket. * * WHY THE TOOLS TRAVEL IN THIS PAYLOAD. `apps/realtime` cannot build the - * realtime tool definitions itself: the conversion (`buildRealtimeTools`) reads + * realtime tool definitions itself: the conversion (`toRealtimeTools`) reads * PageSpace's AI SDK tool registry, which lives in `apps/web/src/lib/ai` behind * the `@/` alias, imports `ai`/`zod`, and has env-dependent branches. There is * no dependency edge from `apps/realtime` to `apps/web` and there must not be