fix(daemon): refuse to replace a reachable daemon newer than the client by okwasniewski · Pull Request #2811 · callstack/agent-device

okwasniewski · 2026-09-23T12:05:16Z

Summary

Seen while running the Bluesky e2e suite: the project depends on @e2edev/mobile, which pins agent-device 0.21.6, and an unrelated dependency hoisted agent-device 0.20.8 onto node_modules/.bin. Running that binary printed

Replacing daemon (pid 88723, v0.21.6) in /Users/okwasniewski/.agent-device: version mismatch (client v0.20.8)

and killed the 0.21.6 daemon that owned the live e2e session. The next engine command replaced it back, so each stray invocation cost two daemon restarts plus every attached session.

resolveDaemonTakeoverReason treated any version difference as grounds to replace the daemon. That is right for an upgrade (newer client meets an older daemon) and wrong for the reverse: an older binary must not kill the daemon a newer install started.

Change

daemon-launch-spec.ts: a version mismatch now distinguishes direction. A daemon older than the client is replaced as before. A reachable daemon newer than the client throws COMMAND_FAILED naming both versions, the pid, and agent-device daemon stop --state-dir <dir> for a deliberate downgrade. An unreachable newer daemon is dead and replaced as before.
daemon-client-lifecycle.ts: passes the state dir so the hint names the right daemon.
host-kit/version: isNewerVersion, numeric-aware segment ordering. Enough to tell an upgrade from a downgrade; equal strings never reach it.

Not in this PR: refusing to replace a same-version-mismatch daemon that has active sessions. That needs a sessions probe before the takeover decision and is a separate change.

Validation

Launch-spec tests: reachable newer daemon rejects with the message and hint; unreachable newer daemon and reachable older daemon both still return the version-mismatch reason.
Lifecycle test over loopback fixtures: an older client meeting a newer reachable daemon rejects, spawns nothing, only probes GET /health, prints no takeover notice, and leaves the daemon metadata in place.
Both new tests fail on main. unit-core for the touched files, tsc -p tsconfig.json, tsc -b packages/host-kit, oxlint, oxfmt, check:layering clean.

In a project depending on @e2edev/mobile (pinning agent-device 0.21.6) an unrelated dependency hoisted agent-device 0.20.8 onto node_modules/.bin. Running it printed "Replacing daemon (pid 88723, v0.21.6): version mismatch (client v0.20.8)" and killed the daemon that owned the live e2e session. The next engine command replaced it back, so every stray old-CLI invocation cost two daemon restarts and the attached sessions. A version mismatch now distinguishes direction. A daemon OLDER than the client is still replaced (the upgrade path). A reachable daemon NEWER than the client is neither reused nor replaced: the command fails with both versions and the stop command for a deliberate downgrade. An unreachable newer daemon is dead and replaced as before.

Copilot

Copilot review overview

🟡 Changes recommended

Address SemVer prerelease handling, restore existing version tests, and safely quote state-directory hints.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 2

Open (3)

Use SemVer-aware comparison for prerelease versions · New
Retain existing coverage when adding comparator tests · New
Shell-quote state directory in CLI hints · New

What changed in this PR

This PR prevents older clients from replacing reachable newer daemons.

Changes:

Adds directional daemon version comparison and refusal errors.
Passes the state directory into lifecycle diagnostics.
Adds takeover and version-comparison regression tests.

File	Description
`src/daemon-client/daemon-launch-spec.ts`	Implements newer-daemon refusal logic.
`src/daemon-client/daemon-client-lifecycle.ts`	Supplies the daemon state directory.
`src/daemon-client/__tests__/daemon-launch-spec.test.ts`	Tests version-direction decisions.
`src/daemon-client/__tests__/daemon-client-lifecycle.test.ts`	Tests end-to-end refusal behavior.
`packages/host-kit/src/version.ts`	Exports version comparison.
`packages/host-kit/src/internal/version.ts`	Implements version ordering.
`packages/host-kit/src/internal/version.test.ts`	Tests version ordering.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Review follow-ups on the newer-daemon takeover refusal: - isNewerVersion orders numeric release segments, ranks a release above any prerelease of the same base (0.21.13 > 0.21.13-dev, the shape main carries between releases) and compares prerelease fields per SemVer; tests cover the -dev, rc and build-metadata cases - restore the readVersion and project-root tests the previous commit replaced and append the comparator cases instead - shell-quote the state dir in the stop hint so it pastes back correctly - move the sendToDaemon refusal test into its own file; the lifecycle suite is past the size tripwire and may not grow

okwasniewski · 2026-09-23T12:21:34Z

Addressed the review:

SemVer ordering: isNewerVersion now parses major.minor.patch, ranks a release above any prerelease of the same base (0.21.13 > 0.21.13-dev), compares prerelease fields per SemVer (numeric vs lexical), and ignores build metadata. Tests cover -dev in both directions, rc.10 vs rc.9, beta vs alpha.1, and build metadata.
Overwritten suite: my mistake, the previous commit replaced version.test.ts. The original readVersion and project-root tests are restored and the comparator cases appended.
Hint quoting: the state dir goes through shellQuoteIfNeeded; the launch-spec test now uses /tmp/state dir and asserts the quoted form.
Size ratchet (the Coverage failure): the sendToDaemon refusal test moved to daemon-client-newer-daemon.test.ts with its own minimal fixtures; daemon-client-lifecycle.test.ts is back to its merge-base length.

The Fallow complexity gate flagged compareVersions at cyclomatic 18; release, prerelease and per-field comparison are now separate functions.

Copilot

Copilot review overview

🟡 Changes recommended

The critical transport liveness issue can still allow termination of a reachable newer daemon.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

Prevent takeover when daemon is reachable via another transport · New

Resolved since last review (3)

Use SemVer-aware comparison for prerelease versions
Shell-quote state directory in CLI hints
Retain existing coverage when adding comparator tests

Review follow-ups on the newer-daemon refusal: - resolveDaemonTakeover returns a typed decision (reuse | replace | refuseNewer) instead of a reason string that sometimes throws; the lifecycle layer, which already owns the takeover notice and knows the state dir, builds the refusal error and its shell-quoted hint. The launch-spec module no longer imports device-shell. - compareVersions is exported from host-kit/version and src/cli/update-check.ts uses it, deleting its numeric-collation copy (which ranked 0.12.0 below 0.12.0-dev and never prompted a -dev build). parseVersion is one anchored regex; a malformed version reads as 0.0.0 and the doc says so. - the loopback daemon fixture and captureStderr move to src/__tests__/test-utils/daemon-http-fixture.ts, shared by the lifecycle suite and the new refusal test instead of copied.

okwasniewski · 2026-09-23T12:49:18Z

Restructured after an internal quality review (latest commit):

resolveDaemonTakeover returns a typed decision (reuse | replace | refuseNewer) instead of a reason string that sometimes throws; the lifecycle layer builds the refusal error and hint, so the decision module no longer renders shell text or imports device-shell.
compareVersions is now the one comparator in the repo: exported from host-kit/version, and src/cli/update-check.ts uses it instead of its numeric-collation copy (which never prompted a -dev build about the same-base release; test added).
parseVersion is a single anchored regex, malformed input reads as 0.0.0 and the doc says so.
The loopback daemon fixture and captureStderr live in src/__tests__/test-utils/daemon-http-fixture.ts, shared by the lifecycle suite and the new test.
Hint reads: Use the agent-device vX CLI that started it, or stop it deliberately: agent-device daemon stop --state-dir '<dir>'.

Copilot

Copilot review overview

🟡 Changes recommended

Correct invalid-version ordering and update the user-facing trust documentation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

Prevent takeover when daemon is reachable via another transport
Malformed versions can rank newer than prerelease fallbacks · New

+function parseVersion(version: string): ParsedVersion {
+  const match = SEMVER.exec(version.trim());
+  if (!match) return { release: [0, 0, 0], prerelease: [] };


thymikee · 2026-09-23T13:41:03Z

Reviewed at 1bcdc8c. The refuseNewer gate decides refusal from existingReachable, which comes from canConnectReusableDaemon(existing, settings.transportPreference) (src/daemon-client/daemon-client-lifecycle.ts#L188). When an older client runs with --daemon-transport http (or AGENT_DEVICE_DAEMON_TRANSPORT=http) against a newer socket-only daemon (default server mode is socket, src/daemon/daemon-resolution.ts#L84), requireDaemonTransport throws, canConnectReusableDaemon reports unreachable, and resolveDaemonTakeover returns replace instead of refuse. The same happens in reverse for a socket preference against an http-only daemon. This was raised on f13e49f and is still open at head. The result is that an older hoisted binary can still kill a live newer daemon and every session on it, through a transport flag instead of the default path — the exact incident this PR is meant to close. The refusal must be decided on whether the daemon is alive on any transport it advertises, not on the requesting client's transport preference; please compute reachability for the refusal check with canConnect(existing, 'auto') (or a process-identity check via isAgentDeviceDaemonProcess), and keep transportPreference only for routing after the decision. Please add a lifecycle test for a socket-only daemon reporting version 999.0.0 with a request carrying daemonTransport: 'http': it must reject with the refusal, spawn nothing, and leave the info file in place.

Not blocking: the refusal could carry a typed details.reason (e.g. DAEMON_NEWER_THAN_CLIENT) instead of a bare COMMAND_FAILED so callers don't have to match on message text (daemon-client-lifecycle.ts#L229); CHANGELOG.md is missing Unreleased entries for both the refuse-newer behavior and the update-check -dev prompt change; the update-check change (src/cli/update-check.ts#L5) looks like a second, separate fix riding along and could be its own PR or at least called out; and readVersion's '0.0.0' fallback (packages/host-kit/src/internal/version.ts#L20) would make a broken client install refuse every daemon forever under the new gate, so an unparseable or fallback version should probably be treated as "can't decide direction" and fall back to replace. Any of these can be taken or left.

Is dropping the exported isNewerVersion one-liner and just calling compareVersions(a, b) > 0 at its single call site, with the update-check swap split out, enough to reach the same result with less surface? I didn't find another owner for this logic besides daemon-client.

I did not run any tests; the before/after behavior claims come from reading the code on both sides of the change. I did not confirm that normalizeError preserves details.hint for this AppError on the MCP surface. I also did not trace the pre-existing race in startLocalDaemon's cleanupFailedDaemonStartupMetadata (stopLiveProcesses=true) when an older client's spawned daemon exits early while a newer daemon is writing its info file; that looks pre-existing and outside this diff.

Smoke Tests was queued at review time; it runs a same-version client through readReusableLocalDaemon, which touches the changed takeover ladder, but that branch only changed its return shape, so I don't expect it to fail there — I have no result yet, though. Before this can merge, the refuseNewer decision needs to key off reachability over any transport the daemon advertises rather than the client's transport preference, with a regression test covering a socket-only newer daemon and an http-preferring client.

okwasniewski · 2026-09-23T13:58:44Z

The iOS smoke failure on run 35862934402 is the same lane flake noted on #2804 (regular depth-1 snapshot must disclose the Simulator AX bridge evidence gap). It has hit four branches today, including fix/scroll-movement-disclosure and fix/native-stack-bridge-hittability. This PR touches only the daemon client's takeover decision and the version comparator, nothing on the snapshot path. Same shape as #2491; a rerun should clear it.

The refusal keyed off reachability over the client's transport preference, so an older client with --daemon-transport http replaced a newer socket-only daemon. Reuse still requires the client's transport; refusal only needs the daemon alive. Co-Authored-By: Claude <noreply@anthropic.com>

Copilot

Copilot review overview

🔵 Needs a closer look

Resolve malformed-version ordering and add coverage for socket-only daemon reachability.

Review effort: Lite
Findings: 1

Open (1)

Malformed versions can rank newer than prerelease fallbacks

Resolved since last review (1)

Prevent takeover when daemon is reachable via another transport

thymikee · 2026-09-23T16:41:34Z

Reviewed at 2c8e0fa. The earlier question is answered: the refusal now checks whether the newer daemon is alive on any transport it advertises, and the new sendToDaemon test covers the socket-only daemon with an http client. The code looks right.

One small note, not blocking: when settings.transportPreference is already 'auto', onAnyAdvertisedTransport runs the same canConnectReusableDaemon(existing, 'auto') probe a second time, so a hung newer daemon costs two healthcheck timeouts (https://github.com/callstack/agent-device/blob/2c8e0fa/src/daemon-client/daemon-client-lifecycle.ts#L192). Skipping the second probe for 'auto' would fix it.

The two Smoke Tests failures look unrelated. Android fails on "automation-alert-result did not become visible after five scrolls", and iOS fails in testAbandonedTreeCaptureSkipsQuerySweepAndHonorsWarmupExemption. Both start a same-version daemon, where this change does not run, and neither is the depth-frontier assertion the PR body mentions.

Copilot AI lite review requested due to automatic review settings September 23, 2026 12:05

Copilot started reviewing on behalf of okwasniewski September 23, 2026 12:07 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Comment thread packages/host-kit/src/internal/version.ts Outdated

Comment thread packages/host-kit/src/internal/version.test.ts Outdated

Comment thread src/daemon-client/daemon-launch-spec.ts Outdated

Copilot AI review requested due to automatic review settings September 23, 2026 12:21

refactor(host-kit): split the SemVer comparator so each rule reads alone

f13e49f

The Fallow complexity gate flagged compareVersions at cyclomatic 18; release, prerelease and per-field comparison are now separate functions.

Copilot started reviewing on behalf of okwasniewski September 23, 2026 12:31 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Comment thread src/daemon-client/daemon-client-lifecycle.ts Outdated

Copilot AI review requested due to automatic review settings September 23, 2026 12:49

Copilot started reviewing on behalf of okwasniewski September 23, 2026 12:57 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Comment thread packages/host-kit/src/internal/version.ts

Comment on lines +59 to +61

function parseVersion(version: string): ParsedVersion {

const match = SEMVER.exec(version.trim());

if (!match) return { release: [0, 0, 0], prerelease: [] };

This was referenced Sep 23, 2026

fix(ios-runner): log the fast app guard and gesture policy only when they change #2814

Merged

ci(ios): the iOS simulator smoke lane is failing on main across several rotating signatures #2491

Open

Copilot AI review requested due to automatic review settings September 23, 2026 15:49

Copilot started reviewing on behalf of okwasniewski September 23, 2026 15:50 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 23, 2026

thymikee merged commit 802adbe into callstack:main Sep 23, 2026
11 of 13 checks passed

Navigation Menu

Uh oh!

fix(daemon): refuse to replace a reachable daemon newer than the client - #2811

fix(daemon): refuse to replace a reachable daemon newer than the client#2811
thymikee merged 5 commits into
callstack:mainfrom
okwasniewski:oskar/daemon-refuse-newer-takeover

okwasniewski commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

okwasniewski commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

okwasniewski commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

thymikee commented Sep 23, 2026

Uh oh!

okwasniewski commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

thymikee commented Sep 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

okwasniewski commented Sep 23, 2026

Summary

Change

Validation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

okwasniewski commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

okwasniewski commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

thymikee commented Sep 23, 2026

Uh oh!

okwasniewski commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

thymikee commented Sep 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants