refactor(server): resolve self invocations through one shared helper - #12946
Conversation
The ACP adapter assembled `process.execPath` plus `argv[1]` by hand and then special-cased the single-executable with `node:sea`, while the rest of the server already answers "are we the packaged binary" through `HostProcessIsExecutable`. Add `resolveSelfInvocation` beside `resolveNodeExecutable` in `@t3tools/shared/nodeRuntime` so there is one place that knows how another process runs this install's CLI, and thread the resolved value into the ACP adapter instead of reading the process in the adapter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| import * as Schema from "effect/Schema"; | ||
|
|
||
| import { | ||
| HostProcessArguments, |
There was a problem hiding this comment.
Import this local Effect service module as a namespace rather than adding another service tag as a named import. Please use import * as HostProcess from "./hostProcess.ts" and reference its tags through that public namespace (for example, yield* HostProcess.HostProcessArguments), updating the existing host-process references in this file accordingly.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
This file already imported HostProcessEnvironment, HostProcessExecutablePath, HostProcessIsExecutable and HostProcessPlatform by name before this PR (same on main). Adding HostProcessArguments to that list matches the file; switching the whole file to a namespace import is unrelated churn for this change.
|
Effect Service Conventions found one issue in Posted via Macroscope — Effect Service Conventions |
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This PR centralizes existing ACP self-invocation logic and corrects packaged-binary handling without changing schemas, defaults, or production infrastructure. The runtime effect is narrowly limited to launching the existing MCP bridge and is covered by focused tests. You can add or adjust custom eligibility rules. Learn more. |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: unavailable · PR result: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
d7df317
into
t3code/codex-turn-mapping
…12946) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…12946) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…12946) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…12946) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…12946) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…12946) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…12946) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…12946) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…12946) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…12946) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
#12747 and #12791 each fixed one place where the server spawned itself with
process.execPathplusargv[1], which breaks under the single-executable, and #12747 then reached fornode:seadirectly inside the ACP adapter. The rest of the server already answers "are we the packaged binary" through theHostProcessIsExecutablereference andresolveNodeExecutable, so this leaves three ways of asking the same question.This adds
resolveSelfInvocationnext toresolveNodeExecutablein@t3tools/shared/nodeRuntime: one Effect that returns{ command, entrypoint }fromHostProcessExecutablePath,HostProcessIsExecutableandHostProcessArguments, plusselfInvocationArgsto build the argv. The ACP adapter takes the resolved value as an option instead of reading the process, and the three adapter factories (Grok, Antigravity, ACP registry) resolve it in their Effect layer.AcpAdapterV2.tsno longer touchesprocess.execPath,process.argv, ornode:sea.The
/bin/shcgroup wrapper from #12791 is unaffected: it no longer spawns Node at all.Verified:
packages/sharednodeRuntime.test.ts(2 new cases, 11 total),apps/serverAcpAdapterV2.test.ts/AcpRegistryAdapterV2.test.ts/GrokAdapterV2.test.ts/AntigravityAdapterV2.test.ts(125),vpr typecheckin both packages, lint on touched files. The existing self-contained bridge test now providesHostProcessIsExecutableinstead of mockingnode:sea.Claude Fable 5 via Claude Code.
🤖 Generated with Claude Code