diff --git a/api/server/controllers/agents/__tests__/callbacks.spec.js b/api/server/controllers/agents/__tests__/callbacks.spec.js index 6a171e7ba0..9b1f3b6eb7 100644 --- a/api/server/controllers/agents/__tests__/callbacks.spec.js +++ b/api/server/controllers/agents/__tests__/callbacks.spec.js @@ -187,6 +187,26 @@ describe('createPtcProgressEmitter', () => { }); }); + it('absorbs a rejected resumable emit instead of leaving an unhandled rejection', async () => { + const { GenerationJobManager } = require('@librechat/api'); + const { createPtcProgressEmitter } = require('../callbacks'); + GenerationJobManager.emitChunk.mockRejectedValueOnce(new Error('transport down')); + const unhandled = jest.fn(); + process.on('unhandledRejection', unhandled); + + const emit = createPtcProgressEmitter({ + res: { write: jest.fn() }, + streamId: 'conversation-1', + jobCreatedAt: 1234, + }); + + expect(() => emit(ptcEvent)).not.toThrow(); + await new Promise((resolve) => setImmediate(resolve)); + process.off('unhandledRejection', unhandled); + + expect(unhandled).not.toHaveBeenCalled(); + }); + it('drops the event once the response has closed', () => { const { sendEvent } = require('@librechat/api'); const { createPtcProgressEmitter } = require('../callbacks'); diff --git a/api/server/controllers/agents/callbacks.js b/api/server/controllers/agents/callbacks.js index 9cca31bc81..7e6f5f4af7 100644 --- a/api/server/controllers/agents/callbacks.js +++ b/api/server/controllers/agents/callbacks.js @@ -1106,7 +1106,16 @@ function createPtcProgressEmitter({ res, streamId = null, jobCreatedAt }) { } const payload = { event: StepEvents.ON_PTC_TOOL_CALL, data: event }; if (streamId) { - GenerationJobManager.emitChunk(streamId, payload, { expectedCreatedAt: jobCreatedAt }); + /* Absorb a rejected transport here. The emitter is called from a + * synchronous try/catch inside `instrumentPtcToolMap`, which cannot + * observe a rejected promise — without this catch a failed emit would + * surface as an unhandled rejection on every affected inner call + * instead of being dropped as the telemetry it is. */ + Promise.resolve( + GenerationJobManager.emitChunk(streamId, payload, { expectedCreatedAt: jobCreatedAt }), + ).catch(() => { + /* dropped: the trace is best-effort */ + }); return; } sendEvent(res, payload); diff --git a/client/src/components/Chat/Messages/Content/Parts/PtcToolTrace.tsx b/client/src/components/Chat/Messages/Content/Parts/PtcToolTrace.tsx index dc0d1b9b21..4567bd984a 100644 --- a/client/src/components/Chat/Messages/Content/Parts/PtcToolTrace.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/PtcToolTrace.tsx @@ -1,9 +1,11 @@ import { useRecoilValue } from 'recoil'; import { useTranslation } from 'react-i18next'; import { isReportableRunStepDuration } from 'librechat-data-provider'; +import type { TranslationKeys } from '~/hooks'; import type { PtcTraceEntry } from '~/store'; import { cn, parseToolName, getRunStepDurationLabels } from '~/utils'; -import { ptcTraceByToolCallId } from '~/store'; +import { ptcTraceByToolCallId, ptcTraceKey } from '~/store'; +import { useMessageContext } from '~/Providers/MessageContext'; import { useMCPServerNames } from '~/hooks/MCP'; import { useLocalize } from '~/hooks'; @@ -15,6 +17,15 @@ const STATUS_GLYPH: Record = { error: '✗', }; +/** Spoken equivalent of the glyph. The glyph itself is decorative, and a + * fast call renders no duration, so without this a completed row would + * announce no outcome at all. */ +const STATUS_LABEL_KEYS: Record = { + running: 'com_ui_ptc_trace_running', + success: 'com_ui_ptc_trace_done', + error: 'com_ui_ptc_trace_failed', +}; + /** * One line of the trace. Reads as a shell transcript — status glyph, the tool * being called, its arguments, and how long it took — with the failure message @@ -44,6 +55,7 @@ function PtcTraceLine({ entry }: { entry: PtcTraceEntry }) { > {STATUS_GLYPH[entry.status]} + {localize(STATUS_LABEL_KEYS[entry.status])} {parsed.mcpServer && ( <> @@ -64,7 +76,8 @@ function PtcTraceLine({ entry }: { entry: PtcTraceEntry }) { ) : ( - + /* The sr-only status above already speaks this cell's meaning. */ + )} @@ -98,7 +111,12 @@ export default function PtcToolTrace({ className?: string; }) { const localize = useLocalize(); - const entries = useRecoilValue(ptcTraceByToolCallId(toolCallId ?? '')); + /** Scope to this card's own message: providers reuse `tool_call_id` across + * turns, so the raw id alone would show a later program's calls here. */ + const { messageId } = useMessageContext(); + const entries = useRecoilValue( + ptcTraceByToolCallId(toolCallId && messageId ? ptcTraceKey(messageId, toolCallId) : ''), + ); if (entries.length === 0) { return null; diff --git a/client/src/components/Chat/Messages/Content/Parts/__tests__/ExecuteCode.test.tsx b/client/src/components/Chat/Messages/Content/Parts/__tests__/ExecuteCode.test.tsx index c546203371..0174755940 100644 --- a/client/src/components/Chat/Messages/Content/Parts/__tests__/ExecuteCode.test.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/__tests__/ExecuteCode.test.tsx @@ -2,7 +2,8 @@ import React from 'react'; import { RecoilRoot } from 'recoil'; import { render } from '@testing-library/react'; import type { PtcTraceEntry } from '~/store/ptc'; -import { ptcTraceByToolCallId } from '~/store/ptc'; +import { ptcTraceByToolCallId, ptcTraceKey } from '~/store/ptc'; +import { MessageContext } from '~/Providers/MessageContext'; import ExecuteCode from '../ExecuteCode'; import store from '~/store'; @@ -144,22 +145,25 @@ describe('ExecuteCode streaming follow-scroll', () => { describe('ExecuteCode programmatic tool trace', () => { const TOOL_CALL_ID = 'call_ptc_1'; + const MESSAGE_ID = 'response-msg-1'; const renderWithTrace = (entries: PtcTraceEntry[]) => render( { set(store.autoExpandTools, true); - set(ptcTraceByToolCallId(TOOL_CALL_ID), entries); + set(ptcTraceByToolCallId(ptcTraceKey(MESSAGE_ID, TOOL_CALL_ID)), entries); }} > - + + + , ); diff --git a/client/src/components/Chat/Messages/Content/Parts/__tests__/PtcToolTrace.test.tsx b/client/src/components/Chat/Messages/Content/Parts/__tests__/PtcToolTrace.test.tsx index df2107c785..c4410c2ec0 100644 --- a/client/src/components/Chat/Messages/Content/Parts/__tests__/PtcToolTrace.test.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/__tests__/PtcToolTrace.test.tsx @@ -2,7 +2,8 @@ import React from 'react'; import { RecoilRoot, useSetRecoilState } from 'recoil'; import { render, screen } from '@testing-library/react'; import type { PtcTraceEntry } from '~/store/ptc'; -import { ptcTraceByToolCallId } from '~/store/ptc'; +import { ptcTraceByToolCallId, ptcTraceKey } from '~/store/ptc'; +import { MessageContext } from '~/Providers/MessageContext'; import PtcToolTrace from '../PtcToolTrace'; jest.mock('~/hooks', () => ({ @@ -13,6 +14,7 @@ jest.mock('~/hooks', () => ({ com_ui_ptc_trace_title: 'Tool calls', com_ui_ptc_trace_running: 'running', com_ui_ptc_trace_failed: 'failed', + com_ui_ptc_trace_done: 'done', com_ui_tool_name_code: 'Code', }; if (key === 'com_ui_duration_seconds') { @@ -31,10 +33,11 @@ jest.mock('~/utils', () => ({ })); const TOOL_CALL_ID = 'call_ptc_1'; +const MESSAGE_ID = 'response-msg-1'; -function renderTrace(entries: PtcTraceEntry[]) { +function renderTrace(entries: PtcTraceEntry[], messageId = MESSAGE_ID) { const Seed = () => { - const set = useSetRecoilState(ptcTraceByToolCallId(TOOL_CALL_ID)); + const set = useSetRecoilState(ptcTraceByToolCallId(ptcTraceKey(MESSAGE_ID, TOOL_CALL_ID))); React.useEffect(() => { if (entries.length > 0) { set(entries); @@ -46,7 +49,9 @@ function renderTrace(entries: PtcTraceEntry[]) { return render( - + + + , ); } @@ -67,7 +72,8 @@ describe('PtcToolTrace', () => { expect(screen.getByText('read_file')).toBeInTheDocument(); expect(screen.getByText('path=a.ts')).toBeInTheDocument(); expect(screen.getByText('write_file')).toBeInTheDocument(); - expect(screen.getByText('running')).toBeInTheDocument(); + /* Twice by design: the visible cell plus the screen-reader status. */ + expect(screen.getAllByText('running')).toHaveLength(2); }); it('splits an MCP tool id into its server and tool name', () => { @@ -95,6 +101,18 @@ describe('PtcToolTrace', () => { it('falls back to a generic failure label when no message came through', () => { renderTrace([{ callId: 'a', name: 'write_file', status: 'error' }]); - expect(screen.getByText('failed')).toBeInTheDocument(); + expect(screen.getAllByText('failed').length).toBeGreaterThan(0); + }); + + it('speaks a completion status even when the call was too fast to time', () => { + renderTrace([{ callId: 'a', name: 'read_file', status: 'success' }]); + + expect(screen.getByText('done')).toBeInTheDocument(); + }); + + it('does not show another message\u2019s trace in this card', () => { + renderTrace([{ callId: 'a', name: 'read_file', status: 'success' }], 'a-different-message'); + + expect(screen.queryByText('read_file')).not.toBeInTheDocument(); }); }); diff --git a/client/src/hooks/SSE/__tests__/useResumableSSE.spec.ts b/client/src/hooks/SSE/__tests__/useResumableSSE.spec.ts index 49fd41fe8e..514f6b6456 100644 --- a/client/src/hooks/SSE/__tests__/useResumableSSE.spec.ts +++ b/client/src/hooks/SSE/__tests__/useResumableSSE.spec.ts @@ -236,6 +236,7 @@ jest.mock('~/hooks/SSE/useEventHandlers', () => { contentHandler: jest.fn(), resetContentHandler: jest.fn(), syncStepMessage: jest.fn(), + prunePtcTraces: jest.fn(), clearStepMaps: mockClearStepMaps, flushPendingDeltas: jest.fn(), messageHandler: jest.fn(), diff --git a/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts b/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts index 1e3ebd3569..a2169b4f57 100644 --- a/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts +++ b/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts @@ -20,7 +20,7 @@ import type { } from 'librechat-data-provider'; import type { PtcTraceEntry } from '~/store/ptc'; import { subagentProgressByToolCallId, subagentProgressKey } from '~/store/subagents'; -import { ptcTraceByToolCallId } from '~/store/ptc'; +import { ptcTraceByToolCallId, ptcTraceKey } from '~/store/ptc'; import { resolveAskUserQuestionPart } from '~/utils/approval'; import useStepHandler from '~/hooks/SSE/useStepHandler'; @@ -3888,8 +3888,10 @@ describe('useStepHandler', () => { const stepHandler = useStepHandler(createHookParams()); const read = useRecoilCallback( ({ snapshot }) => - (toolCallId: string): PtcTraceEntry[] => - snapshot.getLoadable(ptcTraceByToolCallId(toolCallId)).valueOrThrow(), + (messageId: string, toolCallId: string): PtcTraceEntry[] => + snapshot + .getLoadable(ptcTraceByToolCallId(ptcTraceKey(messageId, toolCallId))) + .valueOrThrow(), [], ); return { ...stepHandler, read }; @@ -3898,10 +3900,12 @@ describe('useStepHandler', () => { ); return { result: hookResult.result, - getTrace: (toolCallId: string): PtcTraceEntry[] => - (hookResult.result.current as unknown as { read: (id: string) => PtcTraceEntry[] }).read( - toolCallId, - ), + getTrace: (toolCallId: string, messageId = 'response-msg-1'): PtcTraceEntry[] => + ( + hookResult.result.current as unknown as { + read: (m: string, id: string) => PtcTraceEntry[]; + } + ).read(messageId, toolCallId), }; }; @@ -3910,6 +3914,7 @@ describe('useStepHandler', () => { call_id: 'call_ptc:0', name: 'read_file', status: 'running', + runId: 'response-msg-1', ...overrides, }); @@ -4005,6 +4010,71 @@ describe('useStepHandler', () => { expect(getTrace('call_ptc')).toEqual([]); }); + it('keeps a reused tool_call_id isolated across parent messages', () => { + const { result, getTrace } = renderWithTraceReader(); + const submission = createSubmission(); + + act(() => { + result.current.stepHandler( + { event: StepEvents.ON_PTC_TOOL_CALL, data: ptcEvent({ name: 'read_file' }) }, + submission, + ); + result.current.stepHandler( + { + event: StepEvents.ON_PTC_TOOL_CALL, + data: ptcEvent({ name: 'write_file', runId: 'response-msg-2' }), + }, + submission, + ); + }); + + expect(getTrace('call_ptc').map((e) => e.name)).toEqual(['read_file']); + expect(getTrace('call_ptc', 'response-msg-2').map((e) => e.name)).toEqual(['write_file']); + }); + + it('drops an envelope that cannot be scoped to a parent message', () => { + const { result, getTrace } = renderWithTraceReader(); + const submission = createSubmission(); + + act(() => { + result.current.stepHandler( + { event: StepEvents.ON_PTC_TOOL_CALL, data: ptcEvent({ runId: undefined }) }, + submission, + ); + }); + + expect(getTrace('call_ptc')).toEqual([]); + }); + + it('prunes rows still running across a resume gap and keeps settled ones', () => { + const { result, getTrace } = renderWithTraceReader(); + const submission = createSubmission(); + + act(() => { + result.current.stepHandler( + { + event: StepEvents.ON_PTC_TOOL_CALL, + data: ptcEvent({ call_id: 'call_ptc:0', status: 'success', durationMs: 900 }), + }, + submission, + ); + result.current.stepHandler( + { + event: StepEvents.ON_PTC_TOOL_CALL, + data: ptcEvent({ call_id: 'call_ptc:1', name: 'write_file' }), + }, + submission, + ); + }); + expect(getTrace('call_ptc')).toHaveLength(2); + + act(() => { + (result.current as unknown as { prunePtcTraces: () => void }).prunePtcTraces(); + }); + + expect(getTrace('call_ptc').map((e) => e.callId)).toEqual(['call_ptc:0']); + }); + it('releases the trace atoms on reset', () => { const { result, getTrace } = renderWithTraceReader(); const submission = createSubmission(); diff --git a/client/src/hooks/SSE/useEventHandlers.ts b/client/src/hooks/SSE/useEventHandlers.ts index 3e132b8dbb..d1b7d4f247 100644 --- a/client/src/hooks/SSE/useEventHandlers.ts +++ b/client/src/hooks/SSE/useEventHandlers.ts @@ -376,6 +376,7 @@ export default function useEventHandlers({ clearStepMaps, resetSubagentAtoms, resetPtcAtoms, + prunePtcTraces, syncStepMessage, cancelPendingDeltaFlush, flushPendingDeltas, @@ -1211,6 +1212,7 @@ export default function useEventHandlers({ createdHandler, titleHandler, syncStepMessage, + prunePtcTraces, cancelPendingDeltaFlush, flushPendingDeltas, attachmentHandler, diff --git a/client/src/hooks/SSE/useResumableSSE.ts b/client/src/hooks/SSE/useResumableSSE.ts index 613a86e48c..84ae06686b 100644 --- a/client/src/hooks/SSE/useResumableSSE.ts +++ b/client/src/hooks/SSE/useResumableSSE.ts @@ -1137,6 +1137,7 @@ export default function useResumableSSE( createdHandler, titleHandler, syncStepMessage, + prunePtcTraces, attachmentHandler, resetContentHandler, flushPendingDeltas, @@ -2126,6 +2127,12 @@ export default function useResumableSSE( titleHandler(data.resumeState.titleEvent); } + /** PTC inner calls are not content parts, so the resume snapshot + * can't rebuild them and their settling events aren't replayed. + * Any row still `running` across this gap is unknowable — drop it + * rather than leave a spinner that never resolves. */ + prunePtcTraces(); + if (data.resumeState?.replayEvents?.length > 0) { logger.log( 'ResumableSSE', @@ -3474,6 +3481,7 @@ export default function useResumableSSE( contentHandler, resetContentHandler, syncStepMessage, + prunePtcTraces, clearStepMaps, messageHandler, errorHandler, diff --git a/client/src/hooks/SSE/useStepHandler.ts b/client/src/hooks/SSE/useStepHandler.ts index 1f79453d10..8c3422a009 100644 --- a/client/src/hooks/SSE/useStepHandler.ts +++ b/client/src/hooks/SSE/useStepHandler.ts @@ -34,6 +34,7 @@ import { sandboxStartingByToolCallId, ptcTraceByToolCallId, subagentProgressKey, + ptcTraceKey, } from '~/store'; import { isAskUserQuestionPart, isAnsweredAskUserQuestionPart } from '~/utils/approval'; import { MESSAGE_UPDATE_INTERVAL } from '~/common'; @@ -380,13 +381,16 @@ export default function useStepHandler({ */ const applyPtcToolCall = useRecoilCallback( ({ set }) => - (event: PtcToolCallEvent): void => { + (event: PtcToolCallEvent, parentMessageId: string): void => { const { tool_call_id: toolCallId, call_id: callId, name, status } = event; - if (!toolCallId || !callId) { + /** No parent message means no occurrence to scope this to; a raw + * `tool_call_id` would leak the rows into whichever card reused it. */ + if (!toolCallId || !callId || !parentMessageId) { return; } - knownPtcAtomKeys.current.add(toolCallId); - set(ptcTraceByToolCallId(toolCallId), (previous) => { + const atomKey = ptcTraceKey(parentMessageId, toolCallId); + knownPtcAtomKeys.current.add(atomKey); + set(ptcTraceByToolCallId(atomKey), (previous) => { const index = previous.findIndex((entry) => entry.callId === callId); const entry = { callId, @@ -410,14 +414,35 @@ export default function useStepHandler({ const resetPtcAtoms = useRecoilCallback( ({ reset }) => (): void => { - for (const toolCallId of knownPtcAtomKeys.current) { - reset(ptcTraceByToolCallId(toolCallId)); + for (const atomKey of knownPtcAtomKeys.current) { + reset(ptcTraceByToolCallId(atomKey)); } knownPtcAtomKeys.current.clear(); }, [], ); + /** + * Drops rows still marked `running` after a stream gap. Inner calls carry no + * durable state — they are not content parts, so the resume snapshot cannot + * rebuild them and their settling events are not replayed — which means a + * call that finished during a disconnect would otherwise show a spinner + * forever. Settled rows are real history and stay. + */ + const prunePtcTraces = useRecoilCallback( + ({ set }) => + (): void => { + for (const atomKey of knownPtcAtomKeys.current) { + set(ptcTraceByToolCallId(atomKey), (previous) => + previous.some((entry) => entry.status === 'running') + ? previous.filter((entry) => entry.status !== 'running') + : previous, + ); + } + }, + [], + ); + /** * Calculate content index for a run step. * @@ -1331,7 +1356,13 @@ export default function useStepHandler({ } else if (stepEvent.event === StepEvents.ON_SANDBOX_STARTING) { setSandboxStarting(stepEvent.data.tool_call_id); } else if (stepEvent.event === StepEvents.ON_PTC_TOOL_CALL) { - applyPtcToolCall(stepEvent.data); + /** `runId` is the response message id (the run configurable's + * `run_id`), the same correlation the subagent path uses. */ + let responseMessageId = stepEvent.data.runId ?? ''; + if (responseMessageId === Constants.USE_PRELIM_RESPONSE_MESSAGE_ID) { + responseMessageId = submission?.initialResponse?.messageId ?? ''; + } + applyPtcToolCall(stepEvent.data, responseMessageId); } else if (stepEvent.event === StepEvents.ON_SUBAGENT_UPDATE) { let responseMessageId = stepEvent.data.runId; if (responseMessageId === Constants.USE_PRELIM_RESPONSE_MESSAGE_ID) { @@ -1534,6 +1565,7 @@ export default function useStepHandler({ clearStepMaps, resetSubagentAtoms, resetPtcAtoms, + prunePtcTraces, syncStepMessage, cancelPendingDeltaFlush, flushPendingDeltas, diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index d4fdf46253..074d27bd41 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1742,6 +1742,7 @@ "com_ui_provider": "Provider", "com_ui_provider_api_keys_description": "Manage API keys for endpoints configured to use a user-provided key.", "com_ui_provider_api_keys_not_set": "No key set", + "com_ui_ptc_trace_done": "done", "com_ui_ptc_trace_failed": "failed", "com_ui_ptc_trace_running": "running", "com_ui_ptc_trace_title": "Tool calls", diff --git a/client/src/store/ptc.ts b/client/src/store/ptc.ts index 8fa0165795..4a8b4c96a2 100644 --- a/client/src/store/ptc.ts +++ b/client/src/store/ptc.ts @@ -15,10 +15,20 @@ export interface PtcTraceEntry { durationMs?: number; } +/** + * Stable identity for one PTC invocation in its parent message. Providers + * reuse `tool_call_id` across turns and agents, so the raw id is not + * sufficient identity for live progress — the same rule `subagentProgressKey` + * exists for. Without the message scope, a later program's rows would land in + * an earlier card that happened to share `call_0`. + */ +export const ptcTraceKey = (parentMessageId: string, toolCallId: string) => + `${parentMessageId}\u0000${toolCallId}`; + /** * Live trace of the inner tool calls made by one PTC run step, in the order - * the sandbox started them (`on_ptc_tool_call` SSE events). Keyed by the PTC - * `tool_call_id` — the card the trace renders under. + * the sandbox started them (`on_ptc_tool_call` SSE events). Keyed by + * {@link ptcTraceKey} — one concrete invocation in one message. * * Session-scoped like the subagent ticker: inner calls open no run step, so * nothing persists them on the message. Cleared on conversation switch rather diff --git a/packages/api/src/agents/ptc.spec.ts b/packages/api/src/agents/ptc.spec.ts index aad6dc37c0..80be9f525c 100644 --- a/packages/api/src/agents/ptc.spec.ts +++ b/packages/api/src/agents/ptc.spec.ts @@ -41,6 +41,34 @@ describe('summarizePtcArgs', () => { expect(summary.length).toBeLessThanOrEqual(97); }); + it('stops once the preview budget is spent instead of visiting every key', () => { + const seen: string[] = []; + const probe: Record = {}; + for (let i = 0; i < 40; i++) { + Object.defineProperty(probe, `key${i}`, { + enumerable: true, + get() { + seen.push(`key${i}`); + return `value${i}`; + }, + }); + } + + summarizePtcArgs(probe); + + expect(seen.length).toBeLessThan(40); + }); + + it('does not rewrite the whole of an oversized value to build a short preview', () => { + const huge = 'a b '.repeat(500_000); + const started = Date.now(); + const summary = summarizePtcArgs({ content: huge, path: 'a.ts' }); + + expect(summary).toContain('path=a.ts'); + expect(summary.length).toBeLessThanOrEqual(97); + expect(Date.now() - started).toBeLessThan(150); + }); + it('falls back to the raw string for a non-object input', () => { expect(summarizePtcArgs('ls -la')).toBe('ls -la'); }); diff --git a/packages/api/src/agents/ptc.ts b/packages/api/src/agents/ptc.ts index 404cd78e6d..28de7b4d26 100644 --- a/packages/api/src/agents/ptc.ts +++ b/packages/api/src/agents/ptc.ts @@ -6,42 +6,70 @@ const ARGS_PREVIEW_MAX_CHARS = 96; /** Per-value budget, so one long string can't crowd out the other keys. */ const ARGS_VALUE_MAX_CHARS = 40; const ERROR_PREVIEW_MAX_CHARS = 160; +/** + * Collapsing whitespace can only shorten a string, so a window this many times + * the visible budget is always long enough to fill it. Slicing to the window + * before rewriting matters: this runs synchronously ahead of every inner + * `invoke`, and without it a multi-megabyte argument would be collapsed in + * full to produce a forty-character preview. + */ +const CLIP_OVERSCAN = 4; -const truncate = (input: string, max: number): string => - input.length <= max ? input : `${input.slice(0, max)}…`; - -const collapse = (input: string): string => input.replace(/\s+/g, ' ').trim(); +/** Bounded collapse-and-clip: never rewrites more of `input` than the budget + * can possibly need, and marks any truncation it performed. */ +const clip = (input: string, max: number): string => { + const window = input.length > max * CLIP_OVERSCAN ? input.slice(0, max * CLIP_OVERSCAN) : input; + const collapsed = window.replace(/\s+/g, ' ').trim(); + if (collapsed.length <= max && window.length === input.length) { + return collapsed; + } + return `${collapsed.slice(0, max)}…`; +}; /** * Collapses an inner call's input into a single `key=value, key=value` line - * for the CLI-style trace. Values are stringified and clipped individually; - * the joined result is clipped again so the payload stays bounded no matter - * how many keys the tool takes. + * for the CLI-style trace. Values are clipped individually and iteration stops + * as soon as the joined preview can no longer grow, so a call with a large + * body or many keys costs the same as a small one. */ export function summarizePtcArgs(input: unknown): string { if (input == null) { return ''; } if (typeof input === 'string') { - return truncate(collapse(input), ARGS_PREVIEW_MAX_CHARS); + return clip(input, ARGS_PREVIEW_MAX_CHARS); } if (typeof input !== 'object' || Array.isArray(input)) { - return truncate(collapse(String(input)), ARGS_PREVIEW_MAX_CHARS); + return clip(String(input), ARGS_PREVIEW_MAX_CHARS); } + const record = input as Record; const entries: string[] = []; - for (const [key, value] of Object.entries(input as Record)) { + let budget = ARGS_PREVIEW_MAX_CHARS; + /* Keys, not entries: `Object.entries` would materialize every value before + * the loop starts, so the budget check below could never skip the work it + * exists to skip. */ + for (const key of Object.keys(record)) { + if (budget <= 0) { + break; + } + const value = record[key]; if (value == null || value === '') { continue; } + /* Strings are the values that get large (file bodies, request payloads), + * and `clip` bounds them without touching the tail. Everything else is + * small enough that serializing it first is cheaper than inspecting it. */ const rendered = typeof value === 'string' ? value : safeStringify(value); if (rendered === '') { continue; } - entries.push(`${key}=${truncate(collapse(rendered), ARGS_VALUE_MAX_CHARS)}`); + const entry = `${key}=${clip(rendered, ARGS_VALUE_MAX_CHARS)}`; + entries.push(entry); + budget -= entry.length + 2; } - return truncate(entries.join(', '), ARGS_PREVIEW_MAX_CHARS); + return clip(entries.join(', '), ARGS_PREVIEW_MAX_CHARS); } function safeStringify(value: unknown): string { @@ -126,8 +154,8 @@ export function instrumentPtcToolMap({ call_id: callId, name, status: 'error', - error: truncate( - collapse(error instanceof Error ? error.message : String(error)), + error: clip( + error instanceof Error ? error.message : String(error), ERROR_PREVIEW_MAX_CHARS, ), durationMs: Date.now() - startedAt,