Skip to content

fix(cli): harden managed service replacement - #41946

Open
kitlangton wants to merge 1 commit into
v2from
service-election-safety
Open

fix(cli): harden managed service replacement#41946
kitlangton wants to merge 1 commit into
v2from
service-election-safety

Conversation

@kitlangton

Copy link
Copy Markdown
Contributor

What

Prevent managed-service restart storms when a running TUI outlives an automatic CLI update, and harden lifecycle recovery so clients never terminate a process from registration data alone.

This fixes the incident where a next-17271 TUI remained alive after the installed package became next-17272. The old process repeatedly treated each 17272 service it spawned as incompatible, stopped it, and spawned the same newer binary again. The TUI repeatedly lost transport, migration polling mislabeled the disconnect as a data migration failure, and restart recovery added repeated continuation entries.

The lifecycle rules are now explicit:

Caller Registered service Behavior
fresh newer client older service authenticated exact-instance stop, then start installed version
fresh older client newer service bounded VersionMismatchError; never stop or spawn
explicit service restart newer service after deliberate downgrade authenticated exact-instance stop, then require installed older version
reconnecting TUI any modern version version-agnostic reconnect; never activate replacement
any client unresponsive or legacy service never send an OS signal; return manual recovery guidance
any client unknown/incomparable version non-destructive mismatch; replacement can never be mutual

Before / After

Before

  1. A next-17271 TUI launched and started an asynchronous update.
  2. The installed executable became next-17272, while the TUI process image remained 17271.
  3. The stale TUI spawned serve --service; the on-disk executable started 17272.
  4. The stale TUI observed 17272 as a mismatch and stopped it.
  5. It spawned the same 17272 executable again, repeating roughly every two seconds.
  6. Each short-lived server ran restart continuity, disconnected event streams, and made migration polling throw Transport.

After

  1. Version replacement has a directional policy.
  2. A newer fresh launch may activate an upgrade.
  3. An older fresh launch cannot automatically activate a downgrade and receives an actionable error.
  4. An explicit restart is the deliberate downgrade operation and requires the installed version after restart.
  5. Reconnect remains versionless and cannot join a replacement loop.
  6. Lifecycle code uses only authenticated /api/service/stop; it has no SIGTERM or SIGKILL path.
  7. Transport failure during migration polling is retried instead of reported as migration corruption.

How

  • packages/cli/src/services/service-config.ts
    • compares release and preview build numbers directionally;
    • treats missing, malformed, and incomparable versions as non-destructive;
    • guarantees that two versions can never both replace each other.
  • packages/client/src/effect/service.ts and packages/client/src/promise/service.ts
    • return VersionMismatchError when replacement policy protects the owner;
    • remove automatic unresponsive-owner eviction;
    • remove legacy and timeout-based OS signal fallback;
    • surface rejected, unsupported, timed-out, and non-exiting stop failures;
    • include the registration password in exact identity comparisons.
  • packages/client/src/promise/service.ts
    • validates registration and health structures with Effect-equivalent constraints;
    • rejects invalid PIDs, primitive health JSON, partial modern responses, and wrong field types.
  • packages/cli/src/commands/handlers/service/restart.ts
    • requires the installed version after explicit restart, including deliberate downgrade.
  • packages/cli/src/services/server-connection.ts
    • preserves versionless reconnect while making user-triggered restart require the launch version.
  • packages/tui/src/component/migration-overlay.tsx
    • retries transient ClientError("Transport") instead of showing Data migration failed.

Scope

This PR is the incident containment and client-side safety layer. It deliberately does not implement the larger ownership redesign already described in docs/design/service-lifecycle.md:

  • lifetime process-held OS service ownership;
  • registration self-repair instead of registration-driven owner shutdown;
  • globally coordinated contender spawning across client processes;
  • exact health instanceID and explicit lifecycle-state protocol fields;
  • automatic recovery of a truly frozen service without a safe OS process-identity primitive.

The last point is intentionally conservative: a stale registration PID may have been reused by another application, so this PR refuses to signal it.

Testing

  • cd packages/client && bun run test: 64 passed.
  • cd packages/client && bun typecheck: passed.
  • cd packages/cli && bun run test test/service.test.ts test/server-connection.test.ts --test-name-pattern 'managed version replacement|only newer clients|resolution groups|service options|concurrent service processes': 5 passed.
  • cd packages/cli && bun typecheck: passed.
  • cd packages/tui && bun run test: 661 passed, 5 skipped.
  • cd packages/tui && bun typecheck: passed.
  • High-risk lifecycle subset repeated 10 times on the committed code: no failures.
  • Version-order and anti-mutual-replacement subset repeated 50 times: no failures.
  • Pre-push repository typecheck: 32 packages passed.

The lifecycle battery covers:

  • twenty mixed Effect/Promise launchers converging on one replacement owner;
  • stale clients refusing to replace newer owners;
  • explicit downgrade activation through restart;
  • ready, starting, failed, unresponsive, rejecting, timeout, and legacy owners;
  • malformed registrations and health payloads;
  • no automatic eviction and no OS signal fallback;
  • exact authenticated stop identity;
  • contender success, failure, signal, delay, and loser convergence;
  • symmetric version-pair checks proving replacement is never mutual.

Flow

sequenceDiagram
    participant TUI as Fresh TUI
    participant Reg as Registration
    participant Old as Current service
    participant New as Installed service

    TUI->>Reg: Read and authenticate owner
    Reg-->>TUI: version + exact instance
    alt installed version is newer
        TUI->>Old: POST /api/service/stop(instanceID)
        Old-->>TUI: accepted
        Old-->>Old: graceful shutdown
        TUI->>New: spawn installed binary
        New-->>Reg: publish new owner
        TUI->>New: attach when ready
    else installed version is older
        TUI-->>TUI: VersionMismatchError
        Note over TUI: Explicit service restart is required
    else reconnect
        TUI->>Reg: rediscover without version gate
        TUI->>Old: attach to current owner
    end
Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant