Repository navigation
fix(client): keep a newer background service instead of replacing it - #55434
reynold-hu wants to merge 1 commit into
Conversation
github-actions
Bot
added
the
needs:compliance
|
The following comment was made by an LLM, it may be inaccurate: |
github-actions
Bot
removed
the
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
This fixes the reported downgrade. I ran it from source with a 2.0.29 service and a bare 2.0.28 TUI. On the base, the TUI replaced the service and the splash said "updated to 2.0.28". With this change, the 2.0.29 registration stays untouched. The new decide() tests and the packages/client type check pass.
Public API: no signatures change. One behaviour change in @opencode/client: Service.ensure (both the Promise and Effect versions), when given an exact version string, now fails instead of replacing a running service with a newer major.minor.patch. Callers that relied on replacement get an error instead.
Concerns:
- A hard failure instead of reusing the service. An older bare
opencode(andmini) now refuses to start whenever a newer service is running, for example after Desktop or another CLI has upgraded it, until the user upgrades or passes--standalone. The service already reportscompatible, so reusing a newer compatible service, and failing only when it's incompatible, would avoid both the ping-pong and the lockout. #55429 also suggests this direction. See the inline comment. - Desktop is affected too.
decide()is shared. Desktop's initial connect passes its bundled CLI version, so an older Desktop now fails to start when a newer CLI owns the service. The error also tells the user torerun with --standalone, which means nothing in Desktop. Before, Desktop replaced the service. Please handle that caller, for example by keeping the error text neutral and checking Desktop's startup path.
Related: #53375
| // Replacing a newer service silently downgrades whatever started it (for example a | ||
| // Desktop-managed host) and starts a version ping-pong. The older client steps aside | ||
| // instead of winning by starting last. | ||
| if ( |
There was a problem hiding this comment.
This turns a silent downgrade into a hard failure for the older client. After this, a bare opencode (or mini) older than the running service can't start at all without --standalone. That situation is common once Desktop or another install has upgraded the shared service. Since service.compatible is already known here, could a newer compatible service be reused (with PtyHandoff.complete, as in the reuse path), and only an incompatible one fail? That keeps the newer service in place and leaves the client usable.
This branch also runs for every ensure() caller that passes a version string, including Desktop's initial connect in packages/desktop/src/main/service/background-service.ts (it passes cli.version). An older Desktop would now fail to start against a newer CLI's service, with an error suggesting --standalone, which a Desktop user can't use.
|
Temporarily closed while this repository's open pull request limit is reached. The fix is unchanged; I will reopen it as soon as a slot frees. |
Issue for this PR
Closes #55429
Type of change
What does this PR do?
An older client could silently replace a newer registered service with its own binary — the bare
opencodeTUI hardcodesmismatch: "replace", anddecide()treated any version inequality as "replace", so whoever started last won. On a Desktop-managed host this dropped the Desktop connection and started a version ping-pong (older service spawns, Desktop restarts it, repeat).decide()now checks the direction of the mismatch: when the running service is newer than this client, the client fails with a message naming the service version instead of replacing it. Replacing an older service (the normal upgrade path) and versions that cannot be ordered (dev builds, predicate requirements) keep the existing behavior.How did you verify your code works?
packages/client/test/service-decision.test.ts: a newer service fails inreadyandwaitingstates; an older service still replaces;devbuilds and predicate requirements still replace.bun run typecheckpasses inpackages/client, and the fullpackages/clientsuite matchesdevon this machine (no new failures).Screenshots / recordings
Not a UI change.
Checklist
Added by the opencode bot (AI-generated).
Related: #53375
Public API: @opencode/client Service.ensure now fails instead of replacing a newer running service when given an exact version (behaviour change, no signature change)