chore(agents): bump built-in Claude and Pi adapter ranges - #513
chore(agents): bump built-in Claude and Pi adapter ranges#513lucasyarza wants to merge 4 commits into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs maintainer review before merge. Reviewed August 28, 2026, 7:29 PM ET / 23:29 UTC. ClawSweeper reviewWhat this changesUpdates the built-in Claude and Pi ACP adapter ranges and maps their immediately superseded saved-session commands to the current launch arguments when no stored argv exists. Merge readinessThis PR remains necessary: current main and v0.13.2 still select the older adapter ranges. The compatibility mapping and focused coverage are sound; rebase the currently dirty head and rerun required CI before merge. Priority: P2 Review scores
Verification
How this fits togetherACPX resolves friendly agent names into ACP adapter processes and retains launch metadata with persistent sessions. On later reuse, saved command metadata is parsed into argv and the client selects an installed adapter or npm package-exec launch. flowchart LR
A[CLI agent name] --> B[Built-in command registry]
B --> C[Adapter launch selection]
D[Saved session record] --> E[Historical command mapping]
E --> C
C --> F[ACP adapter process]
F --> G[Persistent ACP session]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Rebase onto current main, retain the narrow legacy-command mappings and precedence behavior, then merge after the required checks validate the exact rebased head. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug report. The submitted terminal traces do provide high-confidence after-fix evidence for the changed launch selection and Claude legacy-session path. Is this the best way to solve the issue? Yes. Updating the registry at its existing ownership boundary and adding exact historical-command mappings is the narrow maintainable path; it preserves usable stored argv rather than silently rewriting user-selected launchers. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ef6b81e71756. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (6 earlier review cycles)
|
|
Thanks — all four review items are addressed in [P1] Migrate the superseded adapter commands. Confirmed and fixed. [P1] Resolve merge risk. Same fix. Persisted records that reconstruct argv through the exact-command map now land on the current built-in argv rather than the retired launcher. [P2] Synchronize the required harness documentation. Done. [P2] Remove adapter ranges from the changelog. Done. The Real behavior proof. Two runs against this branch's build, both in the PR body:
One thing outside my reach: the @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Both remaining items are addressed in [P2] Qualify the legacy-session migration guarantee. Correct finding, and my wording was wrong. [P1] Real proof of the legacy no-argv upgrade path. Added, and it is the run you asked for rather than another fresh launch. A real persisted record was rewritten to its pre-upgrade shape, The retired command resolved to the current built-in argv, the newly selected adapter launched, and the turn completed. The record afterwards holds Still not proven, same as before. A completed Pi model turn.
The @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Fixed in [P1] Preserve built-in resolution for legacy Claude commands. Confirmed against the source: Both retired forms are now in the Claude spec's Covered by a new test in Live confirmation on a legacy no-argv record, same setup as the previous resume proof: That is the package-exec bridge. Before the fix the same record spawned One thing I deliberately did not touch. The older Claude forms already in
Scratch sessions used for the proofs were deleted and the session index restored. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Advance the source-owned npm ranges for the Claude and Pi built-in adapters to their current npm releases: - `@agentclientprotocol/claude-agent-acp` `^0.60.0` -> `^0.70.0` - `pi-acp` `^0.0.31` -> `^0.0.33` The Codex (`^1.1.5`) and Mux (`^0.28.0`) ranges already float to the latest published releases (1.6.2 and 0.28.2), so they are left unchanged. Claude 0.61.0 through 0.70.0 and Pi 0.0.32 through 0.0.33 publish no breaking changes; the release notes contain features and fixes only. Built-in names, command routing, installed-package-first resolution, package-exec fallback, queueing, persistence, cancellation, and reconnect behavior are unchanged. No ACPX dependency, manifest, or lockfile changes. Co-Authored-By: Claude <noreply@anthropic.com>
Registering the new ranges alone left the outgoing launch commands unmapped, so persisted sessions recorded under them reconstructed no argv and kept launching the retired adapter line. Add the superseded forms to `LEGACY_AGENT_COMMANDS`: - `npx pi-acp@^0.0.31` - `npx -y @agentclientprotocol/claude-agent-acp@^0.60.0` - `npm exec @agentclientprotocol/claude-agent-acp@^0.60.0` Cover all three in the historical built-in command table in `test/session-persistence.test.ts`. Synchronize the harness documentation the range change requires: mark the Pi built-in as ACPX-range-owned in `skills/acpx/SKILL.md`, document the persisted command mapping there and in `agents/Claude.md`, add the missing `agents/Pi.md`, and link it first in `agents/README.md` per the mandatory example ordering. Drop the adapter version specifiers from the changelog entry, which the documentation policy forbids. Co-Authored-By: Claude <noreply@anthropic.com>
A usable stored `agent_argv` is resolved before the historical-command map, so a record saved under a retired command only picks up the current argv when it has no usable stored argv. The previous wording promised the migration for every such record. Qualify the claim in `agents/Claude.md`, `agents/Pi.md`, `skills/acpx/SKILL.md`, and the changelog entry, and lock the precedence with a regression test that a retired command plus usable stored argv keeps the stored launcher. Co-Authored-By: Claude <noreply@anthropic.com>
Backfilling argv was not enough. The client resolves built-in launch behavior from the unchanged persisted `agent_command`, and `findBuiltInAgentPackage` matches only `fallbackCommand` and `legacyFallbackCommands`. Neither retired `^0.60.0` form appeared there, so a saved Claude session without stored argv took a raw npx launcher and bypassed the installed-package-first and package-exec paths that recognized Claude commands use. Register both retired forms in the Claude spec's `legacyFallbackCommands`, which `resolveAgentArgvForCommand` already consults before `LEGACY_AGENT_COMMANDS`, and drop the now-duplicate entries from that map so each retired form has one home. Cover the launch path: package-exec resolution must recognize both retired commands and bridge them at the current package range. Co-Authored-By: Claude <noreply@anthropic.com>
c280ac2 to
19055a1
Compare
|
Rebased onto current The only conflict was The Re-verified on the rebased head after Registry state is unchanged since the review:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
What Problem This Solves
Fresh ACPX built-in launches of the Claude and Pi agents resolve to adapter releases that are no longer current. The repository-owned range for
@agentclientprotocol/claude-agent-acpsits ten minor lines behind the published release. The Pi range pins exactly, because a caret on a0.0.xversion cannot float, so it can never reach the published patch.Users on the built-in
claudeandpiagents therefore stay on older adapter behavior unless they override the built-in command themselves.Why This Change Was Made
Advance the two stale source-owned ranges in
ACP_ADAPTER_PACKAGE_RANGESto the current npm releases. The Codex and Mux ranges already float to their latest published releases, so they are deliberately left untouched.Advancing a range retires a launch command, so this PR also keeps the upgrade path intact:
legacyFallbackCommands, and the superseded Pi form inLEGACY_AGENT_COMMANDS. Persisted session records without usableagent_argvresolve through those exact-command lookups, so without the entries an older record would keep reconstructing the retired launcher instead of the current built-in argv.resolveBuiltInAgentLaunchreads the unchanged persistedagent_command, andfindBuiltInAgentPackagematches onlyfallbackCommandandlegacyFallbackCommands. Putting the retired forms there is what keeps those saved sessions on the installed-package-first and package-exec paths instead of a raw npx launcher.test/session-persistence.test.tscovers all three forms. A further test asserts the documented precedence, that a usable storedagent_argvbeats the historical-command map.test/agent-registry.test.tscovers the launch path: package-exec resolution must recognize both retired Claude commands and bridge them at the current package range.Harness documentation is synchronized per the repository policy: the Pi built-in is now marked as ACPX-range-owned in
skills/acpx/SKILL.md, the persisted-command mapping is documented there and inagents/Claude.md, the missingagents/Pi.mdis added, and it is linked first inagents/README.mdto respect the mandatory example ordering. No documentation surface publishes a version specifier or semver range, and the changelog entry is generic for the same reason.Both bumps are non-breaking. Every intervening release on the Claude line and the Pi line publishes features and fixes only, with no
BREAKING CHANGEsection. Notable additions picked up on the Claude line include the provider-neutral ACP goal extension, typed session failures, the model-fallback warning advisory,Skilltool call metadata, and provider switching for loaded sessions. On the Pi line,session/deletesupport and Pi session-settlement handling.Boundaries: built-in names, command routing, installed-package-first resolution, package-exec fallback, queueing, persistence, cancellation, and reconnect behavior are unchanged. No adapters are added as ACPX dependencies, and the package manifest and lockfile are untouched.
User Impact
Users running
acpx claudeandacpx piwithout an override now get the current adapter releases on a fresh launch. A session already persisted under a superseded launch command resolves to the current built-in argv instead of relaunching the retired adapter line, as long as its record carries no usable storedagent_argv. A record with usable stored argv intentionally keeps that launcher, and the docs now say so.Evidence
Real Claude launch, end to end
acpx claude execagainst this branch's build, in a scratch cwd, with--deny-all. Redacted transcript, adapter version and turn result verbatim:The launched adapter reports the newly selected release, the turn streams real model output, and it settles on
end_turn.Real legacy-session upgrade path, end to end
The path the migration exists for. A real persisted record was rewritten to its pre-upgrade shape,
agent_commandset to the retirednpx -y @agentclientprotocol/claude-agent-acp@^0.60.0andagent_argvremoved, then reused:The retired command resolved to the current built-in argv, the newly selected adapter launched, and the turn completed. The record afterwards:
The resolved argv is persisted, so the upgrade is durable rather than re-derived on every reuse.
After registering the retired forms for built-in recognition, the adapter the legacy record spawns is the package-exec bridge rather than a raw npx launcher:
Before the fix the same legacy record spawned
npm exec @agentclientprotocol/claude-agent-acp@^0.70.0, the raw npx form taken straight from the backfilled argv.Real Pi launch, handshake only
Same build, Pi CLI
0.84.2installed into a scratch prefix and put onPATH:Stated plainly: the newly selected Pi adapter launches and completes the ACP
initializehandshake at the expected release, butsession/newstops at Pi provider authentication on this machine, so I cannot show a completed Pi model turn. The launch and version resolution are proven; the model turn is not.Checks
The repo-local autoreview helper is clean on the branch:
The range and migration assertions were written first and confirmed failing against the old values before each fix:
Published versions confirmed against the registry at the time of writing, including the two ranges left untouched:
Note for CI: the
CIworkflow run on this PR is parked inaction_required, the first-time fork-contributor approval gate. It needs a maintainer to release it.Generated with Claude Code