diff --git a/client/src/components/Chat/Messages/Content/Parts/PtcToolTrace.tsx b/client/src/components/Chat/Messages/Content/Parts/PtcToolTrace.tsx index 50ad9273a2..40e8a9f644 100644 --- a/client/src/components/Chat/Messages/Content/Parts/PtcToolTrace.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/PtcToolTrace.tsx @@ -114,21 +114,32 @@ export default function PtcToolTrace({ /** 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( + const trace = useRecoilValue( ptcTraceByToolCallId(toolCallId && messageId ? ptcTraceKey(messageId, toolCallId) : ''), ); - if (entries.length === 0) { + if (trace.entries.length === 0) { return null; } + /** No background of its own: the pane inherits the card's surface in both + * themes, which is what the sibling output pane resolves to anyway. A + * `dark:` override here would step outside the semantic roles and lose the + * intended separation under a custom theme. */ return ( -
+
{localize('com_ui_ptc_trace_title')}
    - {entries.map((entry) => ( + {/* The retained tail is a window, not the whole program — say so + rather than let the reader assume these are all the calls. */} + {trace.dropped > 0 && ( +
  1. + {localize('com_ui_ptc_trace_earlier', { count: trace.dropped })} +
    2. + )} + {trace.entries.map((entry) => ( ))}
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 0174755940..8b0c39b14c 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 @@ -152,7 +152,7 @@ describe('ExecuteCode programmatic tool trace', () => { { set(store.autoExpandTools, true); - set(ptcTraceByToolCallId(ptcTraceKey(MESSAGE_ID, TOOL_CALL_ID)), entries); + set(ptcTraceByToolCallId(ptcTraceKey(MESSAGE_ID, TOOL_CALL_ID)), { entries, dropped: 0 }); }} > 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 c4410c2ec0..66ed6e6796 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 @@ -15,6 +15,7 @@ jest.mock('~/hooks', () => ({ com_ui_ptc_trace_running: 'running', com_ui_ptc_trace_failed: 'failed', com_ui_ptc_trace_done: 'done', + com_ui_ptc_trace_earlier: `+${values?.count} earlier calls`, com_ui_tool_name_code: 'Code', }; if (key === 'com_ui_duration_seconds') { @@ -35,12 +36,12 @@ jest.mock('~/utils', () => ({ const TOOL_CALL_ID = 'call_ptc_1'; const MESSAGE_ID = 'response-msg-1'; -function renderTrace(entries: PtcTraceEntry[], messageId = MESSAGE_ID) { +function renderTrace(entries: PtcTraceEntry[], messageId = MESSAGE_ID, dropped = 0) { const Seed = () => { const set = useSetRecoilState(ptcTraceByToolCallId(ptcTraceKey(MESSAGE_ID, TOOL_CALL_ID))); React.useEffect(() => { if (entries.length > 0) { - set(entries); + set({ entries, dropped }); } }, [set]); return null; @@ -110,6 +111,18 @@ describe('PtcToolTrace', () => { expect(screen.getByText('done')).toBeInTheDocument(); }); + it('says how many calls the cap dropped rather than truncating silently', () => { + renderTrace([{ callId: 'a', name: 'read_file', status: 'success' }], MESSAGE_ID, 42); + + expect(screen.getByText('+42 earlier calls')).toBeInTheDocument(); + }); + + it('shows no truncation notice when nothing was dropped', () => { + renderTrace([{ callId: 'a', name: 'read_file', status: 'success' }]); + + expect(screen.queryByText(/earlier calls/)).not.toBeInTheDocument(); + }); + it('does not show another message\u2019s trace in this card', () => { renderTrace([{ callId: 'a', name: 'read_file', status: 'success' }], 'a-different-message'); diff --git a/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts b/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts index 72dd636e9e..668e607a3e 100644 --- a/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts +++ b/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts @@ -18,9 +18,9 @@ import type { PtcToolCallEvent, Agents, } from 'librechat-data-provider'; -import type { PtcTraceEntry } from '~/store/ptc'; +import type { PtcTrace, PtcTraceEntry } from '~/store/ptc'; +import { ptcTraceByToolCallId, ptcTraceKey, PTC_TRACE_MAX_ENTRIES } from '~/store/ptc'; import { subagentProgressByToolCallId, subagentProgressKey } from '~/store/subagents'; -import { ptcTraceByToolCallId, ptcTraceKey } from '~/store/ptc'; import { resolveAskUserQuestionPart } from '~/utils/approval'; import useStepHandler from '~/hooks/SSE/useStepHandler'; @@ -3924,7 +3924,7 @@ describe('useStepHandler', () => { const stepHandler = useStepHandler(createHookParams()); const read = useRecoilCallback( ({ snapshot }) => - (messageId: string, toolCallId: string): PtcTraceEntry[] => + (messageId: string, toolCallId: string): PtcTrace => snapshot .getLoadable(ptcTraceByToolCallId(ptcTraceKey(messageId, toolCallId))) .valueOrThrow(), @@ -3936,11 +3936,13 @@ describe('useStepHandler', () => { ); return { result: hookResult.result, - getTrace: (toolCallId: string, messageId = 'response-msg-1'): PtcTraceEntry[] => + getEntries: (toolCallId: string, messageId = 'response-msg-1'): PtcTraceEntry[] => ( - hookResult.result.current as unknown as { - read: (m: string, id: string) => PtcTraceEntry[]; - } + hookResult.result.current as unknown as { read: (m: string, id: string) => PtcTrace } + ).read(messageId, toolCallId).entries, + getTrace: (toolCallId: string, messageId = 'response-msg-1'): PtcTrace => + ( + hookResult.result.current as unknown as { read: (m: string, id: string) => PtcTrace } ).read(messageId, toolCallId), }; }; @@ -3955,7 +3957,7 @@ describe('useStepHandler', () => { }); it('appends a row for each inner call the program starts', () => { - const { result, getTrace } = renderWithTraceReader(); + const { result, getEntries } = renderWithTraceReader(); const submission = createSubmission(); act(() => { @@ -3975,14 +3977,14 @@ describe('useStepHandler', () => { ); }); - expect(getTrace('call_ptc')).toEqual([ + expect(getEntries('call_ptc')).toEqual([ { callId: 'call_ptc:0', name: 'read_file', status: 'running', args: 'path=a.ts' }, { callId: 'call_ptc:1', name: 'write_file', status: 'running', args: 'path=b.ts' }, ]); }); it('settles a row in place instead of appending a duplicate', () => { - const { result, getTrace } = renderWithTraceReader(); + const { result, getEntries } = renderWithTraceReader(); const submission = createSubmission(); act(() => { @@ -3999,7 +4001,7 @@ describe('useStepHandler', () => { ); }); - expect(getTrace('call_ptc')).toEqual([ + expect(getEntries('call_ptc')).toEqual([ { callId: 'call_ptc:0', name: 'read_file', @@ -4011,7 +4013,7 @@ describe('useStepHandler', () => { }); it('keeps each PTC call trace under its own tool call id', () => { - const { result, getTrace } = renderWithTraceReader(); + const { result, getEntries } = renderWithTraceReader(); const submission = createSubmission(); act(() => { @@ -4028,12 +4030,12 @@ describe('useStepHandler', () => { ); }); - expect(getTrace('call_ptc')).toHaveLength(1); - expect(getTrace('call_other')).toHaveLength(1); + expect(getEntries('call_ptc')).toHaveLength(1); + expect(getEntries('call_other')).toHaveLength(1); }); it('drops an envelope with no call identity rather than seeding a blank row', () => { - const { result, getTrace } = renderWithTraceReader(); + const { result, getEntries } = renderWithTraceReader(); const submission = createSubmission(); act(() => { @@ -4043,11 +4045,11 @@ describe('useStepHandler', () => { ); }); - expect(getTrace('call_ptc')).toEqual([]); + expect(getEntries('call_ptc')).toEqual([]); }); it('keeps a reused tool_call_id isolated across parent messages', () => { - const { result, getTrace } = renderWithTraceReader(); + const { result, getEntries } = renderWithTraceReader(); const submission = createSubmission(); act(() => { @@ -4064,12 +4066,12 @@ describe('useStepHandler', () => { ); }); - 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']); + expect(getEntries('call_ptc').map((e) => e.name)).toEqual(['read_file']); + expect(getEntries('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 { result, getEntries } = renderWithTraceReader(); const submission = createSubmission(); act(() => { @@ -4079,14 +4081,19 @@ describe('useStepHandler', () => { ); }); - expect(getTrace('call_ptc')).toEqual([]); + expect(getEntries('call_ptc')).toEqual([]); }); it('prunes rows still running across a resume gap and keeps settled ones', () => { - const { result, getTrace } = renderWithTraceReader(); + const { result, getEntries } = renderWithTraceReader(); const submission = createSubmission(); act(() => { + /** Real order: each row opens as `running` before it settles. */ + result.current.stepHandler( + { event: StepEvents.ON_PTC_TOOL_CALL, data: ptcEvent({ call_id: 'call_ptc:0' }) }, + submission, + ); result.current.stepHandler( { event: StepEvents.ON_PTC_TOOL_CALL, @@ -4102,17 +4109,63 @@ describe('useStepHandler', () => { submission, ); }); - expect(getTrace('call_ptc')).toHaveLength(2); + expect(getEntries('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']); + expect(getEntries('call_ptc').map((e) => e.callId)).toEqual(['call_ptc:0']); + }); + + it('caps the retained rows and reports how many it dropped', () => { + const { result, getTrace } = renderWithTraceReader(); + const submission = createSubmission(); + const total = PTC_TRACE_MAX_ENTRIES + 25; + + act(() => { + for (let i = 0; i < total; i++) { + result.current.stepHandler( + { event: StepEvents.ON_PTC_TOOL_CALL, data: ptcEvent({ call_id: `call_ptc:${i}` }) }, + submission, + ); + } + }); + + const trace = getTrace('call_ptc'); + expect(trace.entries).toHaveLength(PTC_TRACE_MAX_ENTRIES); + expect(trace.dropped).toBe(25); + /** The tail is retained, so the newest call is still visible. */ + expect(trace.entries.at(-1)?.callId).toBe(`call_ptc:${total - 1}`); + }); + + it('ignores a settle whose row the cap already evicted', () => { + const { result, getTrace } = renderWithTraceReader(); + const submission = createSubmission(); + + act(() => { + for (let i = 0; i < PTC_TRACE_MAX_ENTRIES + 5; i++) { + result.current.stepHandler( + { event: StepEvents.ON_PTC_TOOL_CALL, data: ptcEvent({ call_id: `call_ptc:${i}` }) }, + submission, + ); + } + result.current.stepHandler( + { + event: StepEvents.ON_PTC_TOOL_CALL, + data: ptcEvent({ call_id: 'call_ptc:0', status: 'success', durationMs: 10 }), + }, + submission, + ); + }); + + const trace = getTrace('call_ptc'); + expect(trace.entries).toHaveLength(PTC_TRACE_MAX_ENTRIES); + expect(trace.entries.some((e) => e.callId === 'call_ptc:0')).toBe(false); }); it('releases the trace atoms on reset', () => { - const { result, getTrace } = renderWithTraceReader(); + const { result, getEntries } = renderWithTraceReader(); const submission = createSubmission(); act(() => { @@ -4121,13 +4174,13 @@ describe('useStepHandler', () => { submission, ); }); - expect(getTrace('call_ptc')).toHaveLength(1); + expect(getEntries('call_ptc')).toHaveLength(1); act(() => { (result.current as unknown as { resetPtcAtoms: () => void }).resetPtcAtoms(); }); - expect(getTrace('call_ptc')).toEqual([]); + expect(getEntries('call_ptc')).toEqual([]); }); }); }); diff --git a/client/src/hooks/SSE/useStepHandler.ts b/client/src/hooks/SSE/useStepHandler.ts index 21c35ed5fc..b816c46f0e 100644 --- a/client/src/hooks/SSE/useStepHandler.ts +++ b/client/src/hooks/SSE/useStepHandler.ts @@ -33,6 +33,7 @@ import { takeRegisteredSubagentProgressKeys, sandboxStartingByToolCallId, ptcTraceByToolCallId, + PTC_TRACE_MAX_ENTRIES, subagentProgressKey, ptcTraceKey, } from '~/store'; @@ -371,7 +372,7 @@ export default function useStepHandler({ const atomKey = ptcTraceKey(parentMessageId, toolCallId); knownPtcAtomKeys.current.add(atomKey); set(ptcTraceByToolCallId(atomKey), (previous) => { - const index = previous.findIndex((entry) => entry.callId === callId); + const index = previous.entries.findIndex((entry) => entry.callId === callId); const entry = { callId, name, @@ -380,12 +381,28 @@ export default function useStepHandler({ ...(event.error ? { error: event.error } : {}), ...(event.durationMs != null ? { durationMs: event.durationMs } : {}), }; - if (index === -1) { - return [...previous, entry]; + + if (index !== -1) { + const next = [...previous.entries]; + next[index] = { ...previous.entries[index], ...entry }; + return { entries: next, dropped: previous.dropped }; } - const next = [...previous]; - next[index] = { ...previous[index], ...entry }; - return next; + + /** A settle whose row is gone — evicted by the cap, or pruned across + * a resume gap — must not reappear at the tail out of order. */ + if (status !== 'running') { + return previous; + } + + const appended = [...previous.entries, entry]; + const overflow = appended.length - PTC_TRACE_MAX_ENTRIES; + if (overflow <= 0) { + return { entries: appended, dropped: previous.dropped }; + } + return { + entries: appended.slice(overflow), + dropped: previous.dropped + overflow, + }; }); }, [], @@ -414,8 +431,11 @@ export default function useStepHandler({ (): void => { for (const atomKey of knownPtcAtomKeys.current) { set(ptcTraceByToolCallId(atomKey), (previous) => - previous.some((entry) => entry.status === 'running') - ? previous.filter((entry) => entry.status !== 'running') + previous.entries.some((entry) => entry.status === 'running') + ? { + ...previous, + entries: previous.entries.filter((entry) => entry.status !== 'running'), + } : previous, ); } diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 074d27bd41..08ef11d068 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1743,6 +1743,7 @@ "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_earlier": "+{{count}} earlier calls", "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 4a8b4c96a2..67d4d21f19 100644 --- a/client/src/store/ptc.ts +++ b/client/src/store/ptc.ts @@ -25,6 +25,23 @@ export interface PtcTraceEntry { export const ptcTraceKey = (parentMessageId: string, toolCallId: string) => `${parentMessageId}\u0000${toolCallId}`; +/** + * Rolling cap on retained rows. A program looping over a large collection can + * make thousands of inner calls; without a bound, every event would copy an + * ever-growing array and the card would render a row per call. + */ +export const PTC_TRACE_MAX_ENTRIES = 100; + +/** One PTC program's trace: the retained tail plus what the cap discarded. */ +export interface PtcTrace { + entries: PtcTraceEntry[]; + /** Rows evicted by {@link PTC_TRACE_MAX_ENTRIES}, so the cap is never silent. */ + dropped: number; +} + +/** Shared empty value — one reference, so untouched atoms compare equal. */ +export const EMPTY_PTC_TRACE: PtcTrace = { entries: [], dropped: 0 }; + /** * 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 @@ -34,7 +51,7 @@ export const ptcTraceKey = (parentMessageId: string, toolCallId: string) => * nothing persists them on the message. Cleared on conversation switch rather * than at run boundaries, so a finished program's trace stays readable. */ -export const ptcTraceByToolCallId = atomFamily({ +export const ptcTraceByToolCallId = atomFamily({ key: 'ptcTraceByToolCallId', - default: [], + default: EMPTY_PTC_TRACE, }); diff --git a/packages/api/src/agents/handlers.ts b/packages/api/src/agents/handlers.ts index 00d440aab6..224ee090fc 100644 --- a/packages/api/src/agents/handlers.ts +++ b/packages/api/src/agents/handlers.ts @@ -4986,6 +4986,13 @@ export function createToolExecuteHandler(options: ToolExecuteOptions): EventHand * this trace. Instrument the map the sandbox bridge * resolves against — `invoke` is the single seam every * inner call passes through. */ + /* Inner calls never reach `filteredToolArgumentsResult` + * — the sandbox bridge invokes them directly — so when + * the deployment filters tool arguments for PII, the + * trace must not put their values on the wire. */ + const ptcArgumentPii = ( + mergedConfigurable?.req as ServerRequest | undefined + )?.config?.filters?.toolArguments?.pii; toolCallConfig.toolMap = emitPtcProgress ? instrumentPtcToolMap({ toolMap: resolvedPtcToolMap, @@ -4993,6 +5000,11 @@ export function createToolExecuteHandler(options: ToolExecuteOptions): EventHand runId: (metadata as Record)?.run_id as | string | undefined, + includePreviews: !hasActivePiiFields(ptcArgumentPii, [ + 'name', + 'arguments', + 'output', + ]), emit: emitPtcProgress, }) : resolvedPtcToolMap; diff --git a/packages/api/src/agents/ptc.spec.ts b/packages/api/src/agents/ptc.spec.ts index 80be9f525c..09ee1595db 100644 --- a/packages/api/src/agents/ptc.spec.ts +++ b/packages/api/src/agents/ptc.spec.ts @@ -181,6 +181,35 @@ describe('instrumentPtcToolMap', () => { expect(tool.schema).toEqual({ type: 'object' }); }); + it('omits argument and failure previews when tool-argument filtering is on', async () => { + const { events, emit } = collect(); + const toolMap = new Map([ + [ + 'write_file', + createTool('write_file', async () => { + throw new Error('rejected value 555-01-0000'); + }), + ], + ]); + + const instrumented = instrumentPtcToolMap({ + toolMap, + toolCallId: 'call_1', + includePreviews: false, + emit, + }); + + await expect(instrumented.get('write_file')?.invoke({ ssn: '555-01-0000' })).rejects.toThrow(); + + /** Name, status and duration still report; nothing derived from the + * arguments or the failure text reaches the stream. */ + expect(events[0].args).toBeUndefined(); + expect(events[1].error).toBeUndefined(); + expect(events.map((e) => e.status)).toEqual(['running', 'error']); + expect(events[1].durationMs).toBeGreaterThanOrEqual(0); + expect(JSON.stringify(events)).not.toContain('555-01-0000'); + }); + it('runs the inner call even when the emitter throws', async () => { const toolMap = new Map([['read_file', createTool('read_file', async () => 'ok')]]); const instrumented = instrumentPtcToolMap({ diff --git a/packages/api/src/agents/ptc.ts b/packages/api/src/agents/ptc.ts index 28de7b4d26..a56e3f1221 100644 --- a/packages/api/src/agents/ptc.ts +++ b/packages/api/src/agents/ptc.ts @@ -86,6 +86,15 @@ export interface InstrumentPtcToolMapParams { /** The PTC run step's tool call id — the card the trace renders under. */ toolCallId: string; runId?: string; + /** + * Whether argument and failure previews may ride the stream. False when the + * deployment filters tool arguments for PII: inner calls never pass through + * `filteredToolArgumentsResult` (the sandbox bridge invokes them directly), + * so a preview would put values on the wire that the configured policy + * exists to keep off it — and a failure message routinely quotes the very + * argument that caused it. The trace still reports name, status and duration. + */ + includePreviews?: boolean; emit: (event: PtcToolCallEvent) => void; } @@ -101,6 +110,7 @@ export function instrumentPtcToolMap({ toolMap, toolCallId, runId, + includePreviews = true, emit, }: InstrumentPtcToolMapParams): Map { let sequence = 0; @@ -132,7 +142,7 @@ export function instrumentPtcToolMap({ call_id: callId, name, status: 'running', - args: summarizePtcArgs(input), + ...(includePreviews ? { args: summarizePtcArgs(input) } : {}), ...(runId != null ? { runId } : {}), }); try { @@ -154,10 +164,14 @@ export function instrumentPtcToolMap({ call_id: callId, name, status: 'error', - error: clip( - error instanceof Error ? error.message : String(error), - ERROR_PREVIEW_MAX_CHARS, - ), + ...(includePreviews + ? { + error: clip( + error instanceof Error ? error.message : String(error), + ERROR_PREVIEW_MAX_CHARS, + ), + } + : {}), durationMs: Date.now() - startedAt, ...(runId != null ? { runId } : {}), });