mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-22 01:53:33 +00:00
🔐 fix: Address Second Codex Review on the PTC Tool Trace
Three of five findings actioned; two answered on the thread. Respect tool-argument PII filtering (P1). Inner calls never reach `filteredToolArgumentsResult` — the sandbox bridge invokes them directly — so the trace was the one path putting their values on the wire in a deployment that had configured `filters.toolArguments.pii`. When any of the name / arguments / output fields are filtered, the emitter now omits both the argument preview and the failure message, which routinely quotes the argument that caused it. Name, status and duration still report. Drop the light/dark-specific background (P1). `dark:bg-transparent` stepped outside the semantic roles and would lose the intended separation under a custom theme. The pane now sets no background at all and inherits the card's surface, which resolves to the same color the override produced in both default themes and stays correct when a theme reassigns its roles. Bound the live trace (P2). A program looping over a large collection made every event copy an ever-growing array and rendered a row per call. The trace now keeps a rolling tail of 100 rows and counts what it evicted, surfaced as "+N earlier calls" so the cap is never silent. A settle whose row is gone — evicted, or pruned across a resume gap — no longer reappears out of order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP
This commit is contained in:
parent
ebaf371b7b
commit
17a9ec9772
10 changed files with 219 additions and 49 deletions
|
|
@ -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 (
|
||||
<div className={cn('bg-surface-primary-alt p-4 text-xs dark:bg-transparent', className)}>
|
||||
<div className={cn('p-4 text-xs', className)}>
|
||||
<div className="mb-1.5 text-[10px] font-medium uppercase tracking-wide text-text-secondary">
|
||||
{localize('com_ui_ptc_trace_title')}
|
||||
</div>
|
||||
<ol className="max-h-[200px] overflow-auto font-mono" aria-live="polite">
|
||||
{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 && (
|
||||
<li className="leading-5 text-text-tertiary">
|
||||
{localize('com_ui_ptc_trace_earlier', { count: trace.dropped })}
|
||||
</li>
|
||||
)}
|
||||
{trace.entries.map((entry) => (
|
||||
<PtcTraceLine key={entry.callId} entry={entry} />
|
||||
))}
|
||||
</ol>
|
||||
|
|
|
|||
|
|
@ -152,7 +152,7 @@ describe('ExecuteCode programmatic tool trace', () => {
|
|||
<RecoilRoot
|
||||
initializeState={({ set }) => {
|
||||
set(store.autoExpandTools, true);
|
||||
set(ptcTraceByToolCallId(ptcTraceKey(MESSAGE_ID, TOOL_CALL_ID)), entries);
|
||||
set(ptcTraceByToolCallId(ptcTraceKey(MESSAGE_ID, TOOL_CALL_ID)), { entries, dropped: 0 });
|
||||
}}
|
||||
>
|
||||
<MessageContext.Provider value={{ messageId: MESSAGE_ID, isExpanded: true }}>
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
||||
|
|
|
|||
|
|
@ -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([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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<PtcTraceEntry[], string>({
|
||||
export const ptcTraceByToolCallId = atomFamily<PtcTrace, string>({
|
||||
key: 'ptcTraceByToolCallId',
|
||||
default: [],
|
||||
default: EMPTY_PTC_TRACE,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>)?.run_id as
|
||||
| string
|
||||
| undefined,
|
||||
includePreviews: !hasActivePiiFields(ptcArgumentPii, [
|
||||
'name',
|
||||
'arguments',
|
||||
'output',
|
||||
]),
|
||||
emit: emitPtcProgress,
|
||||
})
|
||||
: resolvedPtcToolMap;
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
|
|
@ -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<string, StructuredToolInterface> {
|
||||
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 } : {}),
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue