🖥️ feat: Stream PTC Inner Tool Calls as a CLI-Style Trace (#15115)

* 🖥️ feat: Stream PTC Inner Tool Calls as a CLI-Style Trace

Programmatic tool calling runs a whole program inside the sandbox, and the
tool calls that program makes open no run step of their own. The card showed
one running spinner for the entire execution, with no sign of what the code
was doing.

Emit a new `on_ptc_tool_call` step event for each inner invocation — once on
dispatch, once on settle — and render them under the code as a terminal-style
trace: status glyph, tool identity, argument preview, duration, with a failure
message printed under the call that produced it.

The seam is the tool map the sandbox bridge resolves inner calls against.
`instrumentPtcToolMap` proxies `invoke` on each entry and leaves every other
property (name, schema, mcp) passing straight through, so nothing about
execution changes and emission failures can never fail a tool call.

Client state is a per-tool-call Recoil atom keyed like the sandbox-starting
and subagent atoms — live for the session, cleared on conversation switch so
a finished program's trace stays readable.

* 🩹 fix: Address Codex Review on the PTC Tool Trace

Five findings, all confirmed against the source before fixing.

Scope the trace atoms to a message occurrence. The hook already documents
that providers repeat a tool_call_id across turns and even within one
message, and `call_id` restarts at :0 for every outer call — so two programs
sharing `call_0` merged into one card. Key by (response message id, tool call
id) via `ptcTraceKey`, mirroring `subagentProgressKey`; the event's `runId`
already carries the message id and the card reads its own from MessageContext.

Prune unsettled rows on resume. Inner calls are not content parts, so the
resume snapshot cannot rebuild them, and `trackReplayEvent` only persists
OAuth events — a call that settled during a disconnect left a spinner that
never resolved. Settled rows are real history and stay.

Make the argument preview budget-aware. Iterate keys rather than entries so
the budget check can actually skip work, and clip against a bounded window so
a multi-megabyte value is never collapsed in full to build a 40-character
preview.

Catch the resumable emission promise. The synchronous try/catch around the
emitter cannot observe a rejected `emitChunk`, so a failing transport raised
an unhandled rejection per event instead of dropping telemetry.

Announce completion to assistive technology. The check glyph is decorative and
a fast call renders no duration, so a settled row previously announced no
outcome; each row now carries an sr-only status and the visible cell that
duplicated it is hidden.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP

* 🧹 fix: Repair CI Failures on the PTC Tool Trace

Two failures on the previous head, both mine.

`Tests: api (shard 2/3)` — 46 failures in `initialize.spec.js`, all
`TypeError: createPtcProgressEmitter is not a function`. The suite mocks the
callbacks module with an object literal, and wiring the new emitter into
`initialize.js` without adding it there left the factory undefined at call
time. Added it alongside `createAttachmentEmitter`, plus an assertion that it
receives the same generation fence as every other resumable emitter — a stale
epoch would leak one run's inner calls into the next.

`Static checks` — import-order drift in `PtcToolTrace.tsx` and `handlers.ts`,
repaired with `scripts/sort-imports.mts`. ESLint and Prettier both passed, so
only the dedicated check caught it.

`openai.js` and `responses.js` never take the emitter, so their specs were
unaffected; verified the initialize mock now covers every name the module
destructures.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP

* 🔐 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

*  test: Keep PTC Trace Tests Aligned With Caller-Capability Filtering

Left out of the merge commit by a staging slip; without them
`handlers.spec.ts` fails on the merged tree.

`#15105` restricts the PTC tool map to tools whose `allowed_callers` admit
code execution, so the existing trace test's registry entry — which declared
none, defaulting to `direct` — was filtered out before the instrumentation
could see it. Declare the fixture `code_execution`.

Add a guard for the resolution itself: a `direct`-only tool must never appear
in the instrumented map. Tracing wraps the eligible map, and this fails if a
later change reorders that and lets the trace widen what the sandbox reaches.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP

* 🛡️ fix: Close Name Disclosure and Follow the PTC Trace Tail

Two findings from the third Codex pass on `17a9ec9`.

Redact filtered inner-tool names (P1). The previous gate suppressed argument
and failure previews but the event still carried `name` verbatim, so a
deployment whose `filters.toolArguments.pii.fields` includes `name` could see
a blocked identifier disclosed through the trace — the one path inner calls
take, since they never reach `filteredToolArgumentsResult`. Inner tool names
are now inspected once per PTC call with the same `extractToolArgumentContent`
+ `inspectContent` pair the executor uses; any that trip the policy are left
unwrapped, so they still execute and emit nothing. An un-inspectable name
fails closed.

Follow the trace tail (P2). The row list is a 200px scroller that never moved,
so once a program exceeded the viewport the card sat on the oldest calls while
live activity accumulated below the fold. Reuse `useFollowScroll` — the hook
the code and command panes already use — which pins to the tail while calls
are running and yields the moment the reader scrolls up. The host card threads
its disclosure state so a collapsed pane is never scrolled invisibly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP

* 📌 fix: Pin the PTC Trace Through Its Final Settle

The fourth Codex pass on `4bf68e1`, one P2 finding.

`useFollowScroll` returned early whenever `active` was false, so the one
change it most needed to follow was the one it skipped. A failing inner call
settles by appending its error line in the same commit that clears the last
running row: the content grows and the stream ends together, and the pin that
would have revealed that line never fired. On an expanded, bottom-pinned pane
the failure — the row a reader most wants — stayed below the fold.

The falling edge of `active` now pins too, but only when the content changed
with it. Ending a stream on its own still leaves the pane where the reader
left it, which is what the existing contract promises and what the sibling
code and command panes rely on; a reader who has scrolled up is untouched
either way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP

* 🔌 fix: Keep PTC Calls That Outlive a Reconnect

Fifth Codex pass on `085a83f`; one of its two findings.

Pruning rows across a resume gap deleted every `running` row, but a stream gap
is not proof the call ended. A call still executing across the reconnect
settles normally on the restored live stream — and `applyPtcToolCall` drops a
settle whose row is gone, by design, so an evicted row cannot reappear out of
order. The call therefore vanished from the trace despite having run, which is
worse than the spinner the pruning existed to prevent.

Rows are now marked `interrupted` instead of removed. A call whose settle was
genuinely lost in the gap reports that honestly rather than spinning forever,
and one that survives the gap settles onto the row it opened, reporting its
real outcome and duration. `interrupted` is a client-side conclusion, so it
widens the row status locally and leaves the wire contract alone.

Two cases added: the gap marks rather than drops, and a post-reconnect settle
lands on its marked row; plus a render case for the new outcome.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Danny Avila 2026-08-23 01:18:14 -04:00 committed by GitHub
parent caa938fec6
commit c2aa688d73
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
26 changed files with 1707 additions and 12 deletions

View file

@ -39,6 +39,7 @@ const {
const {
createToolEndCallback,
createAttachmentEmitter,
createPtcProgressEmitter,
createBackgroundCodeResultHandler,
getDefaultHandlers,
} = require('~/server/controllers/agents/callbacks');
@ -374,6 +375,7 @@ const initializeClient = async ({
updateToolCallResult: db.updateToolCallResult,
}),
emitAttachment: createAttachmentEmitter({ res, streamId, jobCreatedAt }),
emitPtcProgress: createPtcProgressEmitter({ res, streamId, jobCreatedAt }),
...getSkillToolDeps(),
};

View file

@ -49,6 +49,7 @@ const mockArtifactToolEndCallback = jest.fn();
jest.mock('~/server/controllers/agents/callbacks', () => ({
createToolEndCallback: jest.fn(() => mockArtifactToolEndCallback),
createAttachmentEmitter: jest.fn(() => jest.fn()),
createPtcProgressEmitter: jest.fn(() => jest.fn()),
createBackgroundCodeResultHandler: jest.fn(() => jest.fn()),
getDefaultHandlers: jest.fn((opts) => {
capturedDefaultHandlerOptions = opts;
@ -206,6 +207,7 @@ describe('initializeClient — processAgent ACL gate', () => {
it('threads the owning job epoch into resumable event handlers', async () => {
const {
createAttachmentEmitter,
createPtcProgressEmitter,
createToolEndCallback,
} = require('~/server/controllers/agents/callbacks');
mockInitializeAgent.mockResolvedValue(makePrimaryConfig([]));
@ -237,6 +239,13 @@ describe('initializeClient — processAgent ACL gate', () => {
streamId: 'conv_1',
jobCreatedAt: 1234,
});
/** The PTC trace emitter is generation-fenced like every other resumable
* emitter; a stale epoch would leak one run's inner calls into the next. */
expect(createPtcProgressEmitter).toHaveBeenCalledWith({
res: {},
streamId: 'conv_1',
jobCreatedAt: 1234,
});
mockLoadToolsForExecution.mockResolvedValue({ loadedTools: [], configurable: {} });
await capturedToolExecuteOptions.loadTools([], PRIMARY_ID);