Skip to content

refactor(runtime): extract tool call argument boundary - #4971

Open
testikun wants to merge 1 commit into
apache:mainfrom
testikun:codex/issue-4908-call-data-v2
Open

refactor(runtime): extract tool call argument boundary#4971
testikun wants to merge 1 commit into
apache:mainfrom
testikun:codex/issue-4908-call-data-v2

Conversation

@testikun

@testikun testikun commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Refs #4908
Refs #4909

Summary

This is the pre-#4879 call-data slice of A06.

It extracts the existing tool-call argument snapshot, declared-schema validation, permission projection, and Computer Use model-facing projection into one small internal module. The public ToolRuntime API and execution pipeline remain unchanged.

The slice intentionally does not touch transcript/message ownership or common-field construction. In particular, it does not change appendMessage, ToolCallMessage, ToolResultMessage, RuntimeEvent schemas, T1/T2, or the session_messages publication path. This keeps the change independent of #4879, which is converging ordinary transcript facts on RuntimeEvent plus a projector.

What is preserved

  • Recursive frozen snapshots and cycle/non-data-property rejection.
  • Synchronous direct-only and step registration before the first await.
  • Direct-only nested calls skip validation and permission projection.
  • Ordinary admission failures keep the existing projection order.
  • Unavailable sandbox-boundary surfaces retain deferred validation behavior.
  • Separate execution, permission, persisted, and model-facing argument ownership.
  • Existing Computer Use field-name and privacy projections.

Measurements

  • tool-runtime.ts: 4,197 baseline lines on current main; 4,086 after extraction.
  • computer-use-tools.ts: unchanged at 2,871 lines.
  • New argument module: 172 lines.
  • The reduction in tool-runtime.ts is reported as extraction; no unsupported net-deletion claim is made.
  • Common-fields consolidation is intentionally deferred until the final refactor(runtime): derive Session transcripts from RuntimeEvents #4879 writer shape is known.

Verification

Using Node 24.19.0:

  • core, storage, and runtime TypeScript builds passed;
  • Biome check passed;
  • git diff --check passed;
  • 22 targeted Runtime tests passed, including the argument-ownership and async snapshot regressions, settlement, Computer Use model-loop, and Computer Use privacy-boundary suites.

The full workspace test suite was not run because this PR is intentionally limited to the Runtime call-data seam.

@testikun
testikun force-pushed the codex/issue-4908-call-data-v2 branch 2 times, most recently from 4a6b357 to ef9da46 Compare September 7, 2026 09:37
@github-actions github-actions Bot added the effort/L Under 1000 readable lines label Sep 7, 2026
@testikun
testikun force-pushed the codex/issue-4908-call-data-v2 branch from ef9da46 to 66f2b36 Compare September 7, 2026 09:45
@testikun
testikun force-pushed the codex/issue-4908-call-data-v2 branch from 66f2b36 to 36feaa7 Compare September 7, 2026 09:52
@testikun

testikun commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Why this extraction exists

This change is not intended to create four independently transformed or redacted copies of every tool argument. The four names describe different consumers of the same call data:

  • executionArgs: the stable snapshot used by the implementation or managed transform;
  • permissionArgs: the projection used by permission and policy logic;
  • persistedArgs: the call shape recorded in RuntimeEvent/durable data;
  • modelFacingArgs: the call shape replayed to the model, currently identical to persistedArgs.

For ordinary tools these values may be identical. They diverge when a tool has a permission projection, and Computer Use additionally applies its existing privacy and accepted-field-name projection before persistence/model replay.

The extraction gives that existing relationship one internal owner. Previously it lived inline in executeTool(), alongside admission, client preparation, managed mutation, T1/T2, execution and publication. Because every view is typed as unknown, using the wrong view would normally compile and surface later as a permission, replay or privacy regression. tool-call-snapshot.ts owns only snapshot/validation/projection construction; it does not own admission, dispatch identity, durability, transcript publication or execution.

A separate module is used because this is a pure data boundary with no ToolRuntime state. Keeping the helpers in the 4,197-line lifecycle module would shorten executeTool() but leave the rule without an independently readable and testable owner. The boundary remains deliberately small: no full call object, no runId/operationId, no service classes, and no common-field abstraction.

This is also intentionally independent of #4879. It does not change appendMessage, ToolCallMessage, ToolResultMessage, RuntimeEvent schemas or the transcript path. #4879 may remove one consumer, but RuntimeEvent, durable preparation and model replay still need the same argument views.

Measured production change: tool-runtime.ts 4,197 -> 4,086 lines, plus a 172-line internal module, for a temporary net +61 production lines. The extraction is not presented as net deletion. Test changes are kept on the real settleToolCall() entry point; the new async case proves the provider input is snapshotted before schema validation yields, and it fails if the entry snapshot is removed.

@testikun

testikun commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

@Astro-Han could you please review this specifically against the direction of #4879?

The intended boundary is entirely before transcript publication: it extracts the existing argument snapshot, declared validation, permission projection and persisted/model-facing projection, while leaving appendMessage, call/result messages, RuntimeEvent construction and T1/T2 unchanged. The main question is whether this remains a clean precursor to #4879, or whether any part of the argument-view ownership should wait for the RuntimeEvent-only transcript cutover.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/L Under 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant