Skip to content

refactor(acp): keep protocol implementation exports private - #10165

Merged
juliusmarminge merged 2 commits into
mainfrom
cleanup-packages/acp-internals
Sep 5, 2026
Merged

refactor(acp): keep protocol implementation exports private#10165
juliusmarminge merged 2 commits into
mainfrom
cleanup-packages/acp-internals

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 5, 2026

Copy link
Copy Markdown
Member

The private ACP package exports individual RPC definitions and implementation helpers that no repository consumer imports.

Keep those definitions local and remove the unused client layer wrapper. The live client factory, child-process layer, agent constructor/layers, and RPC groups remain exported. The agent constructor has a local @public explanation because the repository's Effect service convention requires public construction APIs. Generated upstream protocol definitions are untouched.

Verification on the integrated stack:

  • ACP Knip export scan passes.
  • All 40 existing ACP tests pass.
  • ACP typecheck, targeted lint, and formatting pass.

No tests or runtime protocol behavior changed. This is the first cleanup layer; #10171 enforces exports across all seven internal packages.

Model: gpt-6 astra. Harness: Codex in T3 Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 5, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 5, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 913f038

Macroscope's review found this PR approvable — This is a localized private-API cleanup that hides implementation details without changing ACP protocol behavior or existing in-repository call paths. The only removed helper is an unused client layer wrapper, while the supported factories and RPC groups remain available.

You can add or adjust custom eligibility rules. Learn more.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.6 KiB 13.6 KiB +2 B (+0.0%) 15.1 KiB
Codex Thread snapshot wire 7.0 KiB 7.0 KiB +1 B (+0.0%) 7.3 KiB
Codex Live turn WebSocket wire 6.6 KiB 6.6 KiB +1 B (+0.0%) 7.8 KiB
Codex Live turn WebSocket decoded 57.1 KiB 57.1 KiB 0 B (0.0%) 66.4 KiB
Codex Live turn messages 10 10 0 (0.0%) 21
Claude Total thread wire 13.6 KiB 13.5 KiB −12 B (−0.1%) 15.1 KiB
Claude Thread snapshot wire 7.0 KiB 7.0 KiB +4 B (+0.1%) 7.3 KiB
Claude Live turn WebSocket wire 6.5 KiB 6.5 KiB −16 B (−0.2%) 7.8 KiB
Claude Live turn WebSocket decoded 57.8 KiB 57.8 KiB −44 B (−0.1%) 66.4 KiB
Claude Live turn messages 9 8 −1 (−11.1%) 21

Baseline: 62ed748 · PR result: 913f038 · Source CI: success

Scenario and decoded snapshot size

10 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.

  • Codex decoded thread snapshot: 113.8 KiB
  • Claude decoded thread snapshot: 114.5 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@juliusmarminge juliusmarminge changed the title cleanup packages/acp internals refactor(acp): keep protocol implementation exports private Sep 5, 2026
Comment thread packages/effect-acp/src/agent.ts Outdated
@macroscopeapp
macroscopeapp Bot dismissed their stale review September 5, 2026 17:04

Dismissing prior approval to re-evaluate 913f038

@juliusmarminge
juliusmarminge merged commit cb9a694 into main Sep 5, 2026
25 checks passed
@juliusmarminge
juliusmarminge deleted the cleanup-packages/acp-internals branch September 5, 2026 17:18
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 5, 2026
## What's Changed
* test(web): drop provider banner styling assertions by @juliusmarminge in pingdotgg/t3code#10148
* refactor(client-runtime): remove unused relay token waiter by @juliusmarminge in pingdotgg/t3code#10151
* test(web): drop sidebar artwork styling snapshots by @juliusmarminge in pingdotgg/t3code#10152
* refactor(ssh): keep package internals private by @juliusmarminge in pingdotgg/t3code#10144
* ci: reject unused SSH exports with Knip by @juliusmarminge in pingdotgg/t3code#10145
* refactor(acp): keep protocol implementation exports private by @juliusmarminge in pingdotgg/t3code#10165
* fix(shared): validate cloudflared with the version subcommand by @maria-rcks in pingdotgg/t3code#9880
* fix(desktop): separate LAN and Tailscale pairing endpoints by @maria-rcks in pingdotgg/t3code#9882
* fix(server): install pinned runtime when pnpm node lacks npm by @maria-rcks in pingdotgg/t3code#9923
* fix(web): hide sidebar search shortcut on mobile by @maria-rcks in pingdotgg/t3code#9932


**Full Changelog**: pingdotgg/t3code@v0.0.39-nightly.20260905.1288...v0.0.39-nightly.20260905.1289

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.39-nightly.20260905.1289
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant