Skip to content

feat(desktop): respawn host-service on every Electron auto-update#4218

Merged
Kitenite merged 8 commits into
mainfrom
sync-service-electron-ver
May 8, 2026
Merged

feat(desktop): respawn host-service on every Electron auto-update#4218
Kitenite merged 8 commits into
mainfrom
sync-service-electron-ver

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented May 7, 2026

Plan: apps/desktop/plans/done/20260507-host-service-bundled-version-pin.md

Summary

  • Auto-updated desktops now always end up running a freshly spawned host-service. The coordinator pins host-service adoption to the desktop's app.getVersion(): if a running host-service was spawned by a different desktop version, it's killed (SIGTERM) and respawned from the bundled binary on the next Electron launch.
  • Pty-daemon — the durable session-state holder — has its own manifest + supervisor and is unaffected, so terminal sessions survive the host-service swap.
  • MIN_HOST_SERVICE_VERSION is retained for the renderer-side remote-host gate (useRemoteHostStatus) where we can't kill the remote process and a floor is still the right shape.

Why / Context

Previously, tryAdopt accepted any local host-service satisfying >= MIN_HOST_SERVICE_VERSION — a manually-bumped floor. After auto-update, a stale host-service that cleared the floor stuck around indefinitely; non-breaking host-service changes never reached users until someone remembered to bump the floor.

Why "kill on Electron update" is OK here

Host-service has two deployment modes, and they want different things:

  • Bundled with Electron (this PR's path). Host-service ships in the desktop binary. Co-deployed → freshness wins → killing on next-launch adoption is fine.
  • Standalone CLI on a remote machine (consumed by the desktop via the renderer remote-host flow). Has its own update cadence independent of any one Electron build → lifecycle preservation matters there. Unchanged by this PR: the desktop never reads or writes that machine's manifest; the renderer-side useRemoteHostStatus floor gate is the only contract.

The detached/manifest/fd-handoff machinery exists primarily for the standalone case. Repurposing the bundled-local manifest for "always fresh" is deliberate now that the standalone case is structurally separated.

User-visible cost is small: pty-daemon (packages/host-service/src/daemon/DaemonSupervisor.ts) has its own manifest+adoption lifecycle, so killing host-service drops only tunnel-client WebSockets (auto-reconnect) and the in-memory terminal sessions Map (rebuilt on re-attach via daemon-client). Real PTY state stays in the daemon.

We still do not kill host-service in the auto-updater path itself (no stopBackendAndWaitForExit-style step before quitAndInstall). The kill happens on the next Electron launch via tryAdopt — single code path that handles auto-update, crash recovery, and manual restart uniformly.

How It Works

  • HostServiceManifest gains spawnedByAppVersion: string. The host-service child writes it from a new SUPERSET_APP_VERSION env var. The coordinator sets that env from app.getVersion() when spawning.
  • tryAdopt:
    if (manifest.spawnedByAppVersion !== app.getVersion()) {
        // SIGTERM the old PID, drop the manifest, fall through to spawn()
    }
  • Pre-existing manifests (without the field) are coerced to "" by readManifest so the old PID is still found and killed — no orphan. Result: every existing user respawns host-service exactly once on first launch after this ships.

Why app-version, not host-service version

A host-service version pin was implemented and then removed. In our build flow host-service ships with the desktop, so the app-version pin strictly subsumes it for the auto-update use case — plus it catches the case where someone forgets to bump the host-service version on a host-service code change. The removed pin only added value for a hand-rolled binary in dev.

Side cleanup

  • packages/host-service/package.json#version was previously stuck at 0.1.0 while host.info hard-coded "0.8.0". Reconciled: host.info now reads pkg.version via with { type: "json" } (mirrors EXPECTED_DAEMON_VERSION), and the package.json was bumped 0.1.00.8.1. Added "./package.json": "./package.json" to the exports map. The renderer-side remote-host gate still uses MIN_HOST_SERVICE_VERSION = "0.8.0" against this reported value.
  • semver import dropped from the coordinator (no longer needed there).

Manual QA Checklist

App-version pin (the main behavior)

  • Fresh launch with no manifest → host-service spawns, manifest written with spawnedByAppVersion: app.getVersion().
  • Same Electron version restarts → adoption succeeds, no respawn (no "killing" log).
  • Simulate auto-update: change apps/desktop/package.json version (or run a different packaged build), restart → coordinator logs Adopted service spawned by app X.X.X != current Y.Y.Y, killing, SIGTERMs the old PID, spawns fresh.
  • Pre-existing manifest (no spawnedByAppVersion) → logs no recorded app version (pre-upgrade manifest), killing once, then settles.

Pty-daemon survives the swap

  • Open a v2 workspace terminal, type some text.
  • Trigger a host-service respawn (via the version-mismatch path above).
  • After Electron relaunches, the terminal session re-attaches to the same daemon-side PTY and shows the prior buffer (no fresh shell).

Tunnel reconnect

  • Brief tunnel reconnect on host-service respawn is acceptable (no permanent disconnect, no error toast loop).
  • Trigger a respawn while a workspace is mid-render — renderer recovers cleanly without manual reload.

Remote-host gate untouched

  • useRemoteHostStatus still uses MIN_HOST_SERVICE_VERSION; connecting to a remote host below the floor still surfaces the existing "host CLI too old" UI.
  • Connecting to a remote on 0.8.0 works (floor = 0.8.0, satisfies). Connecting to a remote on 0.8.1 works (satisfies floor).

Testing

  • bun run typecheck — pass.
  • bun run lint — pass.
  • bun turbo run build --filter=@superset/desktop --filter=@superset/host-service — pass. Verified packaged ASAR bundles host-service@0.8.1 and that SUPERSET_APP_VERSION = electron.app.getVersion(...) is wired through to manifest.spawnedByAppVersion.

Risks / Rollout

  • One-time forced respawn for every existing user on first launch after this ships (their manifest lacks spawnedByAppVersion). In-flight tRPC requests during the kill window fail; renderer reconnect handles it. Pty-daemon is unaffected — terminal sessions survive.
  • Hard SIGTERM, no drain. Same as before this PR. Respawn cadence is now "every desktop update" instead of "every host-service version bump." If users notice tunnel blips after auto-updates, drain-before-SIGTERM is the lever to revisit (deferred in the plan).
  • Rollback: revert this PR. The previous >= MIN_HOST_SERVICE_VERSION floor returns; running host-services are again adopted as long as they clear it.

Follow-ups

  • Drain-before-SIGTERM if respawn cadence becomes user-visible.

Replace MIN_HOST_SERVICE_VERSION floor check in tryAdopt with an equality
check against a build-time-injected BUNDLED_HOST_SERVICE_VERSION, so
auto-updated desktops always end up running the host-service that shipped
with them. Pty-daemon (the durable session state) is untouched.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 7, 2026

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR pins desktop host-service adoption to the exact version bundled with the Electron build by importing the host-service package.json version into the coordinator, replaces semver floor logic with strict equality, exposes package.json via package exports, updates the host router to return the package.json version, and adds a rollout plan doc.

Changes

Version Pinning Strategy Implementation

Layer / File(s) Summary
Strategy Plan and Scope
apps/desktop/plans/done/20260507-host-service-bundled-version-pin.md
Planning document specifies version pinning approach: coordinator uses a build-time BUNDLED_HOST_SERVICE_VERSION for exact equality checks and documents scope/out-of-scope items.
Coordinator Adoption Implementation
apps/desktop/src/main/lib/host-service-coordinator.ts
Coordinator imports @superset/host-service/package.json version as BUNDLED_HOST_SERVICE_VERSION, removes semver-based floor check, and enforces strict equality (version === BUNDLED_HOST_SERVICE_VERSION) to decide adopt vs kill+respawn.
Host-Service Package Manifest
packages/host-service/package.json
Package version bumped 0.1.00.8.0 and exports extended with ./package.json to allow direct JSON import of the version.
Host Router Version Source
packages/host-service/src/trpc/router/host/host.ts
Replaces hardcoded version literal with import of @superset/host-service/package.json version and returns it unchanged from hostRouter.info.
Shared Host-Version Documentation
packages/shared/src/host-version.ts
Clarifies comments about remote vs bundled host-service and updates the 0.8.0 history note concerning terminal/session changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I pin the version tight and snug,
No semver drift or stretchy rug.
If remote and bundled fail to match,
A hop, a respawn, a quick dispatch.
Bunny approves — exactness is snug.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'feat(desktop): respawn host-service on every Electron auto-update' is partially related to the changeset but does not capture the main change. The core change is pinning host-service adoption to the bundled version (a strict equality check), not respawning on every auto-update. The actual respawn only occurs on version mismatch. Consider a more precise title like 'feat(desktop): pin host-service adoption to bundled version' that clearly indicates the core mechanism (version pinning) rather than just the observable behavior.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description comprehensively covers the change's purpose, implementation, context, and testing requirements with detailed sections on why the change is safe and how it works.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync-service-electron-ver

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/desktop/plans/20260507-host-service-bundled-version-pin.md`:
- Around line 45-46: In the bullet under "**Strict equality vs `>=bundled`?**"
replace the phrase "hand-rolled newer daemon" with "newer host-service" so the
text reads that allowing `>=bundled` lets a hand-rolled newer host-service stick
around indefinitely; update any nearby mentions of "daemon" in that bullet to
"host-service" to avoid pty-daemon confusion (the lines containing the question
header and the offending phrase help locate the change).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1427c83c-62e4-4a3b-8a62-a4df46d36eec

📥 Commits

Reviewing files that changed from the base of the PR and between f4ae66e and dc00534.

📒 Files selected for processing (1)
  • apps/desktop/plans/20260507-host-service-bundled-version-pin.md

Comment on lines +45 to +46
- **Strict equality vs `>=bundled`?** Strict equality means a dev build pointing at a locally newer host-service would get killed on Electron start. `>=bundled` avoids that but lets a hand-rolled newer daemon stick around indefinitely. Default to strict equality unless dev-flow friction shows up.
- **Drain before SIGTERM?** Today line 304 is a hard kill. If respawn cadence becomes user-visible (tunnel reconnect storms), add a short drain — stop accepting new connections, wait N seconds — before SIGTERM. Not needed in v1.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify “newer daemon” wording to avoid pty-daemon confusion.

At Line 45, “hand-rolled newer daemon” reads as pty-daemon, but this section is about host-service adoption. Please rename to “newer host-service” to prevent implementation ambiguity.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/plans/20260507-host-service-bundled-version-pin.md` around lines
45 - 46, In the bullet under "**Strict equality vs `>=bundled`?**" replace the
phrase "hand-rolled newer daemon" with "newer host-service" so the text reads
that allowing `>=bundled` lets a hand-rolled newer host-service stick around
indefinitely; update any nearby mentions of "daemon" in that bullet to
"host-service" to avoid pty-daemon confusion (the lines containing the question
header and the offending phrase help locate the change).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Section was removed when we committed to strict equality (the open-questions only retains the drain-before-SIGTERM bullet now). No-op.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kitenite, understood — since the team committed to strict equality and the "Strict equality vs >=bundled?" bullet was dropped entirely, my concern is already moot. Thanks for the clarification!

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 7, 2026

Greptile Summary

This docs-only PR adds an ExecPlan describing how to replace the >= MIN_HOST_SERVICE_VERSION floor check in host-service-coordinator.tryAdopt with a strict equality check against a build-time-injected BUNDLED_HOST_SERVICE_VERSION, so that auto-updated desktops always spawn the host-service binary that shipped with them.

  • The plan accurately references the current code at host-service-coordinator.ts:289-308 (verified against source), correctly preserves MIN_HOST_SERVICE_VERSION for the renderer-side remote-host gate in useRemoteHostStatus.ts:91, and mirrors the EXPECTED_DAEMON_VERSION injection pattern from packages/host-service/src/daemon/expected-version.ts.
  • The open questions section (strict equality vs >=bundled, drain-before-SIGTERM) are appropriate forward-looking notes; no behavior changes are included in this PR.

Confidence Score: 5/5

Docs-only plan file with no runtime changes; safe to merge.

The plan document accurately matches the current codebase — line numbers, logic, and existing patterns are all verified against live source files. No code is modified.

No files require special attention.

Important Files Changed

Filename Overview
apps/desktop/plans/20260507-host-service-bundled-version-pin.md New plan document for pinning host-service adoption to the bundled version; references accurate line numbers and existing patterns in the codebase.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Electron launch] --> B[tryAdopt reads manifest]
    B --> C[fetchHostVersion]
    C -->|version unknown| D[SIGTERM old pid + remove manifest]
    C -->|version returned| E{Current: version >= MIN_HOST_SERVICE_VERSION\nPlanned: version === BUNDLED_HOST_SERVICE_VERSION}
    E -->|check fails| D
    D --> F[Spawn fresh bundled host-service]
    E -->|check passes| G[Adopt running host-service]
    G --> H[startAdoptedLivenessCheck]

    subgraph "Unaffected by this change"
        I[Renderer remote-host gate\nuseRemoteHostStatus.ts:91\nstill uses MIN_HOST_SERVICE_VERSION]
        J[DaemonSupervisor\npty-daemon lifecycle\nunchanged]
    end
Loading

Reviews (1): Last reviewed commit: "docs(desktop): plan to pin host-service ..." | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Kitenite added 2 commits May 7, 2026 16:46
…plan

Drop the >=bundled alternative — dev builds pointing at a hand-rolled
newer host-service should also get killed and replaced, so only one
host-service version is ever live alongside a given desktop.
Replace the `>= MIN_HOST_SERVICE_VERSION` floor in
`host-service-coordinator.tryAdopt` with strict equality against the
host-service version bundled with this Electron build. After auto-update,
any running host-service whose version differs is killed and respawned
from the bundled binary on the next launch — no more stale daemons
clinging to old code as long as they clear a manually-bumped floor.

- Promote host-service version to a single source of truth at
  `packages/shared/src/host-version.ts` (`HOST_SERVICE_VERSION`).
- `host.info` reads from there, replacing the previously hardcoded
  constant in `packages/host-service/src/trpc/router/host/host.ts`.
- Desktop coordinator imports it as `BUNDLED_HOST_SERVICE_VERSION` and
  pins to strict equality. `semver` import dropped from the coordinator
  since it's no longer needed there.
- `MIN_HOST_SERVICE_VERSION` retained for the renderer-side remote-host
  gate (`useRemoteHostStatus`) where a floor is still the right shape.

Pty-daemon — which holds durable session state — has its own manifest +
supervisor and is unaffected by host-service swaps, so terminal sessions
survive the respawn.

Plan: apps/desktop/plans/done/20260507-host-service-bundled-version-pin.md
@Kitenite Kitenite changed the title docs(desktop): plan to pin host-service adoption to bundled version feat(desktop): pin host-service adoption to bundled version May 8, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/desktop/plans/done/20260507-host-service-bundled-version-pin.md`:
- Around line 23-24: Update step 1 to reflect the actual implementation: state
that the desktop bundle uses a compile-time import of the shared constant
(HOST_SERVICE_VERSION from packages/shared/src/host-version.ts) rather than
build-time injection from packages/host-service/package.json; mention this
mirrors the EXPECTED_DAEMON_VERSION pattern only conceptually, and note that
BUNDLED_HOST_SERVICE_VERSION in the plan should be represented by the shared
HOST_SERVICE_VERSION and therefore must be bumped in packages/shared whenever
host-service is updated; also adjust the reference to
host-service-coordinator.ts (the code block around the HOST_SERVICE_VERSION
usage) so the plan and retrospective are consistent.

In `@apps/desktop/src/main/lib/host-service-coordinator.ts`:
- Line 8: Replace the brittle import of BUNDLED_HOST_SERVICE_VERSION from the
shared constant with a build-time JSON import of the actual host-service
package.json version (the EXPECTED_DAEMON_VERSION pattern): import the
package.json using TypeScript's with { type: "json" } syntax, read its "version"
as EXPECTED_DAEMON_VERSION (or similar), and then use that value in the existing
version equality check instead of BUNDLED_HOST_SERVICE_VERSION so the
coordinator compares the running host-service against the real package.json
version at build time.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 55e90b06-3da5-45ff-84eb-f93d99499932

📥 Commits

Reviewing files that changed from the base of the PR and between f83da66 and 416d2b8.

📒 Files selected for processing (4)
  • apps/desktop/plans/done/20260507-host-service-bundled-version-pin.md
  • apps/desktop/src/main/lib/host-service-coordinator.ts
  • packages/host-service/src/trpc/router/host/host.ts
  • packages/shared/src/host-version.ts
✅ Files skipped from review due to trivial changes (2)
  • packages/host-service/src/trpc/router/host/host.ts
  • packages/shared/src/host-version.ts

Comment thread apps/desktop/plans/done/20260507-host-service-bundled-version-pin.md Outdated
Comment thread apps/desktop/src/main/lib/host-service-coordinator.ts Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch

Thank you for your contribution! 🎉

The plan said BUNDLED_HOST_SERVICE_VERSION would be injected at build
time from packages/host-service/package.json (mirroring EXPECTED_DAEMON_
VERSION). What actually shipped is a regular compile-time import of
HOST_SERVICE_VERSION from packages/shared/src/host-version.ts. As a done/
plan kept as a permanent record, the contradiction was misleading.

Also note why the auto-derive-from-package.json approach was deferred:
host-service's package.json has historically lagged the runtime-reported
version (host.ts hardcoded "0.8.0" while package.json was at "0.1.0"),
so reconciling that is its own change.
Switch HOST_SERVICE_VERSION / BUNDLED_HOST_SERVICE_VERSION from a manual
constant in packages/shared/src/host-version.ts to a build-time JSON
import of packages/host-service/package.json — mirroring the
EXPECTED_DAEMON_VERSION pattern at
packages/host-service/src/daemon/expected-version.ts.

- Bump packages/host-service/package.json version 0.1.0 → 0.8.0 to
  reconcile pre-existing drift: host.ts had been hard-coding "0.8.0"
  for several breaking-change ratchets while package.json was never
  bumped past 0.1.0. With the package.json now load-bearing, every
  bump kills running host-services on the next launch.
- Add `"./package.json": "./package.json"` to host-service's exports
  so the desktop coordinator can import it (same pattern as pty-daemon).
- Drop HOST_SERVICE_VERSION export from packages/shared; keep
  MIN_HOST_SERVICE_VERSION (still load-bearing for the remote-host
  renderer gate in useRemoteHostStatus).

Addresses CodeRabbit review on PR #4218.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
apps/desktop/plans/done/20260507-host-service-bundled-version-pin.md (1)

48-48: ⚡ Quick win

Avoid hard-coded source line references in a permanent done/ plan.

“Today line 304” will drift quickly and make this retrospective stale; referencing the coordinator behavior/function is more durable.

✏️ Suggested wording fix
-- **Drain before SIGTERM?** Today line 304 is a hard kill. If respawn cadence becomes user-visible (tunnel reconnect storms), add a short drain — stop accepting new connections, wait N seconds — before SIGTERM. Not needed in v1.
+- **Drain before SIGTERM?** Today the coordinator uses a hard kill in the mismatch path. If respawn cadence becomes user-visible (tunnel reconnect storms), add a short drain — stop accepting new connections, wait N seconds — before SIGTERM. Not needed in v1.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/plans/done/20260507-host-service-bundled-version-pin.md` at line
48, Replace the hard-coded source line reference "Today line 304" with a durable
reference to the coordinator behavior or function that performs the hard kill
(e.g., mention the coordinator's respawn/termination handler or function name
that sends SIGTERM), so the note reads something like "Today the coordinator
issues a hard kill in its termination handler" or "the coordinator's respawn
logic currently performs a hard SIGTERM"; update the sentence to reference that
behavior instead of a line number to keep the plan accurate over time.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/desktop/plans/done/20260507-host-service-bundled-version-pin.md`:
- Line 43: Update the sentence that currently reads “If you need to test against
a newer daemon in dev, point the desktop at a build that bundles it; don't
hand-roll the running process.” to use the term host-service instead of daemon;
i.e., change “newer daemon” to “newer host-service” so the line reads “If you
need to test against a newer host-service in dev, point the desktop at a build
that bundles it; don't hand-roll the running process.” This keeps terminology
consistent with the rest of the doc where pty-daemon is discussed separately.

---

Nitpick comments:
In `@apps/desktop/plans/done/20260507-host-service-bundled-version-pin.md`:
- Line 48: Replace the hard-coded source line reference "Today line 304" with a
durable reference to the coordinator behavior or function that performs the hard
kill (e.g., mention the coordinator's respawn/termination handler or function
name that sends SIGTERM), so the note reads something like "Today the
coordinator issues a hard kill in its termination handler" or "the coordinator's
respawn logic currently performs a hard SIGTERM"; update the sentence to
reference that behavior instead of a line number to keep the plan accurate over
time.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0b384f46-ea59-4170-931b-4c62ebca01f1

📥 Commits

Reviewing files that changed from the base of the PR and between 6e2fea8 and beaa55b.

📒 Files selected for processing (5)
  • apps/desktop/plans/done/20260507-host-service-bundled-version-pin.md
  • apps/desktop/src/main/lib/host-service-coordinator.ts
  • packages/host-service/package.json
  • packages/host-service/src/trpc/router/host/host.ts
  • packages/shared/src/host-version.ts
✅ Files skipped from review due to trivial changes (3)
  • packages/host-service/package.json
  • packages/shared/src/host-version.ts
  • packages/host-service/src/trpc/router/host/host.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/src/main/lib/host-service-coordinator.ts


- **Auto-update ships new host-service version** → next Electron launch: adoption check fails, manifest PID gets SIGTERM, fresh host-service spawns from the bundled binary. One brief tunnel reconnect.
- **Auto-update ships same host-service version** → adoption succeeds, no respawn.
- **Dev: locally newer host-service** → killed and replaced with the bundled version. If you need to test against a newer daemon in dev, point the desktop at a build that bundles it; don't hand-roll the running process.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use host-service wording instead of daemon here.

“newer daemon” is ambiguous in this doc because pty-daemon is discussed separately; this sentence is about host-service.

✏️ Suggested wording fix
-- **Dev: locally newer host-service** → killed and replaced with the bundled version. If you need to test against a newer daemon in dev, point the desktop at a build that bundles it; don't hand-roll the running process.
+- **Dev: locally newer host-service** → killed and replaced with the bundled version. If you need to test against a newer host-service in dev, point the desktop at a build that bundles it; don't hand-roll the running process.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- **Dev: locally newer host-service** → killed and replaced with the bundled version. If you need to test against a newer daemon in dev, point the desktop at a build that bundles it; don't hand-roll the running process.
- **Dev: locally newer host-service** → killed and replaced with the bundled version. If you need to test against a newer host-service in dev, point the desktop at a build that bundles it; don't hand-roll the running process.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/plans/done/20260507-host-service-bundled-version-pin.md` at line
43, Update the sentence that currently reads “If you need to test against a
newer daemon in dev, point the desktop at a build that bundles it; don't
hand-roll the running process.” to use the term host-service instead of daemon;
i.e., change “newer daemon” to “newer host-service” so the line reads “If you
need to test against a newer host-service in dev, point the desktop at a build
that bundles it; don't hand-roll the running process.” This keeps terminology
consistent with the rest of the doc where pty-daemon is discussed separately.

No code changes — bumping the package.json version so existing installs
running host-service@0.8.0 get killed and respawned exactly once on
their next desktop launch, exercising the new strict-equality adoption
path end-to-end. Pty-daemon survives the swap, so terminal sessions are
preserved.
Add an app-version pin to the host-service manifest as belt + suspenders
alongside the existing host-service version pin. Either mismatch triggers
kill + respawn on the next Electron startup, so an auto-updated desktop
always lands on a freshly spawned host-service — even if a host-service
code change shipped without bumping its package.json version.

- HostServiceManifest gains `spawnedByAppVersion`; child writes it from a
  new SUPERSET_APP_VERSION env var that the coordinator sets from
  `app.getVersion()` when spawning.
- tryAdopt in the coordinator checks `manifest.spawnedByAppVersion ===
  app.getVersion()` before the existing host-service version check.
- Pre-existing manifests without the field are coerced to empty string
  by readManifest so we still find the old PID and kill it on first
  launch after upgrade — instead of orphaning it.

Pty-daemon still has its own manifest + supervisor and is unaffected.
The app-version pin (manifest.spawnedByAppVersion vs app.getVersion())
strictly subsumes the host-service version pin for the auto-update
use case: host-service ships with the desktop, so a host-service
version change always entails an app-version change. The host-service
version pin only added value in dev scenarios with hand-rolled
binaries — not load-bearing for the actual goal.

Removes:
- BUNDLED_HOST_SERVICE_VERSION constant + package.json import in the
  coordinator.
- The version-fetch round-trip (fetchHostVersion + host.info probe) on
  every adoption attempt.

Keeps:
- App-version pin in tryAdopt.
- host.info still returns version from package.json (used by the
  renderer-side remote-host gate via MIN_HOST_SERVICE_VERSION).
- packages/host-service/package.json version bump and exports update.
@Kitenite Kitenite changed the title feat(desktop): pin host-service adoption to bundled version feat(desktop): respawn host-service on every Electron auto-update May 8, 2026
@Kitenite Kitenite merged commit 8cad8cf into main May 8, 2026
17 checks passed
MocA-Love pushed a commit to MocA-Love/superset that referenced this pull request May 21, 2026
…perset-sh#4218)

* docs(desktop): plan to pin host-service adoption to bundled version

Replace MIN_HOST_SERVICE_VERSION floor check in tryAdopt with an equality
check against a build-time-injected BUNDLED_HOST_SERVICE_VERSION, so
auto-updated desktops always end up running the host-service that shipped
with them. Pty-daemon (the durable session state) is untouched.

* docs(desktop): commit to strict-equality version pin in host-service plan

Drop the >=bundled alternative — dev builds pointing at a hand-rolled
newer host-service should also get killed and replaced, so only one
host-service version is ever live alongside a given desktop.

* feat(desktop): pin host-service adoption to bundled version

Replace the `>= MIN_HOST_SERVICE_VERSION` floor in
`host-service-coordinator.tryAdopt` with strict equality against the
host-service version bundled with this Electron build. After auto-update,
any running host-service whose version differs is killed and respawned
from the bundled binary on the next launch — no more stale daemons
clinging to old code as long as they clear a manually-bumped floor.

- Promote host-service version to a single source of truth at
  `packages/shared/src/host-version.ts` (`HOST_SERVICE_VERSION`).
- `host.info` reads from there, replacing the previously hardcoded
  constant in `packages/host-service/src/trpc/router/host/host.ts`.
- Desktop coordinator imports it as `BUNDLED_HOST_SERVICE_VERSION` and
  pins to strict equality. `semver` import dropped from the coordinator
  since it's no longer needed there.
- `MIN_HOST_SERVICE_VERSION` retained for the renderer-side remote-host
  gate (`useRemoteHostStatus`) where a floor is still the right shape.

Pty-daemon — which holds durable session state — has its own manifest +
supervisor and is unaffected by host-service swaps, so terminal sessions
survive the respawn.

Plan: apps/desktop/plans/done/20260507-host-service-bundled-version-pin.md

* docs(desktop): align done/ plan step 1 with what shipped

The plan said BUNDLED_HOST_SERVICE_VERSION would be injected at build
time from packages/host-service/package.json (mirroring EXPECTED_DAEMON_
VERSION). What actually shipped is a regular compile-time import of
HOST_SERVICE_VERSION from packages/shared/src/host-version.ts. As a done/
plan kept as a permanent record, the contradiction was misleading.

Also note why the auto-derive-from-package.json approach was deferred:
host-service's package.json has historically lagged the runtime-reported
version (host.ts hardcoded "0.8.0" while package.json was at "0.1.0"),
so reconciling that is its own change.

* refactor(desktop): derive host-service version from package.json

Switch HOST_SERVICE_VERSION / BUNDLED_HOST_SERVICE_VERSION from a manual
constant in packages/shared/src/host-version.ts to a build-time JSON
import of packages/host-service/package.json — mirroring the
EXPECTED_DAEMON_VERSION pattern at
packages/host-service/src/daemon/expected-version.ts.

- Bump packages/host-service/package.json version 0.1.0 → 0.8.0 to
  reconcile pre-existing drift: host.ts had been hard-coding "0.8.0"
  for several breaking-change ratchets while package.json was never
  bumped past 0.1.0. With the package.json now load-bearing, every
  bump kills running host-services on the next launch.
- Add `"./package.json": "./package.json"` to host-service's exports
  so the desktop coordinator can import it (same pattern as pty-daemon).
- Drop HOST_SERVICE_VERSION export from packages/shared; keep
  MIN_HOST_SERVICE_VERSION (still load-bearing for the remote-host
  renderer gate in useRemoteHostStatus).

Addresses CodeRabbit review on PR superset-sh#4218.

* chore(host-service): bump to 0.8.1 to force respawn on next release

No code changes — bumping the package.json version so existing installs
running host-service@0.8.0 get killed and respawned exactly once on
their next desktop launch, exercising the new strict-equality adoption
path end-to-end. Pty-daemon survives the swap, so terminal sessions are
preserved.

* feat(desktop): respawn host-service on every Electron auto-update

Add an app-version pin to the host-service manifest as belt + suspenders
alongside the existing host-service version pin. Either mismatch triggers
kill + respawn on the next Electron startup, so an auto-updated desktop
always lands on a freshly spawned host-service — even if a host-service
code change shipped without bumping its package.json version.

- HostServiceManifest gains `spawnedByAppVersion`; child writes it from a
  new SUPERSET_APP_VERSION env var that the coordinator sets from
  `app.getVersion()` when spawning.
- tryAdopt in the coordinator checks `manifest.spawnedByAppVersion ===
  app.getVersion()` before the existing host-service version check.
- Pre-existing manifests without the field are coerced to empty string
  by readManifest so we still find the old PID and kill it on first
  launch after upgrade — instead of orphaning it.

Pty-daemon still has its own manifest + supervisor and is unaffected.

* refactor(desktop): drop redundant host-service version pin

The app-version pin (manifest.spawnedByAppVersion vs app.getVersion())
strictly subsumes the host-service version pin for the auto-update
use case: host-service ships with the desktop, so a host-service
version change always entails an app-version change. The host-service
version pin only added value in dev scenarios with hand-rolled
binaries — not load-bearing for the actual goal.

Removes:
- BUNDLED_HOST_SERVICE_VERSION constant + package.json import in the
  coordinator.
- The version-fetch round-trip (fetchHostVersion + host.info probe) on
  every adoption attempt.

Keeps:
- App-version pin in tryAdopt.
- host.info still returns version from package.json (used by the
  renderer-side remote-host gate via MIN_HOST_SERVICE_VERSION).
- packages/host-service/package.json version bump and exports update.
MocA-Love added a commit to MocA-Love/superset that referenced this pull request May 21, 2026
…low-up

superset-sh#4074 exhaustive worktree search in v2 branch picker:
- adopt.ts / search-branches.ts / branch-search.ts: whole-file upstream
  overwrite. Fork's <repoPath>/.worktrees/<branch>-scoped variant
  (FORK NOTE in branch-search.ts) is dropped in favour of upstream's
  exhaustive 'all git worktrees including foreign ones' strategy. This
  matches superset-sh#4226's adopt-foreign-worktree UX direction.

superset-sh#4160 listBranches single git spawn perf (skipped, HEAD-preferred):
- packages/host-service/src/trpc/router/git/git.ts: keep fork's rich
  listBranches (sortOrder/pinDefault/buildBranch per-branch
  upstream/ahead-behind/last-commit). Upstream's 4xN -> 1 spawn
  simplification drops those fields the fork's BaseBranchSelector
  consumes; perf win not worth the rich UI regression. Revisit if
  BaseBranchSelector is unified with upstream's BaseBranchSelector
  in a future cycle.

superset-sh#4218 respawn host-service on Electron auto-update:
- packages/host-service/package.json: merge fork's ./settings export
  and upstream's ./package.json export (both needed).
- packages/host-service/src/trpc/router/host/host.ts: whole-file upstream
  overwrite. HOST_SERVICE_VERSION switches from fork's hard-coded
  '0.7.0' to dynamic hostServicePackageJson.version (currently 0.8.1).
  Bumps via apps/desktop/electron-builder.ts respawn flow are now
  automatic.

superset-sh#4231 restore terminal modes on reattach via headless xterm:
- packages/host-service/src/terminal/terminal.ts: createModeTracker
  call sites referenced un-declared 'cols, rows' inside session-create
  closure; fix to hard-code (120, 32) which matches the daemon.open()
  call above. Real cols/rows arrive via subsequent resize() calls;
  modeTracker.resize handles the update path.

Files clean cherry-picked: superset-sh#4046 host-service bump 0.7.0, plus
adopt-foreign-worktree wiring.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant