diff --git a/docs/dev/chat.md b/docs/dev/chat.md index c760b7b0..fe889087 100644 --- a/docs/dev/chat.md +++ b/docs/dev/chat.md @@ -307,11 +307,11 @@ A chat turn goes: gateway records only the bare status code. - `ChatLoopHandlers` - the event surface the UI uses: text updates, tool start/done/error, persistence events, - `onToolboxesEnabledChange` (for the composer toolbox flash - when `toggle_toolbox` fires), `onGuardRetry` (a - function-side output guard discarded a junk attempt and is - re-rolling). Every handler is optional; the loop runs cleanly - with none of them. (Generated images are NOT delivered through + `onGuardRetry` (a function-side output guard discarded a junk + attempt and is re-rolling). Every handler is optional; the loop + runs cleanly with none of them. (A model-driven `toggle_toolbox` + flip has NO handler: the composer toolbox flash keys off the + `threads` realtime UPDATE echo in the drawer-refresh path.) (Generated images are NOT delivered through a handler: the function attaches them server-side per round and `GeneratedImageCard` resolves them by filename - see [./attachments.md](./attachments.md).) @@ -387,12 +387,15 @@ A chat turn goes: `runExchange` resolved from `ctx.threadId`. See `./exchange.md` for the full lifecycle and the `respondingElsewhere` / observer-side wiring. -- **Tools** - every main-chat round's `/stream` request body - carries `tools: buildToolList(thread.toolboxes_enabled)` assembled - on the browser side. The function-side `performToolCall` - dispatches each call against its own (ported) tool registry, - persists results as `role='tool'` rows, and echoes them back into - the next round's request. See `./tools.md`. +- **Tools** - the `/stream` envelope carries two tool payloads + assembled on the browser side: `tools: buildToolList( + thread.toolboxes_enabled)` (the first round's pre-filtered wire + array) and `toolCatalog: buildToolCatalog(...)` (the full + always-on + gated catalog the orchestrator rebuilds `tools` from + after a mid-turn `toggle_toolbox`). The function-side + `performToolCall` dispatches each call against its own (ported) + tool registry, persists results as `role='tool'` rows, and echoes + them back into the next round's request. See `./tools.md`. - **Memory (recall)** — `memory_recall` is a tool; the main model calls it whenever it judges prior memory context would help. The chat loop dispatches it like any other tool. See `./memory.md`. @@ -496,12 +499,19 @@ A chat turn goes: [./auto-title.md](./auto-title.md). - **`toggle_toolbox` is the only tool that mutates the round loop's gated-toolbox set in-flight.** The function-side round - loop inspects each tool's name and, when it sees - `toggle_toolbox`, applies the new toolbox-enabled set in - memory (no DB re-read) and emits a `toolboxes_enabled_change` - broadcast event so the browser can flash the composer - toolbox button. If you add another tool that also flips - thread state, it needs similar special-casing or a refetch. + loop inspects each round's outcomes and, when a `toggle_toolbox` + call succeeded, rebuilds the wire `tools` array in memory from the + envelope's `toolCatalog` (no DB re-read; see + `venice/tool_catalog.ts`), so a toolbox the model enables is + callable in the SAME turn. Without the rebuild the array stays + frozen at its envelope-POST shape, and a model backend that holds + the model to the declared tool list coerces the intended write + call onto the nearest declared name. The browser has no in-process + signal for the flip - it notices via the `threads` realtime UPDATE + echo and flashes the composer toolbox button off the row delta + (see the drawer-refresh handler in `Chat.svelte`). If you add + another tool that also flips thread state, it needs similar + special-casing or a refetch. - **The round boundary needs an explicit signal; the browser can't derive it.** The round loop runs inside the edge function now, so the browser sees only a flat stream of deltas. The live diff --git a/docs/dev/mcp-integrations.md b/docs/dev/mcp-integrations.md index 8f26ebcf..24782991 100644 --- a/docs/dev/mcp-integrations.md +++ b/docs/dev/mcp-integrations.md @@ -220,7 +220,11 @@ through them. on `buildToolList`. MCP server catalogs can be large (Fastmail's 10 tools). Per-toolbox enablement gates which schemas are armed; gated MCP toolboxes follow the same - shape as built-in toolboxes. + shape as built-in toolboxes. (The /stream envelope separately + carries the FULL catalog - disabled MCP boxes included - via + `buildToolCatalog` for mid-turn toolbox rearming; that is + browser-to-function POST weight only, never Venice-wire or + model-context cost. See `tools.md`.) - **Edge dispatch -** `performToolCall` in `supabase/functions/venice/performToolCall.ts` has a module-load registry populated by diff --git a/docs/dev/prompt-augmentation.md b/docs/dev/prompt-augmentation.md index 406e9d5d..8a162afe 100644 --- a/docs/dev/prompt-augmentation.md +++ b/docs/dev/prompt-augmentation.md @@ -65,7 +65,7 @@ Two distinct injection surfaces: | Samskara compound | `` (row 5) | `getCompoundSummary` (`venice/priming/samskara.ts`) | cached prose row | always-on; no fuse | | Samskara fire | `` (row 6) | `fireSamskaras` (`venice/priming/samskara.ts`) | computed per turn | raced against `SAMSKARA_PRIMING_TIMEOUT_MS` | | Intuition | `` (row 7) | `runIntuitionPipeline` (`venice/priming/intuition.ts`) | `threads.intuition_payload` | `isPayloadFreshForInjection` (STALE_FUSE_MS) | -| Tool catalog | system (row 1) | `buildSystemPrompt` / `buildToolList` (`src/lib/tools`) | n/a (derived from enabled toolboxes) | per-turn snapshot of `toolboxes_enabled` | +| Tool catalog | system (row 1) | `buildSystemPrompt` / `buildToolList` (`src/lib/tools`) | n/a (derived from enabled toolboxes) | per-turn snapshot of `toolboxes_enabled`; the wire `tools` array additionally rearms mid-turn after a `toggle_toolbox` (see `tools.md`) | | Metadata block | system (row 8) | `buildMetadataSystemMessage` (`src/lib/chat/prompt-assembly`) | n/a | rebuilt every turn | ## Ordering diff --git a/docs/dev/tools.md b/docs/dev/tools.md index 4265fbd9..3a87c0e6 100644 --- a/docs/dev/tools.md +++ b/docs/dev/tools.md @@ -278,11 +278,14 @@ Edge dispatch (`supabase/functions/venice/`): - **Chat loop** - `chat/loop.ts` calls `buildToolList(thread.toolboxes_enabled)` to ship the wire - `tools` array, then observes the streamed `tool_call_request` / - `tool_call_response` events. The edge function is - writer-of-record for the whole turn: it dispatches each call via - `performToolCall` and persists the assistant-with-tool-calls row - and the per-call `role='tool'` rows. See `./chat.md`. + `tools` array plus `buildToolCatalog(...)` to ship the full + catalog in the /stream envelope, then observes the streamed + `tool_call_request` / `tool_call_response` events. The edge + function is writer-of-record for the whole turn: it dispatches + each call via `performToolCall`, persists the + assistant-with-tool-calls row and the per-call `role='tool'` + rows, and rebuilds its in-memory `tools` array from the catalog + when a round's `toggle_toolbox` succeeds. See `./chat.md`. - **Background agents** - server-side only. Each agent composes its own prompt and toolbox and calls `runHeadlessAgent`, which drives model -> tool -> model rounds entirely in memory (no DB writes, @@ -389,6 +392,19 @@ Edge dispatch (`supabase/functions/venice/`): ignored; duplicates across toolboxes are deduped by tool name (first-seen wins). Callers should never construct this array by hand. +- `buildToolCatalog(mcpToolboxes?): ToolCatalog` - the full catalog + for the /stream envelope: always-on defs plus every gated + toolbox's wire defs keyed by toolbox name (static boxes in + `TOOLBOXES` order, then MCP boxes). The venice orchestrator + rebuilds the request's `tools` array from it after a successful + mid-turn `toggle_toolbox` (`buildToolsFromCatalog` in + `supabase/functions/venice/tool_catalog.ts`), so a toolbox the + model enables is callable in the same turn. Key order is + load-bearing (the server iterates insertion order); + `tests/tool-catalog-parity.test.ts` pins that a rebuild equals + `buildToolList` for the same enabled set. The server module is + deliberately dependency-free so the vitest parity suite can + import it directly. - `buildSystemPrompt(opts?)` / `buildToolboxStateBlock(enabled)` - live in `src/lib/chat/system-prompt.ts`, importing the registry from here. The baseline is state-free; the state block renders the @@ -455,10 +471,11 @@ Edge dispatch (`supabase/functions/venice/`): ## Interactions with other features - **Chat** - `buildToolList(thread.toolboxes_enabled)` shapes the - wire catalog; the edge function dispatches and persists; - `onToolboxesEnabledChange` fires whenever the model flips the - thread's enabled set so the UI can patch its local thread row - without a refetch. See `./chat.md`. + first round's wire array and `buildToolCatalog` rides the same + envelope for the mid-turn rebuild; the edge function dispatches + and persists. The browser notices a model-driven toolbox flip via + the `threads` realtime UPDATE echo (the composer button flash + keys off the row delta). See `./chat.md`. - **Attachments** - `generate_image` (gated `images` toolbox) is the one tool whose output bypasses the tool-result content entirely: the edge orchestrator harvests its generated bytes and @@ -550,6 +567,22 @@ Edge dispatch (`supabase/functions/venice/`): always-on catalog block by `toggleToolbox.name`; the toggle rule is explained in its own prompt paragraph with the exact call shape (`toggle_toolbox({enabled: [...]})`). +- **Enablement is request-shape only; dispatch does not re-check the + gate.** `performToolCall` runs whatever registered name the model + emits - the catalog filter (`buildToolList` + the mid-turn rebuild + from `toolCatalog`) is the whole gate. Two consequences. First, + the mid-turn rebuild is load-bearing: before it existed, a + toolbox enabled by `toggle_toolbox` shipped no schemas for the + rest of the turn, and whether the flow still worked depended on + the serving backend - most accept calls to undeclared tool names, + but a backend that holds the model to the declared list coerces + the intended write onto the nearest declared name (observed as + `followup_create` coming out as `followup_list` eight rounds in a + row while the activity text said "Creating a follow-up..."). + Second, a model that hallucinates an undeclared-but-registered + write call will still execute it; the gate assumes models call + only declared tools, and history shows most do not - treat that + leniency as load-bearing legacy, not a guarantee. - **Unknown toolbox names are dropped silently.** Both writers (`toggle_toolbox` and the composer popover) filter against `GATED_TOOLBOX_NAMES`, so a renamed or deleted toolbox doesn't diff --git a/docs/qa/use-cases/toolbox-midturn-enable.md b/docs/qa/use-cases/toolbox-midturn-enable.md new file mode 100644 index 00000000..00f1a68f --- /dev/null +++ b/docs/qa/use-cases/toolbox-midturn-enable.md @@ -0,0 +1,91 @@ +# Toolbox gating: mid-turn enable arms the write tools in the same turn + +> Backfilled alongside the mid-turn rearm fix. The pre-fix baseline +> is the production incident this case reproduces (thread "Baking +> Mishap with Joy's Loaf", 2026-08-01): eight `followup_list` calls +> whose activity text said "Creating a follow-up...", after two +> successful toggles. Not yet executed against the fixed code - run +> it and start the results log. + +## Covers + +The toggle-then-write flow inside a single assistant turn +([dev: tools](../../dev/tools.md), "Enablement is request-shape +only" gotcha; [dev: chat](../../dev/chat.md), the `toggle_toolbox` +gotcha): + +- **Mid-turn rearm** - when the model calls `toggle_toolbox` and + then a write tool in the SAME turn, the orchestrator rebuilds the + wire `tools` array from the envelope's `toolCatalog` + (`supabase/functions/venice/tool_catalog.ts`), so the write is + declared to the model on the very next round. +- **Cross-model robustness** - the flow must not depend on the + serving backend accepting calls to undeclared tools. A strict + backend is the case that exposed the gap. + +## Preconditions + +- Local stack up (`mise run dev-start`), signed in as the dev user + (`dev@nak.local` / `devpass123`). +- A fresh thread with EVERY gated toolbox off (composer toolbox + popover shows none enabled) - the point is to force the model + through the toggle. +- Logs drawer open at Info, source filter on `stream` - the rearm + logs a line per rebuild. +- If reproducing the original incident shape, pick a model known to + hold to the declared tool list (the incident model was + `deepseek-v4-flash-0731`); any model exercises the rearm path + itself. + +## Steps + +1. Send a message that makes a follow-up save the obvious next + move, e.g.: "I'm baking a loaf for a friend tomorrow - remind me + later to tell you how it went." +2. Watch the tool-call cards on the assistant turn as they land. +3. In the Logs drawer (source `stream`), find the round lines for + this turn. +4. Check the row landed: + + ```sql + select question, status, relevant_after + from public.followups + where user_id = (select id from auth.users + where email = 'dev@nak.local') + order by created_at desc limit 3; + ``` + +5. Repeat the shape with a different gated box in the same style, + e.g. "Save this recipe: ..." with `cooking` off, and confirm the + same single-turn toggle-then-write pattern. + +## Expected + +- Step 2: the turn shows a `toggle_toolbox` call (activity names + the enable) followed IN THE SAME TURN by a `followup_create` + call - not `followup_list` repurposed with a "creating" activity + line, and not a stall. A preliminary `followup_list` dedup check + before the create is fine. +- Step 3: after the round with the successful toggle, a + `rearmed tools for [followups]` line with a def count larger + than the turn's opening `toolsLen`. +- Step 4: exactly one new open follow-up row for the loaf. +- Step 5: same shape for the other box (`toggle_toolbox` -> + `recipe_save`, one new recipe row). + +## Cleanup + +```sql +delete from public.followups + where user_id = (select id from auth.users + where email = 'dev@nak.local'); +``` + +Delete any test recipe from the Cookbook screen, and the test +threads from the drawer. + +## Results log + +| Date | Env | Commit | Result | Notes | +|---|---|---|---|---| +| - | - | - | - | Fix landed with this case; no runs yet. | diff --git a/src/lib/chat/loop.ts b/src/lib/chat/loop.ts index fd39b4da..2fcb9352 100644 --- a/src/lib/chat/loop.ts +++ b/src/lib/chat/loop.ts @@ -60,7 +60,7 @@ */ import type { VeniceMessage } from '../venice'; -import { buildToolList } from '../tools'; +import { buildToolList, buildToolCatalog } from '../tools'; import { buildSystemPrompt } from './system-prompt'; import { recordSubstrateStub } from '../samskara'; import { createLogger } from '../logger.svelte'; @@ -107,11 +107,13 @@ export async function runChatLoop(opts: ChatLoopOptions): Promise = {}; + for (const tb of GATED_TOOLBOXES) { + gated[tb.name] = tb.tools.map(toOpenAIToolDef); + } + for (const tb of mcpToolboxes) { + gated[tb.name] = tb.tools.map(toOpenAIToolDef); + } + return { + alwaysOn: alwaysOnToolbox.tools.map(toOpenAIToolDef), + gated, + }; +} + /** * Look up the optional pretty-formatter overrides a tool may * declare on its schema. Used by the tool-call detail panel @@ -668,5 +699,5 @@ export { toOpenAIToolDef }; // `toggleToolbox` is read by chat/system-prompt.ts for its `.name`; re-exported // for that one consumer. export { toggleToolbox }; -export type { ToolDef, OpenAIToolDef, ToolContext, ToolResult, Toolbox } from './types'; +export type { ToolDef, OpenAIToolDef, ToolContext, ToolResult, Toolbox, ToolCatalog } from './types'; export type { OpenAIToolCall } from './types'; diff --git a/src/lib/tools/types.ts b/src/lib/tools/types.ts index 2bec0bd1..d18d6f13 100644 --- a/src/lib/tools/types.ts +++ b/src/lib/tools/types.ts @@ -89,6 +89,24 @@ export interface OpenAIToolDef { }; } +/** + * The full tool catalog the chat envelope ships alongside the + * pre-filtered wire `tools` array: always-on defs plus every gated + * toolbox's defs keyed by toolbox name (static boxes in TOOLBOXES + * order, then MCP integration boxes - key order is the order the + * server rebuilds in). The venice edge function rebuilds `tools` + * from this after a mid-turn toggle_toolbox, so a toolbox the model + * enables becomes callable in the same turn. Built by + * `buildToolCatalog` in `./index.ts`; the server-side consumer keeps + * its own structural mirror of this shape + * (supabase/functions/venice/tool_catalog.ts) - same two-interfaces + * posture as ToolContext. + */ +export interface ToolCatalog { + alwaysOn: OpenAIToolDef[]; + gated: Record; +} + /** * OpenAI / Venice wire shape for one item in `choices[0].message.tool_calls`. * `arguments` is a JSON-encoded string (not a parsed object) — the model diff --git a/src/lib/venice.ts b/src/lib/venice.ts index e3b7c91e..4c90e70f 100644 --- a/src/lib/venice.ts +++ b/src/lib/venice.ts @@ -42,7 +42,7 @@ import type { SupabaseClient } from '@supabase/supabase-js'; import type { ReasoningEffort, Verbosity } from './models'; -import type { OpenAIToolDef, OpenAIToolCall } from './tools/types'; +import type { OpenAIToolDef, OpenAIToolCall, ToolCatalog } from './tools/types'; // Priming payload types + coercers. The server publishes the fresh // intuition / context-recall caches over the stream channel as the // priming stage runs; the decode below coerces the raw JSON through @@ -197,6 +197,15 @@ export interface ChatRequest { * may emit `tool_calls` events instead of (or in addition to) text. */ tools?: OpenAIToolDef[]; + /** + * The full tool catalog (always-on + every gated toolbox), for the + * streaming-root path only. Rides the /stream envelope beside the + * wire body so the server-side round chain can rebuild `tools` when + * the model toggles a toolbox mid-turn; `tools` above stays the + * pre-filtered array the first round runs with. Ignored on the + * direct-Venice fallback path, which has no round chain to rearm. + */ + toolCatalog?: ToolCatalog; /** * When set, populates `venice_parameters.enable_web_search` on the * request body. Omitted → field is not sent (Venice's server-side @@ -1169,6 +1178,10 @@ async function* streamChatViaFunction( // when present so a caller that does no priming (sub-completion // paths never reach here, but be explicit) keeps the wire lean. ...(ctx.priming ? { priming: ctx.priming } : {}), + // Full tool catalog for mid-turn toolbox rearming (see + // ChatRequest.toolCatalog). Only the chat loop sets it; + // omitted otherwise so the envelope stays lean. + ...(req.toolCatalog ? { toolCatalog: req.toolCatalog } : {}), body, }, }, diff --git a/src/screens/Chat.svelte b/src/screens/Chat.svelte index cf281c2a..82f940f3 100644 --- a/src/screens/Chat.svelte +++ b/src/screens/Chat.svelte @@ -1877,9 +1877,8 @@ prevTopics.some((p, i) => p !== nextTopics[i]); // If the toolboxes_enabled column changed on the active // thread, drive the composer's brief flash so a human eye - // notices the LLM-initiated state flip. Under streaming-root - // the in-process onToolboxesEnabledChange handler is dead - // (tools execute server-side), so the realtime UPDATE echo is + // notices the LLM-initiated state flip. toggle_toolbox + // executes server-side, so the realtime UPDATE echo is // the only signal we have here. User-initiated flips path // through setToolboxEnabled, which patches the local thread // row optimistically before the realtime UPDATE arrives - so @@ -4335,18 +4334,6 @@ }); } }, - onToolboxesEnabledChange: (enabled) => { - patchThread(ctx.threadId, { - toolboxes_enabled: [...enabled], - }); - // Brief flash on the composer toolbox so a human eye - // notices the LLM-initiated state flip. User-initiated - // flips don't flash (the click itself is the feedback). - toolboxFlash = true; - setTimeout(() => { - toolboxFlash = false; - }, 600); - }, onIntuitionUpdate: (payload: IntuitionPayload) => { // Patch the in-memory thread row so the modal and any // inline indicator pick up the fresh perception/drives/ diff --git a/supabase/functions/tests/tool-catalog.test.ts b/supabase/functions/tests/tool-catalog.test.ts new file mode 100644 index 00000000..c5190558 --- /dev/null +++ b/supabase/functions/tests/tool-catalog.test.ts @@ -0,0 +1,100 @@ +// Offline coverage for the mid-turn toolbox rearm helpers +// (venice/tool_catalog.ts): the envelope boundary coercion, the +// tools-array rebuild the orchestrator runs after a successful +// toggle_toolbox, and the toggle-result extraction that feeds it. +// Parity between a rebuild and the browser's buildToolList lives in +// the vitest suite (tests/tool-catalog-parity.test.ts), which can +// import both sides; these tests pin the server-local semantics. +import { assertEquals } from '@std/assert'; +import { + buildToolsFromCatalog, + coerceToolCatalog, + enabledSetFromToggleResult, + type ToolCatalog, +} from '../venice/tool_catalog.ts'; + +function def(name: string): unknown { + return { type: 'function', function: { name, description: name, parameters: {} } }; +} + +const CATALOG: ToolCatalog = { + alwaysOn: [def('followup_list'), def('web_search')], + gated: { + followups: [def('followup_create'), def('followup_close')], + cooking: [def('recipe_save')], + 'mcp:abc123': [def('mcp:abc123:send_email')], + }, +}; + +Deno.test('coerceToolCatalog accepts the browser shape', () => { + const raw = JSON.parse(JSON.stringify(CATALOG)); + const coerced = coerceToolCatalog(raw); + assertEquals(coerced, CATALOG); +}); + +Deno.test('coerceToolCatalog rejects malformed shapes with null', () => { + assertEquals(coerceToolCatalog(undefined), null); + assertEquals(coerceToolCatalog(null), null); + assertEquals(coerceToolCatalog('nope'), null); + assertEquals(coerceToolCatalog([]), null); + assertEquals(coerceToolCatalog({}), null); + assertEquals(coerceToolCatalog({ alwaysOn: [], gated: [] }), null); + assertEquals(coerceToolCatalog({ alwaysOn: [], gated: { a: 'x' } }), null); + assertEquals(coerceToolCatalog({ alwaysOn: {}, gated: {} }), null); +}); + +Deno.test('buildToolsFromCatalog: always-on only when nothing enabled', () => { + const tools = buildToolsFromCatalog(CATALOG, []); + assertEquals(tools, CATALOG.alwaysOn); +}); + +Deno.test('buildToolsFromCatalog: enabled boxes append in catalog order', () => { + // Enabled order in the toggle result does not matter - catalog key + // order wins, matching buildToolList's TOOLBOXES iteration. + const tools = buildToolsFromCatalog(CATALOG, ['cooking', 'followups']); + assertEquals(tools, [ + ...CATALOG.alwaysOn, + ...CATALOG.gated.followups, + ...CATALOG.gated.cooking, + ]); +}); + +Deno.test('buildToolsFromCatalog: unknown enabled names are ignored', () => { + const tools = buildToolsFromCatalog(CATALOG, ['no_such_box', 'followups']); + assertEquals(tools, [...CATALOG.alwaysOn, ...CATALOG.gated.followups]); +}); + +Deno.test('buildToolsFromCatalog: mcp boxes rebuild like static ones', () => { + const tools = buildToolsFromCatalog(CATALOG, ['mcp:abc123']); + assertEquals(tools, [...CATALOG.alwaysOn, ...CATALOG.gated['mcp:abc123']]); +}); + +Deno.test('buildToolsFromCatalog: duplicate names dedupe first-seen', () => { + const catalog: ToolCatalog = { + alwaysOn: [def('shared')], + gated: { box: [def('shared'), def('unique')] }, + }; + const tools = buildToolsFromCatalog(catalog, ['box']); + assertEquals(tools, [def('shared'), def('unique')]); +}); + +Deno.test('buildToolsFromCatalog: nameless defs pass through undeduped', () => { + const blob = { type: 'function' }; + const catalog: ToolCatalog = { alwaysOn: [blob, blob], gated: {} }; + assertEquals(buildToolsFromCatalog(catalog, []), [blob, blob]); +}); + +Deno.test('enabledSetFromToggleResult reads the toggle result shape', () => { + assertEquals(enabledSetFromToggleResult({ enabled: ['followups'] }), [ + 'followups', + ]); + assertEquals(enabledSetFromToggleResult({ enabled: [] }), []); +}); + +Deno.test('enabledSetFromToggleResult rejects other shapes with null', () => { + assertEquals(enabledSetFromToggleResult(null), null); + assertEquals(enabledSetFromToggleResult('enabled'), null); + assertEquals(enabledSetFromToggleResult({}), null); + assertEquals(enabledSetFromToggleResult({ enabled: 'followups' }), null); + assertEquals(enabledSetFromToggleResult({ enabled: [1] }), null); +}); diff --git a/supabase/functions/venice/getStreamingResponse.ts b/supabase/functions/venice/getStreamingResponse.ts index 67a52650..f330bfb8 100644 --- a/supabase/functions/venice/getStreamingResponse.ts +++ b/supabase/functions/venice/getStreamingResponse.ts @@ -80,6 +80,11 @@ import { samskaraOnTurnTail } from './agents/samskara.ts'; import { secondThoughtsOnTurnTail } from './agents/second_thoughts.ts'; import { createEdgeLogger } from '../_shared/edge-log.ts'; import { runServerPriming, type PrimingInputs } from './priming.ts'; +import { + buildToolsFromCatalog, + enabledSetFromToggleResult, + type ToolCatalog, +} from './tool_catalog.ts'; // Magic flag the ask_user tool returns to suspend the round chain // pending a user answer. Mirrors src/lib/tools/ask_user.ts' @@ -181,9 +186,20 @@ export interface OrchestratorOpts { /** * Full Venice wire body for the first round. Already shaped by the * browser via buildChatBody; the orchestrator copies it round-to- - * round, mutating only `messages` between rounds. + * round, mutating only `messages` between rounds - plus `tools`, + * rebuilt from `toolCatalog` after a successful toggle_toolbox. */ bodyTemplate: VeniceWireBody; + /** + * The full tool catalog (always-on defs + every gated toolbox's + * defs), shipped by the browser alongside the pre-filtered + * bodyTemplate.tools. Lets the round chain rearm `body.tools` the + * moment the model enables a toolbox mid-turn, instead of the new + * box's tools staying undeclared until the next envelope POST. + * Absent (older browser build, malformed field) degrades to the + * frozen-array behavior. + */ + toolCatalog?: ToolCatalog; /** Admin Supabase client (service role) for DB writes and Realtime. */ adminClient: SupabaseClient; /** @@ -700,6 +716,36 @@ export async function getStreamingResponse( } } + // Rearm the wire tools array after a successful toggle_toolbox. + // The browser filtered bodyTemplate.tools against the toolbox + // state at envelope-POST time; without this rebuild, a toolbox + // the model enables mid-turn ships no tool schemas for the rest + // of the turn, and a backend that holds the model to the + // declared list coerces the intended write call onto the nearest + // declared name (observed as followup_create coming out as + // followup_list, repeatedly, right after a successful toggle). + // Last successful toggle wins - the tool replaces the whole set. + // Guarded on Array.isArray(body.tools): a turn that shipped no + // tools cannot have dispatched a toggle, so a missing array here + // means the field was deliberately stripped and must stay off + // the wire. + if (opts.toolCatalog && Array.isArray(body.tools)) { + for (let i = outcomes.length - 1; i >= 0; i -= 1) { + const o = outcomes[i]; + if (!o.ok || o.request.name !== 'toggle_toolbox') continue; + const enabled = enabledSetFromToggleResult(o.result); + if (enabled === null) break; + body = { + ...body, + tools: buildToolsFromCatalog(opts.toolCatalog, enabled), + }; + log.info( + `${runId} round ${round} rearmed tools for [${enabled.join(', ')}]: ${(body.tools as unknown[]).length} defs`, + ); + break; + } + } + // Harvest generated-image payloads off any tool result before // persistRoundToolResults strips them at encode time. Failed // outcomes carry the error shape, not a generated image, so we diff --git a/supabase/functions/venice/index.ts b/supabase/functions/venice/index.ts index ecccf7c6..81a4a299 100644 --- a/supabase/functions/venice/index.ts +++ b/supabase/functions/venice/index.ts @@ -59,6 +59,7 @@ import { type ModelPriceCaps, } from '../_shared/price-cap.ts'; import { getStreamingResponse } from './getStreamingResponse.ts'; +import { coerceToolCatalog } from './tool_catalog.ts'; import { retryWikiThread, runWikiSweepTick } from './agents/wiki.ts'; import { runDigestSweepTick } from './agents/digest.ts'; import { runWikiManualUpdate } from './agents/wiki_manual.ts'; @@ -1003,6 +1004,14 @@ interface StreamRequestBody { * picks it up. */ body?: Record; + /** + * The full tool catalog (always-on defs + gated toolboxes keyed by + * name), built by buildToolCatalog() (src/lib/tools/index.ts). + * Coerced through coerceToolCatalog and forwarded so the round + * chain can rearm `tools` after a mid-turn toggle_toolbox. Optional: + * an envelope without it keeps the turn's tools array frozen. + */ + toolCatalog?: unknown; /** * Reconnect-only flag. When true, the function does NOT start a new * completion - it returns the existing in-flight envelope (or a @@ -1334,6 +1343,9 @@ async function handleStreamFresh( (id): id is string => typeof id === 'string' && UUID_RE.test(id), ) : []; + // Malformed or absent catalogs coerce to null and are simply not + // forwarded - the turn degrades to a frozen tools array, never a 400. + const toolCatalog = coerceToolCatalog(body.toolCatalog); const promise = getStreamingResponse({ apiKey, threadId: body.threadId, @@ -1343,6 +1355,7 @@ async function handleStreamFresh( bodyTemplate: body.body as Record, adminClient: ctx.admin, priming: body.priming, + ...(toolCatalog !== null ? { toolCatalog } : {}), }); edgeWaitUntil(promise); diff --git a/supabase/functions/venice/tool_catalog.ts b/supabase/functions/venice/tool_catalog.ts new file mode 100644 index 00000000..825e3ec4 --- /dev/null +++ b/supabase/functions/venice/tool_catalog.ts @@ -0,0 +1,125 @@ +// Tool-catalog rearming for mid-turn toolbox toggles. +// +// The browser composes the turn's wire `tools` array once, at envelope- +// POST time, from the thread's toolboxes_enabled. The orchestrator's +// round chain reuses that array for every round - so when the model +// calls toggle_toolbox mid-turn, the newly enabled toolbox's write +// tools were absent from the remaining rounds' wire. Most model +// backends papered over the gap by accepting calls to tools the +// request never declared; a backend that holds the model to the +// declared list turns the same flow into a visible failure (the model +// tries to call a write it cannot name, and the call comes out as the +// nearest declared tool instead). +// +// The fix: the envelope now also carries the FULL tool catalog - +// always-on defs plus every gated toolbox's defs keyed by toolbox +// name - and the orchestrator rebuilds `body.tools` from it whenever a +// round's toggle_toolbox call succeeds. The browser stays the single +// source of truth for what tools exist (this module never defines a +// tool); the server only filters and dedupes what it was shipped. +// +// This module is deliberately dependency-free pure functions: +// tests/tool-catalog-parity.test.ts (vitest) imports it alongside the +// browser registry to pin that a rebuild from the catalog reproduces +// buildToolList byte-for-byte, and the Deno unit tests +// (supabase/functions/tests/tool-catalog.test.ts) cover the boundary +// coercion. Keep it import-free so both runtimes can load it. + +/** + * The envelope's catalog shape. Tool defs are opaque to the server - + * they are already wire-shaped by the browser (toOpenAIToolDef, + * activity param included) and go onto `body.tools` verbatim. The + * only field the server reads inside a def is `function.name`, for + * dedupe. + * + * `gated` key order is meaningful: the browser writes boxes in + * catalog order (static TOOLBOXES order, then MCP integrations), and + * the rebuild iterates keys in insertion order so the rebuilt array + * matches what buildToolList would have produced for the same + * enabled set. + */ +export interface ToolCatalog { + alwaysOn: unknown[]; + gated: Record; +} + +/** + * Boundary check for the envelope's optional toolCatalog field. + * Returns null on anything malformed rather than throwing - an + * envelope without a usable catalog degrades to the pre-catalog + * behavior (the turn's tools array stays frozen), never a failed + * turn. Older browser builds that predate the field land here as + * undefined and get the same graceful null. + */ +export function coerceToolCatalog(raw: unknown): ToolCatalog | null { + if (!raw || typeof raw !== 'object' || Array.isArray(raw)) return null; + const obj = raw as Record; + if (!Array.isArray(obj.alwaysOn)) return null; + const gatedRaw = obj.gated; + if (!gatedRaw || typeof gatedRaw !== 'object' || Array.isArray(gatedRaw)) { + return null; + } + const gated: Record = {}; + for (const [name, defs] of Object.entries(gatedRaw as Record)) { + if (!Array.isArray(defs)) return null; + gated[name] = defs; + } + return { alwaysOn: obj.alwaysOn, gated }; +} + +/** The wire name of a tool def, when it has one. */ +function wireName(def: unknown): string | null { + if (!def || typeof def !== 'object') return null; + const fn = (def as Record).function; + if (!fn || typeof fn !== 'object') return null; + const name = (fn as Record).name; + return typeof name === 'string' ? name : null; +} + +/** + * Rebuild the wire `tools` array from the catalog and an enabled- + * toolbox-name set. Mirrors buildToolList's semantics + * (src/lib/tools/index.ts): always-on first, then each enabled gated + * toolbox in catalog order; unknown enabled names are ignored; + * duplicate tool names across boxes dedupe first-seen. A def the + * browser shipped without a readable name is passed through undeduped + * rather than dropped - the server has no business discarding catalog + * content it merely can't index. + */ +export function buildToolsFromCatalog( + catalog: ToolCatalog, + enabled: readonly string[], +): unknown[] { + const enabledSet = new Set(enabled); + const seen = new Set(); + const out: unknown[] = []; + const push = (def: unknown) => { + const name = wireName(def); + if (name !== null) { + if (seen.has(name)) return; + seen.add(name); + } + out.push(def); + }; + for (const def of catalog.alwaysOn) push(def); + for (const [boxName, defs] of Object.entries(catalog.gated)) { + if (!enabledSet.has(boxName)) continue; + for (const def of defs) push(def); + } + return out; +} + +/** + * Extract the accepted enabled-set from a toggle_toolbox result + * (`{enabled: string[]}`). Null on any other shape so a malformed + * result skips the rearm instead of arming an empty set - the toggle + * already persisted whatever it persisted, and the next turn's + * envelope rebuilds from the thread row regardless. + */ +export function enabledSetFromToggleResult(result: unknown): string[] | null { + if (!result || typeof result !== 'object') return null; + const enabled = (result as Record).enabled; + if (!Array.isArray(enabled)) return null; + if (!enabled.every((n): n is string => typeof n === 'string')) return null; + return enabled; +} diff --git a/tests/tool-catalog-parity.test.ts b/tests/tool-catalog-parity.test.ts new file mode 100644 index 00000000..dd26da11 --- /dev/null +++ b/tests/tool-catalog-parity.test.ts @@ -0,0 +1,97 @@ +/** + * Cross-runtime parity for mid-turn toolbox rearming. + * + * The browser builds the first round's wire `tools` array with + * buildToolList and ships the full catalog (buildToolCatalog) in the + * /stream envelope; the venice orchestrator rebuilds `tools` from that + * catalog (buildToolsFromCatalog in + * supabase/functions/venice/tool_catalog.ts) after a mid-turn + * toggle_toolbox. The whole point of the rearm is that the rebuilt + * array is what buildToolList WOULD have produced had the toggle + * happened before the turn - so this suite pins that equivalence for + * every enabled-set shape, plus the catalog invariants the server + * relies on (key order, membership). + * + * The server module is deliberately dependency-free so it can be + * imported here directly - no Deno-only imports to drag in, unlike the + * toggle mirror (tests/toggle-toolbox-mirror.test.ts), which has to + * parse its edge file as text. + */ +import { describe, it, expect } from 'vitest'; +import { + buildToolList, + buildToolCatalog, + GATED_TOOLBOX_NAMES, + alwaysOnToolbox, + type Toolbox, +} from '../src/lib/tools'; +import { serverSideTool } from '../src/lib/tools/server_side'; +import { buildToolsFromCatalog } from '../supabase/functions/venice/tool_catalog'; + +// A fake connected MCP integration, shaped the way buildMcpToolboxes +// produces them (name prefixed `mcp:`, schema-only tools). +const mcpBox: Toolbox = { + name: 'mcp:fake-integration', + description: 'Fake integration for parity coverage', + tools: [ + serverSideTool({ + name: 'mcp:fake-integration:send_thing', + description: 'Send a thing', + shortDescription: 'send a thing', + parameters: { type: 'object', properties: {}, additionalProperties: false }, + }), + ], +}; + +describe('buildToolCatalog', () => { + it('carries every gated toolbox under its own name, in TOOLBOXES order', () => { + const catalog = buildToolCatalog(); + expect(Object.keys(catalog.gated)).toEqual([...GATED_TOOLBOX_NAMES]); + }); + + it('appends MCP toolboxes after the static boxes', () => { + const catalog = buildToolCatalog([mcpBox]); + expect(Object.keys(catalog.gated)).toEqual([ + ...GATED_TOOLBOX_NAMES, + mcpBox.name, + ]); + }); + + it('always-on defs match the wire projection of alwaysOnToolbox', () => { + const catalog = buildToolCatalog(); + expect(catalog.alwaysOn.map((d) => d.function.name)).toEqual( + alwaysOnToolbox.tools.map((t) => t.name), + ); + // The defs must be full wire projections (activity param included), + // because the server puts them on `body.tools` verbatim. + for (const def of catalog.alwaysOn) { + const params = def.function.parameters as { + properties?: Record; + required?: unknown[]; + }; + expect(params.properties).toHaveProperty('activity'); + expect(params.required).toContain('activity'); + } + }); +}); + +describe('server rebuild parity with buildToolList', () => { + const enabledSets: readonly (readonly string[])[] = [ + [], + ['followups'], + ['cooking', 'memories'], + [...GATED_TOOLBOX_NAMES], + ['followups', 'no_such_box'], + [mcpBox.name], + ['wiki', mcpBox.name], + ]; + + for (const enabled of enabledSets) { + it(`rebuild equals buildToolList for [${enabled.join(', ')}]`, () => { + const catalog = buildToolCatalog([mcpBox]); + expect(buildToolsFromCatalog(catalog, enabled)).toEqual( + buildToolList(enabled, [mcpBox]), + ); + }); + } +});