Synced from GitHub
refactor(acp): keep protocol implementation exports private#10165
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.
Added 1 commit
- refactor(acp): keep protocol implementation exports private9fecf16
Synced from GitHub
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.
Synced from GitHub
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.
Synced from GitHub
| … | … | @@ -254,7 +254,7 @@ interface AcpCoreAgentRequestHandlers { | |
| 255 | 255 | const decodeCancelNotification = Schema.decodeUnknownEffect(AcpSchema.CancelNotification); | |
| 256 | 256 | ||
| 257 | - | export const make = Effect.fn("effect-acp/AcpAgent.make")(function* ( | |
| 257 | + | const make = Effect.fn("effect-acp/AcpAgent.make")(function* ( | |
src/agent.ts:257
agent.ts owns construction of the AcpAgent service, so make should stay part of the module's public shape alongside layer/layerStdio (the sibling client.ts still exports its make). Consumers that need to build an agent inside an existing scope rather than via a layer lose that entry point here. Consider keeping the export:
suggestion
| 1 | export const make = Effect.fn("effect-acp/AcpAgent.make")(function* ( |
No longer relevant as of 913f03875dda49579b78d72ec85471b5c3ad2b7f
Restored the public agent constructor in 913f038. The adjacent @public comment explains that construction is part of the canonical Effect module API. The expanded Knip check passes with the constructor retained.
Added 1 commit
- refactor(acp): retain the public service constructor913f038
Synced from GitHub