[A06] Tool Runtime refactoring discussion and implementation plan #4909

Activity

1. testikun commented on Sep 6, 2026

   

   testikun

   on Sep 6, 2026

   Last edited by testikun

   ContributorAuthor

   More actions

   a06-refactor-analysis-1.zh-CN.md

   A06 overall analysis and refactoring proposal

   A06 covers tool-runtime.ts (4,192 lines) and computer-use-tools.ts (2,871 lines), 7,063 lines in this checkout. These files grew as the runtime added Code Mode nesting, permissions and sandbox boundaries, deferred tools, loop gates, subagent limits, managed mutation, durable T1/T2 commits, interactions, telemetry/artifacts and Computer Use. Each addition was reasonable; the current problem is that one executeTool() now owns overlapping decisions about identity, admission, preparation, execution, result projection, persistence and publication.

   The author’s runtime design treats events and durable state as part of the runtime contract. A tool call therefore has identity, argument views, admission outcome, durable lifecycle, model-visible history, UI events and recovery behavior. The existing flow is:

   settleToolCall → snapshot args → permission/persisted/model projections → guards → preparation → T1 → invoke → result projection → T2 → messages/events/telemetry/artifacts → loop/sandbox/CU state

   The first stage must preserve this flow, not redesign it. Current code already recursively snapshots/freezes caller input, computes persistedArgs once and assigns modelFacingArgs = persistedArgs. The goal is to give this existing contract a named internal owner. executionArgs goes to the implementation, permissionArgs to policy/signature logic, persistedArgs to messages/events/durable records, and modelFacingArgs to model replay. They must remain separate ownership views with private output copies.

   The target architecture keeps ToolRuntime as a facade and gradually assigns responsibility to ToolCallCoordinator, ToolAdmission, ToolPreparation, ToolExecutor, ToolResultProjector, ToolDurability and InteractionRegistry. Computer Use can later separate protocol, session, frame, dispatch and presentation. These are different invariants: admission is a pre-execution decision, durability is a commit/recovery protocol, result projection transforms outputs, interaction pauses/resumes execution, and Computer Use session/frame state is not an ordinary tool result.

   The stages are:

   1. Call-data seam: existing immutable snapshot, argument views and common event/message metadata (tracked in #4908).
   2. Typed admission outcomes without changing refusal behavior.
   3. T1/T2 durability and recovery ownership.
   4. Interaction request/settle ownership.
   5. Computer Use session/frame/dispatch boundaries.
   6. Facade cleanup after the seams are proven.

   Every stage is a separate reviewable issue, adds no product capability, preserves public behavior and reports measured source changes. Moving code out of a file is reported as extraction; net deletion requires a before/after count. This issue is the design and coordination entry point; implementation belongs in child issues.
2. testikun commented on Sep 6, 2026

   

   testikun

   on Sep 6, 2026

   ContributorAuthor

   More actions

   For the complete analysis, code-growth history, invariants, and detailed rationale, see the attached a06-refactor-analysis-1.zh-CN.md document above.
3. Joob1n commented on Sep 6, 2026

   

   Joob1n

   on Sep 6, 2026

   Member

   More actions

   Thanks @testikun — the plan is solid, and #4908 is clear to start.

   I checked the numbers against the tree: 4,192 + 2,871 = 7,063 matches, the pinned line regions match, and the invariants you list come from comments already in the code rather than from a fresh reading of it. That, plus the places where you decline to overclaim — no missing-clone claim, no promised deletion count, the Computer Use case framed as a regression invariant rather than a live bug — is why I am comfortable with the scope.

   One sequencing risk you did not list. #4879 (@Astro-Han, refactor(runtime): derive Session transcripts from RuntimeEvents, +2,763/-5,015, already approved) removes appendMessage call sites across the runtime and touches tool-runtime-argument-ownership.test.ts. tool-runtime.ts is not in its diff, so appendCallMessage() likely survives this round — but the direction points straight at your problem #2.

   If ToolCallMessage stops being separately written, that duplicated common-field recipe gets deleted, not consolidated. So I would split #4908:

   • Problem Add Rive workflow Maka tool #1 (name the four argument views, extract the snapshot module) — go ahead now. It is internal to executeTool() and unaffected.
   • Problem Harden runtime, storage, gateway, credentials, and IPC inputs #2 (merge the two common-field recipes) — check with @Astro-Han whether the tool_call message survives refactor(runtime): derive Session transcripts from RuntimeEvents #4879 before writing it.

   Three questions about A06 as a whole. These belong here, and they do not block #4908:

   1. What is the net line target when A06 is done? [A06] Extract an immutable tool-call snapshot and centralize argument projections #4908 will add lines, which is correct for a seam. But six extraction stages can end at more lines spread over more files, and net reduction is the only acceptance criterion this sweep has.
   2. Are the seven classes in the target architecture a target or a hypothesis? If the seams show only three boundaries are real, you should be free to stop at three. [A06] Extract an immutable tool-call snapshot and centralize argument projections #4908 already declines to introduce them, which is the right call — I would like the plan to keep that freedom explicitly.
   3. Which stages are in scope for 2026-09? Six stages across 7,000 lines is likely more than one month. Carrying the rest forward is fine; the sweep is built to expect that.
4. testikun commented on Sep 7, 2026

   

   testikun

   on Sep 7, 2026

   ContributorAuthor

   More actions

   @Joob1n Thanks for the careful review. I rechecked the plan against #4726, #4879, the current #4937 implementation, and the latest main branch.

   1. Net line target

   I agree that A06 should leave the area smaller at the end of the sweep. I do not want to invent a fixed percentage or deletion count before the duplicate-rule inventory is complete.

   I will make the acceptance explicit at two levels:

   • Each implementation PR must report its measured result according to tracking(cleanup): monthly scavenger sweep — 2026-09 #4726: deleted duplicate rule, unique owner, seam extraction, or dead-code removal.
   • The completed A06 work must have a net reduction in aggregate production code relative to a re-measured baseline. Intermediate extraction PRs may temporarily add lines, but that increase must be reported and recovered by the later slices.

   The current main baseline is 4,197 lines for tool-runtime.ts and 2,871 for computer-use-tools.ts (7,068 physical lines). I will re-measure physical and non-comment lines at the final integration point rather than relying on the older 7,063-line number.

   2. Target architecture

   The seven owners are hypotheses about stable responsibility seams, not a requirement to create seven classes.

   I will update the plan to say that a boundary is kept only when it has an independent state owner, invariant, dependency direction, and measurable reduction in duplicated reasoning. If the implementation proves that three or five boundaries are sufficient, the plan should stop there. Empty service classes or file-only moves do not count.

   #4937 intentionally follows this rule by introducing only the call-data module and not creating the full coordinator/admission/durability class hierarchy up front.

   3. September 2026 scope

   I agree that implementing all six stages this month would be too broad. For this month I will scope A06 to:

   • the call-data seam;
   • the sequencing decision with refactor(runtime): derive Session transcripts from RuntimeEvents #4879;
   • one merged behavior-preserving slice with measured results;
   • follow-up issues and baselines for admission, durability/interactions, and Computer Use.

   The later stages will be carried forward rather than treated as September acceptance requirements.

   There is one sequencing detail I will add explicitly. On current main, ToolRuntime still writes ToolCallMessage, but the current #4879 head removes that path. Therefore #4908 should be split:

   • proceed now with the immutable snapshot and argument-view extraction;
   • defer common-field consolidation until refactor(runtime): derive Session transcripts from RuntimeEvents #4879 lands or its final writer shape is confirmed.

   If ToolCallMessage disappears, that part should be reported as deletion of a redundant recipe, not as a new shared helper.

   I will update #4909/#4908 to reflect these constraints and adjust #4937 accordingly.
5. Astro-Han

   added

   trackingTracking or umbrella issueTracking or umbrella issue

   on Sep 17, 2026
6. Astro-Han

   added a commit that references this issue on Sep 27, 2026

   fix(runtime): accept sequence observations and share bound execution (#…

   ...

   a0ce9aa
7. github-actions

   added

   staleNo qualifying activity within the lifecycle policy windowNo qualifying activity within the lifecycle policy window

   on Oct 8, 2026
8. github-actions commented on Oct 8, 2026

   

   github-actionsbot

   on Oct 8, 2026 – with GitHub Actions

   More actions

   This issue has had no human activity for 30 days and has been marked stale. It will be closed in 30 days unless someone comments.

   If the issue is still current, please confirm it against the latest main and add any information that would help move it forward. Assigned issues and issues labelled pinned are exempt from this policy.