Repository navigation
fix(cli): accept --server anywhere and never spawn a second managed service - #58391
kitlangton wants to merge 3 commits into
Conversation
…n configured port
…swords with their URLs
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verified: I reproduced both problems from the issue on the base, and they're gone with this PR. My setup: a running serve --service whose registration file was in a different XDG_STATE_HOME than the client's. On the base, plugin list tried to spawn a second service, service status said stopped, and session --server URL list printed the group help. With the PR, plugin list and service status both found the running service on the configured port, and --server worked before or after the subcommand (plugin add … --server is still rejected). The three test files pass (12/12). bun typecheck fails on both checkouts here (this bun has no check script); bun run typecheck:tsc is clean.
Public API: CLI flags only, backwards compatible. --server and --standalone are added to plugin list|check|update, mcp list|auth|logout and debug agents|config. Both flags now work in any position, including on the session and auth groups. debug heap-dump still takes only --server. No existing command line stops working.
Concerns (inline):
- The port check only happens in
ServerConnection.resolve, soservice start,service restartandpaircan still spawn a second service. - The TUI and
mini(replacemode) keep using an older service found only by its port. ServiceConfig.options()now probes over HTTP and its file-picking rules are hard to follow.
| yield* Effect.tryPromise(() => client.experimental.persistentPty.shutdown()) | ||
| }) | ||
|
|
||
| const resolveManaged = Effect.fnUntraced(function* (options: EnsureOptions, mismatch: NonNullable<Args["mismatch"]>) { |
There was a problem hiding this comment.
Only ServerConnection.resolve checks the configured port before spawning. service start, service restart and pair call Service.ensure(ServiceConfig.options()) directly. In the #56708 setup (registration under another XDG_STATE_HOME, with no config.env pointing at it), they still try to spawn a second service and hit the port-in-use error.
Could the "something healthy is already on the configured port" check go next to Service.ensure? That would cover every caller. It's also where the port-in-use error is raised today.
The ready/unauthorized/incompatible handling below is also written out twice, once for ignore and once for the other modes. A shared helper would keep them from drifting apart.
| return yield* Service.ensure(options) | ||
| } | ||
|
|
||
| const configured = yield* ServiceConfig.probeConfigured({ file: options.file }) |
There was a problem hiding this comment.
For mismatch: "replace" (the default TUI and mini), a service found only through the configured port is reused when it's on an older version, with just a warning. The version-mismatch restart (onStart("version-mismatch"), which the default handler uses to report a restart) never runs in that case. The same happens when options() settles on a secondary registration file, because version is then forced to undefined. Is that intended? If so, the update flow should probably say the old service is still running.
| @@ -132,20 +134,168 @@ const paths = Effect.gen(function* () { | |||
| }) | |||
|
|
|||
| export const options = Effect.fnUntraced(function* (input: { readonly checkVersion?: boolean } = {}) { | |||
There was a problem hiding this comment.
options() is called by every service command and by serve --service itself. It now reads several candidate registration files and runs Service.discover probes over HTTP to choose between them. The rules are hard to follow: the ~/.local/state candidate only counts when env.XDG_STATE_HOME and a password are both set and the registration's password matches. Can this be narrowed to the one case it's for, the state dir set in config.env? A short note on why the other candidates exist would also help.
There was a problem hiding this comment.
The new commit only stops the CLI from probing a registered service twice. It now uses the service it found instead of calling Service.ensure again. The new Service.reuse behaves the same way ensure does when it reuses a service: it accepts only a ready, compatible service with a matching version and clears a stale persistent-terminal handoff first. I found no new problems. The client service tests (26/26) and the CLI tests (12/12) pass, tsc -b is clean, and plugin list still connects to a running service registered under another XDG_STATE_HOME instead of spawning one.
Public API: besides the CLI flags from before, @opencode/client's Effect Service now also exports reuse. Both are additions, so nothing breaks.
My three earlier points are still open: the port check covers only ServerConnection.resolve, a service found only by its port isn't replaced in replace mode, and ServiceConfig.options() is complex.
Issue for this PR
Closes #58389
Type of change
What does this PR do?
With the managed service already running,
opencode plugin listtried to start a second service on the same database. It only failed because the port was taken.--serveralso only worked on leaf commands, and several commands that connect to a server didn't accept it at all.--serverand--standaloneare global flags on every command that connects to a server, so they work before or after the subcommand (session --server URL list). This adds them toplugin list|check|update,mcp list|auth|logoutanddebug agents|config.serve,service,plugin add|removeandmcp addstill reject them.ServerConnection.resolveprobes the configured service port. If a service is there, the CLI reuses it and warns on a version mismatch, the same wayrunalready does. If the service rejects the password, it fails with a clear error. It only spawns when nothing answers.serve --serviceappliesconfig.envbefore reading service options, so it resolves the same state dir that clients look in.localor snapshot build never reads the productionservice.json. Each password only goes to its own URL (config password to the configured port, registration password to the registration URL), andOPENCODE_PASSWORDisn't used on the managed path.How did you verify your code works?
bun test test/server-connection.test.ts test/debug-heap-dump.test.ts test/mini-config.test.tsinpackages/cli: 12 pass. New tests cover flag position, no spawn when a service is live on the port, channel isolation and password pairing. The last two fail without the fix.bun typecheck: clean.Screenshots / recordings
Not a UI change.
Checklist
Added by the opencode bot (AI-generated).
Fixes #53878
Fixes #56708
Related: #55908, #58141
Public API: adds --server/--standalone to plugin list|check|update, mcp list|auth|logout, debug agents|config, accepted in any position; adds Service.reuse to @opencode/client effect service (backwards compatible)