Repository navigation
fix(pi): refuse a wire protocol pi cannot speak, and surface pi's own turn error - #4
Merged
Merged
Conversation
… turn error Fixes #2. On the pi engine, selecting a real dsh model failed every turn with `pi RPC process exited unexpectedly`. Two independent defects were behind it. The reported one: `modelsDocument()` wrote dsh's `api` string into pi's `models.json` verbatim, while codex (`CODEX_WIRE_APIS`) and kimi (`KIMI_PROVIDER_TYPES`) each translate that value into their own vocabulary first. pi resolves `api` against its own registry of implementations, so a spelling outside its nine-entry `Api` union is refused outright. Verified against the pinned `@earendil-works/pi-coding-agent@0.84.3` by driving `pi --mode rpc` with three `models.json` variants: `api: "anthropic"` answers `No API provider registered for api: anthropic` — byte-identical to a deliberately bogus control value — while `anthropic-messages` is accepted and really dials the endpoint. The vocabulary belongs to whichever provider plugin owns that settings section, and dsh's own `llm-pi-ai` routes happen to spell things pi's way, which is why verbatim pass-through looked correct until a third-party provider spelled the same wire differently. The second one is why the report had to reverse-engineer the cause: the driver never read pi's own error. `PiMessage` did not declare `stopReason`/ `errorMessage` and nothing in `src/` touched them, so pi's precise sentence was discarded and the step ended through the RPC client's generic `onExit` message instead. Worse, a run whose every turn failed but which then settled cleanly was reported as a *completed* empty turn. `resolvePiApi` now judges the protocol — `absent` / `supported` / `unsupported` — and the `models.json` writer takes a `PiSupportedHandover` whose `api` is a required pi spelling, so "what lands on disk is always something pi accepts" is a compile-time obligation rather than a runtime check. A known alias (`anthropic` → `anthropic-messages`) is translated; anything still unrecognised fails the step with `PI_UNSUPPORTED_API`, naming the provider, the model and the offending value. It deliberately does not drop the handover the way a protocol-less one is dropped: dropping would leave pi quietly running its own default model while the user believes their dsh selection took effect — the silent degradation this driver exists not to produce. pi's turn verdict is now recorded per turn and reported after the loop (`PI_TURN_ERROR`). It is deliberately not thrown on the first errored `turn_end`: pi retries by itself and a later turn can succeed, which the new spec pins. Both new branches are covered, and the two compat lines that also ship this build pass unchanged. Also updates `docs/engine-pi.md` (§2/§5/§8/§9), `docs/driver-core.md` and `docs/architecture.md` in the same change, per the repo's documentation rule.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2.
What was wrong
Selecting a real dsh model on the pi engine failed every turn with
pi RPC process exited unexpectedly. Two independent defects.1. The
apivocabulary was passed through untranslated.modelsDocument()wrote dsh'sapistring into pi'smodels.jsonverbatim. Codex and Kimi each translate that value into their own vocabulary first (CODEX_WIRE_APIS,KIMI_PROVIDER_TYPES); pi was the only engine that didn't. pi resolvesapiagainst its own registry of implementations, so a spelling outside its nine-entryApiunion is refused outright.Verified against the pinned
@earendil-works/pi-coding-agent@0.84.3by drivingpi --mode rpcwith threemodels.jsonvariants (base URL deliberatelyexample.invalid, so "refused at load" and "really tried to connect" are distinguishable):apivalue"anthropic"stopReason: "error"→No API provider registered for api: anthropic"anthropic-messages"Connection error.+ 3 auto-retries — accepted, really dialled"totally-bogus-api"(control)No API provider registered for api: totally-bogus-api— identical to case 1The vocabulary belongs to whichever provider plugin owns that settings section. dsh's own
llm-pi-airoutes happen to spell things pi's way, which is why verbatim pass-through looked right until a third-party provider spelled the same wire differently (anthropic).2. The driver never read pi's own error.
PiMessagedidn't declarestopReason/errorMessageand nothing insrc/read them, so pi's precise sentence was discarded and the step ended through the RPC client's genericonExitmessage — which is why #2 had to be reverse-engineered from source. A run whose every turn failed but that then settled cleanly was also reported as a completed empty turn.The fix
resolvePiApijudges the protocol (absent/supported/unsupported), and themodels.jsonwriter now takes aPiSupportedHandoverwhoseapiis a required pi spelling — so "what lands on disk is always something pi accepts" is a compile-time obligation instead of a runtime check. A known alias (anthropic→anthropic-messages) is translated; anything else fails the step withPI_UNSUPPORTED_API, naming provider, model and offending value.It deliberately does not drop the handover the way a protocol-less one is dropped: dropping would leave pi quietly running its own default model while the user believes their dsh selection took effect — the silent degradation this driver exists not to produce.
pi's turn verdict is recorded per turn and reported after the loop (
PI_TURN_ERROR), deliberately not thrown on the first erroredturn_end, because pi retries by itself and a later turn can succeed. That premise is empirical — a retrying run emits one errored turn per attempt — and the new spec pins it.Verification
typecheck,build, andtest:coverageall pass locally, per-file 100% maintained: 68 files / 1298 tests.api: "anthropic-messages", pi accepted it and dialled the endpoint —0occurrences ofNo API provider registered.docs/engine-pi.md§2/§5/§8/§9,docs/driver-core.md,docs/architecture.md), per the repo's documentation rule.Not covered
The handover path is verified against the real pi binary, but not against a real endpoint —
anthropic-messagesbeing accepted proves the protocol is right, not that the reporter's endpoint answers. Confirming that needs theirmodlens-zai-coding-cnsetup.Also note this repo's first CI run happens on this PR.