Skip to content

Commit ca8eafe

Browse files
committed
docs(routes): document fail-closed tool discovery, stream buffering, partial-error semantics
1 parent 4f5bd77 commit ca8eafe

5 files changed

Lines changed: 25 additions & 2 deletions

File tree

‎src/proxy.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,10 @@ export function createApp(config: ProxyConfig): CreateAppResult {
707707
}
708708
const normalized = normalizeBackendToolIds(data);
709709
if (normalized.length === 0) {
710+
// Fail-closed by design (locked by tests/tool-alias.test.js): an empty
711+
// tool list under DISABLE_TOOLS=true is treated as unverifiable
712+
// discovery, not as "zero tools, proceed". A real backend always exposes
713+
// tools; empty almost certainly means a broken backend/proxy skew.
710714
if (DISABLE_TOOLS) {
711715
throw new Error('tool discovery returned empty ids');
712716
}

‎src/routes/chat.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,8 @@ export function registerChatRoutes(app: Application, ctx: AppContext): void {
432432
REQUEST_TIMEOUT_MS,
433433
'load tool overrides',
434434
)) as Record<string, boolean> | null;
435+
// Fail-closed by design: null (discovery failed) or verified-empty (no
436+
// tool IDs to disable) both 503. A real backend always exposes tools.
435437
if (DISABLE_TOOLS && (!toolOverrides || Object.keys(toolOverrides).length === 0)) {
436438
try {
437439
await activeClient.session.delete({ path: { id: sessionId as string } });
@@ -484,8 +486,12 @@ export function registerChatRoutes(app: Application, ctx: AppContext): void {
484486
};
485487
}
486488

487-
if (stream) {
488-
489+
if (stream) {
490+
// Intentional: buffer visible text while any external tool contract is
491+
// declared so `<function_calls>` / `<external__*>` markup fragments are
492+
// never streamed incrementally before the parser can classify them.
493+
// Trade-off: pure-text turns with tools declared also emit in one batch
494+
// at completion instead of incrementally.
489495
const shouldBufferExternalStream = externalToolRegistry.length > 0;
490496
let filterContentDelta = createToolCallFilter({
491497
disableTools: DISABLE_TOOLS,
@@ -1032,6 +1038,10 @@ export function registerChatRoutes(app: Application, ctx: AppContext): void {
10321038
}
10331039
break;
10341040
}
1041+
// Fail-closed: any backend error fails even when partial content exists.
1042+
// Returning partial text as success would hide the upstream failure and
1043+
// diverge from stream paths (which throw on snapshot.error). Retry above
1044+
// already covers the no-output transient case.
10351045
if (error != null) {
10361046
if (/^Request timeout after/.test(toErrorMessage(error))) throw error;
10371047
const normalizedDiscovery = normalizeBackendError(error);

‎src/routes/interactions.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ export function registerInteractionsRoutes(app: Application, ctx: AppContext): v
272272
toolOverrides = merged;
273273
}
274274
}
275+
// Fail-closed by design (see chat.ts): null or verified-empty both 503.
275276
if (DISABLE_TOOLS && (!toolOverrides || Object.keys(toolOverrides).length === 0)) {
276277
res.status(503).json({
277278
error: {

‎src/routes/messages.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ export function registerMessagesRoutes(app: Application, ctx: AppContext): void
330330
REQUEST_TIMEOUT_MS,
331331
'load tool overrides',
332332
)) as Record<string, boolean> | null;
333+
// Fail-closed by design (see chat.ts): null or verified-empty both 503.
333334
if (DISABLE_TOOLS && (!toolOverrides || Object.keys(toolOverrides).length === 0)) {
334335
try {
335336
await activeClient.session.delete({ path: { id: sessionId as string } });
@@ -449,6 +450,7 @@ export function registerMessagesRoutes(app: Application, ctx: AppContext): void
449450
if (error && !content && !reasoning && attempt < maxAttempts && isTransientUpstreamError(error)) continue;
450451
break;
451452
}
453+
// Fail-closed: any backend error fails even with partial content (see chat.ts).
452454
if (error != null) {
453455
const t = transformUpstreamError(error);
454456
res.status(t.statusCode).json({
@@ -538,6 +540,9 @@ export function registerMessagesRoutes(app: Application, ctx: AppContext): void
538540
let rawContent = '';
539541
let rawReasoning = '';
540542
const streamedToolCalls: FinalToolCall[] = [];
543+
// Intentional: buffer while any external tool contract is declared so tool
544+
// markup is never emitted as incremental `content_block_delta` before it
545+
// can be classified. Pure-text turns with tools declared also batch.
541546
const shouldBufferExternalStream = externalToolRegistry.length > 0;
542547
const filterContent = createToolCallFilter({ disableTools: DISABLE_TOOLS, forceStrip: shouldBufferExternalStream, registry: externalToolRegistry });
543548
const filterReasoning = createToolCallFilter({ disableTools: DISABLE_TOOLS, forceStrip: shouldBufferExternalStream, registry: externalToolRegistry });

‎src/routes/responses.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,7 @@ export function registerResponsesRoutes(app: Application, ctx: AppContext): void
729729
return;
730730
}
731731
}
732+
// Fail-closed by design (see chat.ts): null or verified-empty both 503.
732733
if (DISABLE_TOOLS && (!toolOverrides || Object.keys(toolOverrides).length === 0)) {
733734
await cleanupOwnedResponsesSession();
734735
if (stream) {
@@ -1007,6 +1008,8 @@ export function registerResponsesRoutes(app: Application, ctx: AppContext): void
10071008
}
10081009
};
10091010
responseState.finalize = () => finalizeOutputItems();
1011+
// Intentional: buffer while any external tool contract is declared so tool
1012+
// markup is never streamed before classification. Pure-text turns also batch.
10101013
const shouldBufferExternalStream = externalToolRegistry.length > 0;
10111014
const deferVisibleOutput = shouldBufferExternalStream || hostedSearch.requested;
10121015
let filterContentDelta = createToolCallFilter({ disableTools: DISABLE_TOOLS, forceStrip: shouldBufferExternalStream, registry: externalToolRegistry });

0 commit comments

Comments
 (0)