Repository navigation
fix(host-service): recover v2 ports promptly after host-service restart - #5430
Conversation
📝 WalkthroughWalkthroughThis PR adds a port-scan warm-up phase to the terminal reaper, extracts port-scan sync into reusable helpers, schedules repeated early syncs after startup, and updates cleanup and tests around the new timing constants. ChangesPort-scan Warm-up Feature
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
Ready to review this PR? Stage has broken it down into 4 individual chapters for you: 
Chapters generated by Stage for commit 682d9cf on Jul 3, 2026 1:50am UTC. |
After a host-service restart the detached pty-daemon keeps dev servers alive, but the new host-service starts with an empty port scanner. #5325 re-registers those adopted sessions via the terminal reaper — but the reaper runs its port-scan sync only once immediately and then every 5 minutes, and the just-adopted daemon may not yet list its sessions the instant that first pass runs. So ports stayed missing until the 5-minute tick or until the user selected the workspace (which attaches a renderer and registers the terminal directly). Decouple the port-scan sync from the 5-minute orphan-reap cadence and run it a few times over the first ~90s after startup (2/5/10/20/45/90s). This re-registers restored dev-server ports promptly without disturbing the reaper's two-pass rowless-orphan clock. Also log when the sync registers unattached sessions so the recovery is observable in the field. - Extract syncPortScans() (daemon.list + row join + applyPortScanSync); the reap pass reuses its result, so no extra daemon.list call there. - Warm-up offsets all stay below REAP_INTERVAL_MS; covered by a schedule test.
648ceb3 to
21ee7ad
Compare
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
Greptile SummaryThis PR fixes a post-restart gap where v2 dev-server ports would not appear in the sidebar until a workspace was selected, by decoupling the port-scan reconcile from the 5-minute orphan-reaper cadence and running it six times over the first ~90 s after startup (2/5/10/20/45/90 s).
Confidence Score: 4/5Safe to merge with minor caveats; the core refactoring is semantically equivalent to the original and the warm-up teardown is correctly handled. The approach is sound and the refactoring is a clean extraction. Two non-blocking issues exist: the observability log in applyPortScanSync fires on every warm-up pass for sessions that are already registered, and the warm-up timers bypass the running guard, allowing a brief concurrent applyPortScanSync call if the daemon is slow to respond. Neither causes persistent incorrect state, but both could mislead a field engineer reading logs or diagnosing an edge-case timing issue. reaper.ts — the applyPortScanSync log and the concurrency interaction between warm-up timers and the initial reap pass are the two spots worth a second look before merging.
|
| Filename | Overview |
|---|---|
| packages/host-service/src/terminal/reaper/reaper.ts | Extracts syncPortScans from the reap pass, exports REAP_INTERVAL_MS and PORT_SCAN_WARMUP_DELAYS_MS, and schedules 6 warm-up syncs over 90 s after startup. The refactoring is semantically equivalent to the original; two P2 issues found: the log fires on every warm-up pass for already-registered sessions, and warm-up calls bypass the running guard and can overlap with the initial reap. |
| packages/host-service/src/terminal/reaper/reaper.test.ts | Adds schedule tests for PORT_SCAN_WARMUP_DELAYS_MS (multi-shot, strictly increasing, all < REAP_INTERVAL_MS) and imports the two new exports. Existing planPortScanSync tests are unchanged. Coverage is appropriate for constants. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant ST as startTerminalReaper
participant SP as syncPortScans
participant RO as reapOrphanedSessions
participant D as daemon.list()
participant PM as portManager
ST->>RO: "run() at t=0"
RO->>SP: syncPortScans(db)
SP->>D: daemon.list()
D-->>SP: liveSessions
SP->>PM: applyPortScanSync
SP-->>RO: liveSessions, rowById
RO->>RO: orphan reap logic
Note over ST: warm-up timers scheduled
ST->>SP: setTimeout 2s
ST->>SP: setTimeout 5s
Note over ST: ...10/20/45/90s
ST->>RO: setInterval 5min - run()
RO->>SP: syncPortScans(db)
SP->>D: daemon.list()
D-->>SP: liveSessions
SP->>PM: applyPortScanSync
SP-->>RO: liveSessions, rowById
RO->>RO: orphan reap logic
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant ST as startTerminalReaper
participant SP as syncPortScans
participant RO as reapOrphanedSessions
participant D as daemon.list()
participant PM as portManager
ST->>RO: "run() at t=0"
RO->>SP: syncPortScans(db)
SP->>D: daemon.list()
D-->>SP: liveSessions
SP->>PM: applyPortScanSync
SP-->>RO: liveSessions, rowById
RO->>RO: orphan reap logic
Note over ST: warm-up timers scheduled
ST->>SP: setTimeout 2s
ST->>SP: setTimeout 5s
Note over ST: ...10/20/45/90s
ST->>RO: setInterval 5min - run()
RO->>SP: syncPortScans(db)
SP->>D: daemon.list()
D-->>SP: liveSessions
SP->>PM: applyPortScanSync
SP-->>RO: liveSessions, rowById
RO->>RO: orphan reap logic
Comments Outside Diff (1)
-
packages/host-service/src/terminal/reaper/reaper.ts, line 112-126 (link)The log fires on every warm-up pass (and every 5-minute reap tick) for sessions that are already registered, because
planPortScanSyncadds a session toregisterwhenever it is alive and active regardless of whether it is already in the port manager. After the first successful warm-up at t=2s registers a session, the warm-ups at t=5s, 10s, 20s, 45s, and 90s each produce another "registered 1 unattached daemon session(s)" line for the same session. In a field log this looks like six separate adoption events when it is really one registration followed by five no-op upserts, making it hard to tell at a glance whether recovery actually happened or is being re-attempted.Prompt To Fix With AI
This is a comment left during a code review. Path: packages/host-service/src/terminal/reaper/reaper.ts Line: 112-126 Comment: The log fires on every warm-up pass (and every 5-minute reap tick) for sessions that are already registered, because `planPortScanSync` adds a session to `register` whenever it is alive and active regardless of whether it is already in the port manager. After the first successful warm-up at t=2s registers a session, the warm-ups at t=5s, 10s, 20s, 45s, and 90s each produce another "registered 1 unattached daemon session(s)" line for the same session. In a field log this looks like six separate adoption events when it is really one registration followed by five no-op upserts, making it hard to tell at a glance whether recovery actually happened or is being re-attempted. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/host-service/src/terminal/reaper/reaper.ts:112-126
The log fires on every warm-up pass (and every 5-minute reap tick) for sessions that are already registered, because `planPortScanSync` adds a session to `register` whenever it is alive and active regardless of whether it is already in the port manager. After the first successful warm-up at t=2s registers a session, the warm-ups at t=5s, 10s, 20s, 45s, and 90s each produce another "registered 1 unattached daemon session(s)" line for the same session. In a field log this looks like six separate adoption events when it is really one registration followed by five no-op upserts, making it hard to tell at a glance whether recovery actually happened or is being re-attempted.
```suggestion
try {
const registeredTerminalIds = portManager.getRegisteredTerminalIds();
const registeredSet = new Set(registeredTerminalIds);
const plan = planPortScanSync({
liveSessions,
rowById,
registeredTerminalIds,
isLive: isLiveTerminalSession,
});
for (const entry of plan.register) {
portManager.upsertSession(entry.terminalId, entry.workspaceId, entry.pid);
}
const newCount = plan.register.filter(
(e) => !registeredSet.has(e.terminalId),
).length;
if (newCount > 0) {
console.log(
`[host-service] port-scan sync: registered ${newCount} unattached daemon session(s) for scanning`,
);
}
```
### Issue 2 of 2
packages/host-service/src/terminal/reaper/reaper.ts:240-246
**Warm-up syncs not guarded against overlap with the initial reap pass**
The initial `run()` fires at t=0 and calls `reapOrphanedSessions` → `syncPortScans`. The `running` flag prevents concurrent *reap* calls, but the warm-up timers call `syncPortScans` directly and are unguarded. If `getDaemonClient()` or `daemon.list()` takes longer than 2 s (e.g., daemon IPC is slow right after adoption), the t=2 s warm-up starts its own `applyPortScanSync` while the initial reap's `syncPortScans` is still in flight. Both paths could then call `portManager.unregisterSession` / `upsertSession` concurrently on the same terminal ID. Because `upsertSession` is idempotent the common case is harmless, but a warm-up that sees an empty daemon list right after the initial reap saw a non-empty one would transiently unregister sessions the reap just adopted.
Reviews (1): Last reviewed commit: "fix(host-service): recover v2 ports prom..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| setTimeout(() => { | ||
| void syncPortScans(db).catch((err) => { | ||
| console.warn("[host-service] port-scan warm-up sync failed:", err); | ||
| }); | ||
| }, delay), | ||
| ); | ||
| for (const timer of warmupTimers) timer.unref(); |
There was a problem hiding this comment.
The initial run() fires at t=0 and calls reapOrphanedSessions → syncPortScans. The running flag prevents concurrent reap calls, but the warm-up timers call syncPortScans directly and are unguarded. If getDaemonClient() or daemon.list() takes longer than 2 s (e.g., daemon IPC is slow right after adoption), the t=2 s warm-up starts its own applyPortScanSync while the initial reap's syncPortScans is still in flight. Both paths could then call portManager.unregisterSession / upsertSession concurrently on the same terminal ID. Because upsertSession is idempotent the common case is harmless, but a warm-up that sees an empty daemon list right after the initial reap saw a non-empty one would transiently unregister sessions the reap just adopted.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/terminal/reaper/reaper.ts
Line: 240-246
Comment:
**Warm-up syncs not guarded against overlap with the initial reap pass**
The initial `run()` fires at t=0 and calls `reapOrphanedSessions` → `syncPortScans`. The `running` flag prevents concurrent *reap* calls, but the warm-up timers call `syncPortScans` directly and are unguarded. If `getDaemonClient()` or `daemon.list()` takes longer than 2 s (e.g., daemon IPC is slow right after adoption), the t=2 s warm-up starts its own `applyPortScanSync` while the initial reap's `syncPortScans` is still in flight. Both paths could then call `portManager.unregisterSession` / `upsertSession` concurrently on the same terminal ID. Because `upsertSession` is idempotent the common case is harmless, but a warm-up that sees an empty daemon list right after the initial reap saw a non-empty one would transiently unregister sessions the reap just adopted.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Good catch — fixed in 21ee7ad→latest. syncPortScans now coalesces concurrent callers onto a single in-flight run via a module-level promise, so the warm-up can't run applyPortScanSync while the initial reap's sync is still in flight. This matters precisely in the slow-daemon-after-adoption case this PR targets.
Address review: the warm-up timers and the initial reap pass both call syncPortScans unguarded. A slow daemon.list right after adoption (exactly when the warm-up fires) could let a second sync observe a transiently-empty list and unregister sessions the first just registered. Coalesce concurrent callers onto one in-flight run.
…ntry (#5438) * fix(desktop): start the terminal reaper in the desktop host-service entry startTerminalReaper was only wired into packages/host-service serve.ts (the standalone/CLI entry), so the packaged desktop app never ran the orphan reap (#5168) or the port-scan reconcile for daemon-adopted sessions (#5325/#5430). Ports for terminals that survived a host-service restart stayed invisible until the workspace was selected. * refactor(host-service): start the terminal reaper inside createApp Both real entry points (serve.ts and the desktop entry) consume createApp, so owning the reaper there removes the per-entry wiring that let the desktop app ship without it. Tests opt out via createTestHost: the reaper's warm-up timers reach the real pty-daemon through the process-wide daemon-client singleton and reap other tests' sessions (observed as 5 integration failures with an unconditional start). The reaper's stop function joins createApp's dispose chain. * refactor(host-service): move the reaper behind a package-owned startHostRuntime createApp goes back to building the request-serving surface only — no reaper and no test-only escape hatch. Process-lifetime background work now lives in startHostRuntime (runtime/host-runtime.ts), which both serving entries call once after binding. Tests consume createApp and are untouched. * refactor(host-service): drop the startHostRuntime pass-through The wrapper only delegated to startTerminalReaper. Entries call the reaper directly, same as installProcessSafetyNet and connectRelay — one invocation per serving entry, logic stays in the package. * chore(desktop): tighten the reaper comment * chore(desktop): drop call-site comment on startTerminalReaper * chore(desktop): one-line comment on startTerminalReaper
Problem
After a host-service restart (installed app, v1.12.6+), v2 dev-server ports don't show in the sidebar until you select the workspace. In production the detached pty-daemon keeps dev servers alive across the restart, so the ports genuinely exist — they're just not scanned.
Root cause
#5325 added a reaper-driven port-scan reconcile to register daemon-adopted sessions without a renderer attach. But that reconcile runs only once at startup and then every 5 minutes (it's embedded in the orphan reaper's
REAP_INTERVAL_MSloop). Right after a restart the just-adopted daemon may not yet list its sessions the instant that single startup pass runs — so nothing gets registered, and the next attempt is 5 minutes away. Selecting the workspace masks it by attaching a renderer, which registers the terminal directly.Ruled out: the client visibility gate (all its collections are persisted and populate on cold start) and
killStaleDaemonForDev(dev-only; production adopts the daemon).Fix
Decouple the port-scan sync from the 5-minute orphan-reap cadence and run it a few times over the first ~90s after startup (
2/5/10/20/45/90s), so restored ports register promptly without waiting for the reap tick or an attach.syncPortScans()(daemon.list + row join +applyPortScanSync); the reap pass reuses its result, so no extradaemon.list()there.registered N unattached daemon session(s)so post-restart recovery is observable in the field.Validation
planPortScanSynclogic unchanged (existing tests pass); added a schedule test asserting the warm-up is multi-shot, strictly increasing, and fully precedes the first reap tick.bun run lintclean,@superset/host-servicetypecheck passes.Caveat: I could not end-to-end reproduce the production timing locally (dev's
killStaleDaemonForDevdestroys the daemon on restart, and an adoption-mode repro was too flaky to finish). This fix targets the most likely cause — the reconcile cadence — and the new log lets us confirm in a real build: if it logs registrations after restart and ports appear, done; if it logs nothing, the daemon isn't listing adopted sessions and we dig into adoption instead.Summary by cubic
Recover v2 dev-server ports promptly after a
@superset/host-servicerestart. Ports now appear within ~90s without opening the workspace.Bug Fixes
syncPortScans(); the reaper reuses its result to avoid an extradaemon.list(); logs registered unattached sessions and warns on warm-up sync failures.Refactors
Written for commit 682d9cf. Summary will update on new commits.
Summary by CodeRabbit