fix(client): preserve compatible background service#36583
Merged
Conversation
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.
What
Stop a transient health-check failure from turning a healthy, same-version background service into a destructive restart. Concurrent CLI windows now converge on the existing service instead of one window invalidating an endpoint another window has already started using.
Before / After
Trigger: A background service running version
Vis healthy overall, but CLI A's first version-gated health probe transiently fails or times out. Before CLI A performs its fallback probe, CLI B successfully resolves that same service and begins using its endpoint.Before: CLI A performed a second, ungated health probe. Even when that probe succeeded and reported the requested version
V,Service.start()inferred that every successful fallback result was a version mismatch. It invoked the replacement callback, killed the service, and spawned another one. This produced both observed symptoms:Vto the identical versionV.ClientError: Transport.After: The fallback probe retains the version reported by a modern health response. When it matches the requested version,
Service.start()returns the existing endpoint without invokingonStart, killing the service, or changing its registration. Both CLI windows keep using the same live process.Genuine version mismatches still replace the daemon. Legacy health responses retain their existing replacement behavior. Missing or unhealthy services still follow the existing spawn path. Concurrent daemon spawning remains protected by the existing server-side election lock.
How
packages/client/src/effect/service.tscarries the health response's version through the private probe result and explicitly checks compatibility before replacement.packages/client/test/service.test.tsdeterministically pauses CLI A's first probe while CLI B resolves the service, then verifies the fallback does not fireonStart, replace the atomic registration, change PID/version, or kill CLI B's endpoint.packages/client/test/fixture/service.tsprovides atomic modern and legacy registrations plus the health-probe barrier..changeset/calm-services-start.mdrecords the Client patch.sequenceDiagram participant A as CLI A participant B as CLI B participant S as Background service V A->>S: version-gated health probe S--xA: transient failure B->>S: version-gated health probe S-->>B: healthy, version V B-->>B: keep endpoint S A->>S: ungated fallback probe S-->>A: healthy, version V A-->>A: compare V == V A-->>A: keep endpoint SScope
This PR fixes the destructive same-version classification that produced the equal-version update and invalidated a concurrently resolved endpoint. Broader pre-existing server registration-finalizer and prolonged multi-probe recovery races are not changed here.
Testing
cd packages/client && bun run test test/service.test.tscd packages/client && bun typecheckcd packages/cli && bun run test test/service.test.tscd packages/client && bun run test: 25 passed; 2 unrelated existing failures expectserver.mcpwhile the generated client exposesmcp.bunx prettier --check packages/client/src/effect/service.ts packages/client/test/service.test.ts packages/client/test/fixture/service.ts .changeset/calm-services-start.md