From 0566b9c6fec967e63d8608f2fccbe4f3e24d99d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Kwas=CC=81niewski?= Date: Wed, 23 Sep 2026 14:05:12 +0200 Subject: [PATCH 1/5] fix(daemon): refuse to replace a reachable daemon newer than the client 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. --- .../host-kit/src/internal/version.test.ts | 137 ++---------------- packages/host-kit/src/internal/version.ts | 9 ++ packages/host-kit/src/version.ts | 2 +- .../__tests__/daemon-client-lifecycle.test.ts | 49 +++++++ .../__tests__/daemon-launch-spec.test.ts | 37 +++++ src/daemon-client/daemon-client-lifecycle.ts | 6 +- src/daemon-client/daemon-launch-spec.ts | 42 +++++- 7 files changed, 150 insertions(+), 132 deletions(-) diff --git a/packages/host-kit/src/internal/version.test.ts b/packages/host-kit/src/internal/version.test.ts index 6e7e164f59..85b2e3271f 100644 --- a/packages/host-kit/src/internal/version.test.ts +++ b/packages/host-kit/src/internal/version.test.ts @@ -1,130 +1,11 @@ import assert from 'node:assert/strict'; -import fs from 'node:fs'; -import path from 'node:path'; -import { afterEach, test, vi } from 'vitest'; -import { mkdtempForTestSync } from './tmp-dir.fixtures.ts'; -import { resetAllProcessMemosForTests } from '@agent-device/kernel/ttl-memo'; -import { resolveAgentDeviceProjectRoot } from './project-root.ts'; -import { findProjectRoot, readVersion } from './version.ts'; - -afterEach(() => { - vi.restoreAllMocks(); -}); - -/** Counts package.json reads while calling through to the real read. */ -function countPackageJsonReads(): () => number { - const readSpy = vi.spyOn(fs, 'readFileSync'); - return () => - readSpy.mock.calls.filter( - ([target]) => typeof target === 'string' && target.endsWith('package.json'), - ).length; -} - -test('readVersion parses each root package.json once per process', () => { - const root = mkdtempForTestSync('agent-device-version-memo-'); - try { - fs.writeFileSync(path.join(root, 'package.json'), '{"version":"1.2.3"}\n', 'utf8'); - const packageJsonReads = countPackageJsonReads(); - - assert.equal(readVersion(root), '1.2.3'); - assert.equal(readVersion(root), '1.2.3'); - - assert.equal(packageJsonReads(), 1); - } finally { - fs.rmSync(root, { recursive: true, force: true }); - } -}); - -test('readVersion keys the memo per root and re-reads after a process memo reset', () => { - const first = mkdtempForTestSync('agent-device-version-memo-first-'); - const second = mkdtempForTestSync('agent-device-version-memo-second-'); - try { - fs.writeFileSync(path.join(first, 'package.json'), '{"version":"1.0.0"}\n', 'utf8'); - fs.writeFileSync(path.join(second, 'package.json'), '{"version":"2.0.0"}\n', 'utf8'); - - assert.equal(readVersion(first), '1.0.0'); - assert.equal(readVersion(second), '2.0.0'); - - fs.writeFileSync(path.join(first, 'package.json'), '{"version":"1.0.1"}\n', 'utf8'); - assert.equal(readVersion(first), '1.0.0'); - - resetAllProcessMemosForTests(); - assert.equal(readVersion(first), '1.0.1'); - } finally { - fs.rmSync(first, { recursive: true, force: true }); - fs.rmSync(second, { recursive: true, force: true }); - } -}); - -test('readVersion does not memoize a package.json it could not read', () => { - const root = mkdtempForTestSync('agent-device-version-memo-absent-'); - try { - assert.equal(readVersion(root), '0.0.0'); - - fs.writeFileSync(path.join(root, 'package.json'), '{"version":"3.0.0"}\n', 'utf8'); - assert.equal(readVersion(root), '3.0.0'); - } finally { - fs.rmSync(root, { recursive: true, force: true }); - } -}); - -test('findProjectRoot walks the ancestor chain once per process', () => { - resetAllProcessMemosForTests(); - const existsSpy = vi.spyOn(fs, 'existsSync'); - - const root = findProjectRoot(); - const walkCalls = existsSpy.mock.calls.length; - assert.ok(walkCalls > 0); - - assert.equal(findProjectRoot(), root); - assert.equal(existsSpy.mock.calls.length, walkCalls); -}); - -test('the resolver walks past a workspace-package manifest to the agent-device root', () => { - const root = mkdtempForTestSync('agent-device-project-root-'); - try { - fs.writeFileSync( - path.join(root, 'package.json'), - JSON.stringify({ name: 'agent-device', version: '9.9.9' }), - ); - const moduleDir = path.join(root, 'packages', 'capture-kit', 'src'); - fs.mkdirSync(moduleDir, { recursive: true }); - fs.writeFileSync( - path.join(root, 'packages', 'capture-kit', 'package.json'), - JSON.stringify({ name: '@agent-device/capture-kit', version: '0.0.0' }), - ); - - assert.equal(resolveAgentDeviceProjectRoot(moduleDir), root); - assert.equal(readVersion(resolveAgentDeviceProjectRoot(moduleDir)), '9.9.9'); - } finally { - resetAllProcessMemosForTests(); - fs.rmSync(root, { recursive: true, force: true }); - } -}); - -test('the resolver falls back to the nearest manifest when none names agent-device', () => { - const root = mkdtempForTestSync('agent-device-project-root-fallback-'); - try { - fs.writeFileSync( - path.join(root, 'package.json'), - JSON.stringify({ name: 'vendored-fork', version: '1.0.0' }), - ); - const nested = path.join(root, 'lib', 'deep'); - fs.mkdirSync(nested, { recursive: true }); - - assert.equal(resolveAgentDeviceProjectRoot(nested), root); - } finally { - resetAllProcessMemosForTests(); - fs.rmSync(root, { recursive: true, force: true }); - } -}); - -test('from this source tree, the project root is the agent-device manifest, not this package', () => { - const resolved = findProjectRoot(); - const manifest = JSON.parse(fs.readFileSync(path.join(resolved, 'package.json'), 'utf8')) as { - name?: string; - version?: string; - }; - assert.equal(manifest.name, 'agent-device'); - assert.equal(readVersion(), manifest.version); +import { test } from 'vitest'; +import { isNewerVersion } from './version.ts'; + +test('isNewerVersion orders release versions numerically per segment', () => { + assert.equal(isNewerVersion('0.21.6', '0.20.8'), true); + assert.equal(isNewerVersion('0.21.12', '0.21.6'), true); + assert.equal(isNewerVersion('1.0.0', '0.21.12'), true); + assert.equal(isNewerVersion('0.20.8', '0.21.6'), false); + assert.equal(isNewerVersion('0.21.6', '0.21.6'), false); }); diff --git a/packages/host-kit/src/internal/version.ts b/packages/host-kit/src/internal/version.ts index bc10587fea..bf805c4cb0 100644 --- a/packages/host-kit/src/internal/version.ts +++ b/packages/host-kit/src/internal/version.ts @@ -31,3 +31,12 @@ export function findProjectRoot(): string { projectRootMemo.set('self', resolved); return resolved; } + +/** + * Whether `candidate` sorts after `baseline` as a release version. Numeric-aware string order is + * enough for the daemon takeover decision: it only has to tell an upgrade from a downgrade, and + * equal strings are never compared here. + */ +export function isNewerVersion(candidate: string, baseline: string): boolean { + return candidate.localeCompare(baseline, undefined, { numeric: true }) > 0; +} diff --git a/packages/host-kit/src/version.ts b/packages/host-kit/src/version.ts index 00d05cc677..5c82b4fd48 100644 --- a/packages/host-kit/src/version.ts +++ b/packages/host-kit/src/version.ts @@ -1,2 +1,2 @@ -export { findProjectRoot, readVersion } from './internal/version.ts'; +export { findProjectRoot, isNewerVersion, readVersion } from './internal/version.ts'; export { DAEMON_SOURCE_ENTRY, isSourceCheckoutProjectRoot } from './internal/project-root.ts'; diff --git a/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts b/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts index 810672ff6b..fd43312b1d 100644 --- a/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts +++ b/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts @@ -566,6 +566,55 @@ test('sendToDaemon does not reuse reachable daemon metadata with mismatched vers } }); +test('sendToDaemon refuses to replace a reachable daemon newer than the client', async (t) => { + if (!(await supportsLoopbackBind())) { + t.skip('loopback listeners are not permitted in this environment'); + return; + } + // The hoisted-CLI shape: an older agent-device on PATH meets the daemon a newer install + // started, with live sessions attached. It must neither spawn nor kill anything. + const stateDir = makeTempStateDir('agent-device-daemon-newer-refused-'); + const paths = resolveDaemonPaths(stateDir); + const newerDaemon = await startHttpDaemonFixture({ via: 'newer-daemon' }); + vi.stubEnv('AGENT_DEVICE_STATE_DIR', stateDir); + mockRunCmdDetached.mockReset(); + writeDaemonInfo(paths, { + httpPort: newerDaemon.port, + transport: 'http', + pid: 999_999, + version: '999.0.0', + }); + const stderrCapture = captureStderr(); + + try { + await assert.rejects( + () => + sendToDaemon({ + session: 'default', + command: 'newer-daemon-smoke', + positionals: [], + flags: { stateDir, daemonTransport: 'http' }, + meta: { requestId: 'req-newer-daemon' }, + }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.match(error.message, /v999\.0\.0\) is newer than this client/); + return true; + }, + ); + + assert.equal(mockRunCmdDetached.mock.calls.length, 0); + assert.deepEqual(newerDaemon.seenPaths, ['GET /health']); + assert.equal(stderrCapture.read(), ''); + assert.ok(fs.existsSync(paths.infoPath), 'the newer daemon keeps its metadata'); + } finally { + stderrCapture.restore(); + await closeLoopbackServer(newerDaemon.server); + fs.rmSync(stateDir, { recursive: true, force: true }); + vi.unstubAllEnvs(); + } +}); + test('sendToDaemon prints a takeover notice before replacing an unreachable daemon', async (t) => { if (!(await supportsLoopbackBind())) { t.skip('loopback listeners are not permitted in this environment'); diff --git a/src/daemon-client/__tests__/daemon-launch-spec.test.ts b/src/daemon-client/__tests__/daemon-launch-spec.test.ts index 0a9b7f587f..9f31b6e823 100644 --- a/src/daemon-client/__tests__/daemon-launch-spec.test.ts +++ b/src/daemon-client/__tests__/daemon-launch-spec.test.ts @@ -2,6 +2,7 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; import { afterEach, test, vi } from 'vitest'; +import { AppError } from '@agent-device/kernel/errors'; import { computeDaemonCodeSignature } from '@agent-device/host-kit/code-signature'; import { resolveDaemonLaunchSpec, @@ -106,6 +107,42 @@ test('a source client fingerprints the source entry through the stat-validated c * in; these cases flip the two inputs that separate the trees — which tree this client * runs from, and which tree the running daemon says it was started from (#2458). */ +test('a reachable daemon newer than the client is refused, not replaced', async () => { + const clientVersion = readVersion(); + await assert.rejects( + () => resolveDaemonTakeoverReason(runningDaemon({ version: '999.0.0' }), true, '/tmp/state'), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal( + error.message, + `Daemon (pid 999999, v999.0.0) is newer than this client (v${clientVersion}); refusing to replace it.`, + ); + assert.equal(error.details?.daemonVersion, '999.0.0'); + assert.equal(error.details?.clientVersion, clientVersion); + assert.match( + String(error.details?.hint), + /agent-device daemon stop --state-dir \/tmp\/state/, + ); + return true; + }, + ); +}); + +test('an unreachable newer daemon is replaced like any version mismatch', async () => { + assert.equal( + await resolveDaemonTakeoverReason(runningDaemon({ version: '999.0.0' }), false), + `version mismatch (client v${readVersion()})`, + ); +}); + +test('a reachable daemon older than the client is replaced', async () => { + assert.equal( + await resolveDaemonTakeoverReason(runningDaemon({ version: '0.0.1' }), true), + `version mismatch (client v${readVersion()})`, + ); +}); + function useClientTree(sourceCheckout: boolean): void { vi.mocked(isSourceCheckoutProjectRoot).mockReturnValue(sourceCheckout); } diff --git a/src/daemon-client/daemon-client-lifecycle.ts b/src/daemon-client/daemon-client-lifecycle.ts index d8d194e1c3..464c013de6 100644 --- a/src/daemon-client/daemon-client-lifecycle.ts +++ b/src/daemon-client/daemon-client-lifecycle.ts @@ -182,7 +182,11 @@ async function readReusableLocalDaemon(settings: DaemonClientSettings): Promise< if (!existing) return null; const existingReachable = await canConnectReusableDaemon(existing, settings.transportPreference); - const takeoverReason = await resolveDaemonTakeoverReason(existing, existingReachable); + const takeoverReason = await resolveDaemonTakeoverReason( + existing, + existingReachable, + settings.paths.baseDir, + ); if (!takeoverReason) return existing; emitDaemonTakeoverNotice(existing, takeoverReason, settings.paths.baseDir); diff --git a/src/daemon-client/daemon-launch-spec.ts b/src/daemon-client/daemon-launch-spec.ts index 645bc89636..d52e8557b4 100644 --- a/src/daemon-client/daemon-launch-spec.ts +++ b/src/daemon-client/daemon-launch-spec.ts @@ -1,7 +1,12 @@ import fs from 'node:fs'; import path from 'node:path'; import { AppError } from '@agent-device/kernel/errors'; -import { DAEMON_SOURCE_ENTRY, findProjectRoot, readVersion } from '@agent-device/host-kit/version'; +import { + DAEMON_SOURCE_ENTRY, + findProjectRoot, + isNewerVersion, + readVersion, +} from '@agent-device/host-kit/version'; import { createTtlMemo } from '@agent-device/kernel/ttl-memo'; import { @@ -110,12 +115,24 @@ export async function resolveLocalDaemonCodeIdentity(): Promise { - if (info.version !== readVersion()) return `version mismatch (client v${readVersion()})`; + const clientVersion = readVersion(); + if (info.version !== clientVersion) { + if (reachable && info.version && isNewerVersion(info.version, clientVersion)) { + throw newerDaemonRefusedError(info, info.version, clientVersion, stateDir); + } + return `version mismatch (client v${clientVersion})`; + } const localIdentity = await resolveLocalDaemonCodeIdentity(); const codeMismatch = resolveCodeIdentityMismatch(localIdentity, info); if (codeMismatch) return codeMismatch; @@ -157,3 +174,24 @@ function describeCodeOriginMismatch( ): `code origin mismatch (${string}, client ${string})` { return `code origin mismatch (daemon ${info.codeOrigin ?? 'unreported'}, client ${local.origin})`; } + +function newerDaemonRefusedError( + info: DaemonInfo, + daemonVersion: string, + clientVersion: string, + stateDir: string | undefined, +): AppError { + const stopCommand = stateDir + ? `agent-device daemon stop --state-dir ${stateDir}` + : 'agent-device daemon stop'; + return new AppError( + 'COMMAND_FAILED', + `Daemon (pid ${info.pid}, v${daemonVersion}) is newer than this client (v${clientVersion}); refusing to replace it.`, + { + daemonPid: info.pid, + daemonVersion, + clientVersion, + hint: `Run the agent-device v${daemonVersion} CLI that started this daemon (an older copy was probably hoisted onto PATH), or stop it first with: ${stopCommand}.`, + }, + ); +} From 537d5b461e0eb93f4339fe7acd344ce9547ed889 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Kwas=CC=81niewski?= Date: Wed, 23 Sep 2026 14:21:30 +0200 Subject: [PATCH 2/5] fix(daemon): compare daemon versions as SemVer and quote the stop hint 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 --- .../host-kit/src/internal/version.test.ts | 147 +++++++++++++++++- packages/host-kit/src/internal/version.ts | 48 +++++- .../__tests__/daemon-client-lifecycle.test.ts | 49 ------ .../daemon-client-newer-daemon.test.ts | 117 ++++++++++++++ .../__tests__/daemon-launch-spec.test.ts | 5 +- src/daemon-client/daemon-launch-spec.ts | 3 +- 6 files changed, 310 insertions(+), 59 deletions(-) create mode 100644 src/daemon-client/__tests__/daemon-client-newer-daemon.test.ts diff --git a/packages/host-kit/src/internal/version.test.ts b/packages/host-kit/src/internal/version.test.ts index 85b2e3271f..5118b87f12 100644 --- a/packages/host-kit/src/internal/version.test.ts +++ b/packages/host-kit/src/internal/version.test.ts @@ -1,11 +1,152 @@ import assert from 'node:assert/strict'; -import { test } from 'vitest'; -import { isNewerVersion } from './version.ts'; +import fs from 'node:fs'; +import path from 'node:path'; +import { afterEach, test, vi } from 'vitest'; +import { mkdtempForTestSync } from './tmp-dir.fixtures.ts'; +import { resetAllProcessMemosForTests } from '@agent-device/kernel/ttl-memo'; +import { resolveAgentDeviceProjectRoot } from './project-root.ts'; +import { findProjectRoot, isNewerVersion, readVersion } from './version.ts'; -test('isNewerVersion orders release versions numerically per segment', () => { +afterEach(() => { + vi.restoreAllMocks(); +}); + +/** Counts package.json reads while calling through to the real read. */ +function countPackageJsonReads(): () => number { + const readSpy = vi.spyOn(fs, 'readFileSync'); + return () => + readSpy.mock.calls.filter( + ([target]) => typeof target === 'string' && target.endsWith('package.json'), + ).length; +} + +test('readVersion parses each root package.json once per process', () => { + const root = mkdtempForTestSync('agent-device-version-memo-'); + try { + fs.writeFileSync(path.join(root, 'package.json'), '{"version":"1.2.3"}\n', 'utf8'); + const packageJsonReads = countPackageJsonReads(); + + assert.equal(readVersion(root), '1.2.3'); + assert.equal(readVersion(root), '1.2.3'); + + assert.equal(packageJsonReads(), 1); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +test('readVersion keys the memo per root and re-reads after a process memo reset', () => { + const first = mkdtempForTestSync('agent-device-version-memo-first-'); + const second = mkdtempForTestSync('agent-device-version-memo-second-'); + try { + fs.writeFileSync(path.join(first, 'package.json'), '{"version":"1.0.0"}\n', 'utf8'); + fs.writeFileSync(path.join(second, 'package.json'), '{"version":"2.0.0"}\n', 'utf8'); + + assert.equal(readVersion(first), '1.0.0'); + assert.equal(readVersion(second), '2.0.0'); + + fs.writeFileSync(path.join(first, 'package.json'), '{"version":"1.0.1"}\n', 'utf8'); + assert.equal(readVersion(first), '1.0.0'); + + resetAllProcessMemosForTests(); + assert.equal(readVersion(first), '1.0.1'); + } finally { + fs.rmSync(first, { recursive: true, force: true }); + fs.rmSync(second, { recursive: true, force: true }); + } +}); + +test('readVersion does not memoize a package.json it could not read', () => { + const root = mkdtempForTestSync('agent-device-version-memo-absent-'); + try { + assert.equal(readVersion(root), '0.0.0'); + + fs.writeFileSync(path.join(root, 'package.json'), '{"version":"3.0.0"}\n', 'utf8'); + assert.equal(readVersion(root), '3.0.0'); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +test('findProjectRoot walks the ancestor chain once per process', () => { + resetAllProcessMemosForTests(); + const existsSpy = vi.spyOn(fs, 'existsSync'); + + const root = findProjectRoot(); + const walkCalls = existsSpy.mock.calls.length; + assert.ok(walkCalls > 0); + + assert.equal(findProjectRoot(), root); + assert.equal(existsSpy.mock.calls.length, walkCalls); +}); + +test('the resolver walks past a workspace-package manifest to the agent-device root', () => { + const root = mkdtempForTestSync('agent-device-project-root-'); + try { + fs.writeFileSync( + path.join(root, 'package.json'), + JSON.stringify({ name: 'agent-device', version: '9.9.9' }), + ); + const moduleDir = path.join(root, 'packages', 'capture-kit', 'src'); + fs.mkdirSync(moduleDir, { recursive: true }); + fs.writeFileSync( + path.join(root, 'packages', 'capture-kit', 'package.json'), + JSON.stringify({ name: '@agent-device/capture-kit', version: '0.0.0' }), + ); + + assert.equal(resolveAgentDeviceProjectRoot(moduleDir), root); + assert.equal(readVersion(resolveAgentDeviceProjectRoot(moduleDir)), '9.9.9'); + } finally { + resetAllProcessMemosForTests(); + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +test('the resolver falls back to the nearest manifest when none names agent-device', () => { + const root = mkdtempForTestSync('agent-device-project-root-fallback-'); + try { + fs.writeFileSync( + path.join(root, 'package.json'), + JSON.stringify({ name: 'vendored-fork', version: '1.0.0' }), + ); + const nested = path.join(root, 'lib', 'deep'); + fs.mkdirSync(nested, { recursive: true }); + + assert.equal(resolveAgentDeviceProjectRoot(nested), root); + } finally { + resetAllProcessMemosForTests(); + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +test('from this source tree, the project root is the agent-device manifest, not this package', () => { + const resolved = findProjectRoot(); + const manifest = JSON.parse(fs.readFileSync(path.join(resolved, 'package.json'), 'utf8')) as { + name?: string; + version?: string; + }; + assert.equal(manifest.name, 'agent-device'); + assert.equal(readVersion(), manifest.version); +}); + +test('isNewerVersion orders releases numerically per segment', () => { assert.equal(isNewerVersion('0.21.6', '0.20.8'), true); assert.equal(isNewerVersion('0.21.12', '0.21.6'), true); assert.equal(isNewerVersion('1.0.0', '0.21.12'), true); assert.equal(isNewerVersion('0.20.8', '0.21.6'), false); assert.equal(isNewerVersion('0.21.6', '0.21.6'), false); }); + +test('isNewerVersion ranks a release above the prerelease of the same base', () => { + // main carries `-dev` between releases (scripts/release-mark-dev.mjs): a released client meeting + // a `-dev` daemon of the same base is the upgrade, and the reverse is the downgrade. + assert.equal(isNewerVersion('0.21.13', '0.21.13-dev'), true); + assert.equal(isNewerVersion('0.21.13-dev', '0.21.13'), false); + assert.equal(isNewerVersion('0.21.13-dev', '0.21.12'), true); + assert.equal(isNewerVersion('0.21.12', '0.21.13-dev'), false); + assert.equal(isNewerVersion('0.21.13-dev', '0.21.13-dev'), false); + assert.equal(isNewerVersion('0.21.13-rc.2', '0.21.13-rc.1'), true); + assert.equal(isNewerVersion('0.21.13-rc.10', '0.21.13-rc.9'), true); + assert.equal(isNewerVersion('0.21.13-beta', '0.21.13-alpha.1'), true); + assert.equal(isNewerVersion('0.21.13+build.2', '0.21.13+build.1'), false); +}); diff --git a/packages/host-kit/src/internal/version.ts b/packages/host-kit/src/internal/version.ts index bf805c4cb0..04cd5cecbd 100644 --- a/packages/host-kit/src/internal/version.ts +++ b/packages/host-kit/src/internal/version.ts @@ -33,10 +33,50 @@ export function findProjectRoot(): string { } /** - * Whether `candidate` sorts after `baseline` as a release version. Numeric-aware string order is - * enough for the daemon takeover decision: it only has to tell an upgrade from a downgrade, and - * equal strings are never compared here. + * Whether `candidate` is a later release than `baseline` under SemVer ordering: numeric + * `major.minor.patch` first, then a release sorts after any prerelease of the same base + * (`0.21.13` > `0.21.13-dev`), and prerelease identifiers compare per dot-separated field, + * numerically when both are numbers and lexically otherwise. Build metadata is ignored. The daemon + * takeover decision needs exactly this to tell an upgrade from a downgrade across the `-dev` + * versions main carries between releases. */ export function isNewerVersion(candidate: string, baseline: string): boolean { - return candidate.localeCompare(baseline, undefined, { numeric: true }) > 0; + return compareVersions(candidate, baseline) > 0; +} + +function compareVersions(left: string, right: string): number { + const a = parseVersion(left); + const b = parseVersion(right); + for (let i = 0; i < 3; i += 1) { + const x = a.release[i] ?? 0; + const y = b.release[i] ?? 0; + if (x !== y) return x > y ? 1 : -1; + } + if (a.prerelease.length === 0 || b.prerelease.length === 0) { + return Math.sign(b.prerelease.length - a.prerelease.length); + } + const fields = Math.max(a.prerelease.length, b.prerelease.length); + for (let i = 0; i < fields; i += 1) { + const x = a.prerelease[i]; + const y = b.prerelease[i]; + if (x === undefined) return -1; + if (y === undefined) return 1; + if (x === y) continue; + const xNumeric = /^\d+$/.test(x); + const yNumeric = /^\d+$/.test(y); + if (xNumeric && yNumeric) return Number(x) > Number(y) ? 1 : -1; + if (xNumeric !== yNumeric) return xNumeric ? -1 : 1; + return x > y ? 1 : -1; + } + return 0; +} + +function parseVersion(version: string): { release: number[]; prerelease: string[] } { + const [core = '', prerelease = ''] = version.split('+', 1)[0]!.split(/-(.*)/s, 2); + const release = core.split('.').map((part) => Number.parseInt(part, 10)); + while (release.length < 3) release.push(0); + return { + release: release.map((part) => (Number.isNaN(part) ? 0 : part)), + prerelease: prerelease ? prerelease.split('.') : [], + }; } diff --git a/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts b/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts index fd43312b1d..810672ff6b 100644 --- a/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts +++ b/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts @@ -566,55 +566,6 @@ test('sendToDaemon does not reuse reachable daemon metadata with mismatched vers } }); -test('sendToDaemon refuses to replace a reachable daemon newer than the client', async (t) => { - if (!(await supportsLoopbackBind())) { - t.skip('loopback listeners are not permitted in this environment'); - return; - } - // The hoisted-CLI shape: an older agent-device on PATH meets the daemon a newer install - // started, with live sessions attached. It must neither spawn nor kill anything. - const stateDir = makeTempStateDir('agent-device-daemon-newer-refused-'); - const paths = resolveDaemonPaths(stateDir); - const newerDaemon = await startHttpDaemonFixture({ via: 'newer-daemon' }); - vi.stubEnv('AGENT_DEVICE_STATE_DIR', stateDir); - mockRunCmdDetached.mockReset(); - writeDaemonInfo(paths, { - httpPort: newerDaemon.port, - transport: 'http', - pid: 999_999, - version: '999.0.0', - }); - const stderrCapture = captureStderr(); - - try { - await assert.rejects( - () => - sendToDaemon({ - session: 'default', - command: 'newer-daemon-smoke', - positionals: [], - flags: { stateDir, daemonTransport: 'http' }, - meta: { requestId: 'req-newer-daemon' }, - }), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.match(error.message, /v999\.0\.0\) is newer than this client/); - return true; - }, - ); - - assert.equal(mockRunCmdDetached.mock.calls.length, 0); - assert.deepEqual(newerDaemon.seenPaths, ['GET /health']); - assert.equal(stderrCapture.read(), ''); - assert.ok(fs.existsSync(paths.infoPath), 'the newer daemon keeps its metadata'); - } finally { - stderrCapture.restore(); - await closeLoopbackServer(newerDaemon.server); - fs.rmSync(stateDir, { recursive: true, force: true }); - vi.unstubAllEnvs(); - } -}); - test('sendToDaemon prints a takeover notice before replacing an unreachable daemon', async (t) => { if (!(await supportsLoopbackBind())) { t.skip('loopback listeners are not permitted in this environment'); diff --git a/src/daemon-client/__tests__/daemon-client-newer-daemon.test.ts b/src/daemon-client/__tests__/daemon-client-newer-daemon.test.ts new file mode 100644 index 0000000000..9eba3b28cf --- /dev/null +++ b/src/daemon-client/__tests__/daemon-client-newer-daemon.test.ts @@ -0,0 +1,117 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import http from 'node:http'; +import { afterEach, test, vi } from 'vitest'; +import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; + +vi.mock('@agent-device/host-kit/command', async (importOriginal) => ({ + ...(await importOriginal()), + runCmdDetached: vi.fn(), + runCmdDetachedMonitored: vi.fn(), + runCmdSync: vi.fn(() => ({ exitCode: 1, stdout: '', stderr: '' })), +})); + +import { resolveDaemonPaths } from '../../daemon-resolution.ts'; +import { sendToDaemon } from '../daemon-client.ts'; +import { + closeLoopbackServer, + listenOnLoopback, + supportsLoopbackBind, +} from '../../__tests__/test-utils/loopback.ts'; +import { AppError } from '@agent-device/kernel/errors'; +import { runCmdDetachedMonitored } from '@agent-device/host-kit/command'; + +// The daemon-version half of the takeover ladder (`resolveDaemonTakeoverReason`): an older CLI +// hoisted onto PATH meets the daemon a newer install started, with live sessions attached. It must +// neither spawn a replacement nor kill the daemon. + +const mockRunCmdDetached = vi.mocked(runCmdDetachedMonitored); + +afterEach(() => { + mockRunCmdDetached.mockReset(); + vi.unstubAllEnvs(); +}); + +/** A reachable daemon that answers `/health` and records every path it was asked for. */ +async function startHealthyDaemon(): Promise<{ + server: http.Server; + port: number; + seenPaths: string[]; +}> { + const seenPaths: string[] = []; + const server = http.createServer((req, res) => { + const url = new URL(req.url || '/', 'http://127.0.0.1'); + seenPaths.push(`${req.method ?? 'GET'} ${url.pathname}`); + res.writeHead(url.pathname === '/health' ? 200 : 404); + res.end(url.pathname === '/health' ? 'ok' : 'not found'); + }); + const port = await listenOnLoopback(server); + return { server, port, seenPaths }; +} + +function captureStderr(): { read: () => string; restore: () => void } { + const originalWrite = process.stderr.write.bind(process.stderr); + let captured = ''; + (process.stderr as { write: typeof process.stderr.write }).write = ((chunk: unknown) => { + captured += String(chunk); + return true; + }) as typeof process.stderr.write; + return { + read: () => captured, + restore: () => { + process.stderr.write = originalWrite; + }, + }; +} + +test('sendToDaemon refuses to replace a reachable daemon newer than the client', async (t) => { + if (!(await supportsLoopbackBind())) { + t.skip('loopback listeners are not permitted in this environment'); + return; + } + const stateDir = mkdtempForTestSync('agent-device-daemon-newer-refused-'); + const paths = resolveDaemonPaths(stateDir); + const newerDaemon = await startHealthyDaemon(); + vi.stubEnv('AGENT_DEVICE_STATE_DIR', stateDir); + fs.mkdirSync(paths.baseDir, { recursive: true }); + fs.writeFileSync( + paths.infoPath, + `${JSON.stringify({ + token: 'local-secret', + pid: 999_999, + version: '999.0.0', + httpPort: newerDaemon.port, + transport: 'http', + })}\n`, + 'utf8', + ); + const stderrCapture = captureStderr(); + + try { + await assert.rejects( + () => + sendToDaemon({ + session: 'default', + command: 'newer-daemon-smoke', + positionals: [], + flags: { stateDir, daemonTransport: 'http' }, + meta: { requestId: 'req-newer-daemon' }, + }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.match(error.message, /v999\.0\.0\) is newer than this client/); + assert.match(String(error.details?.hint), /agent-device daemon stop --state-dir /); + return true; + }, + ); + + assert.equal(mockRunCmdDetached.mock.calls.length, 0, 'no replacement daemon is spawned'); + assert.deepEqual(newerDaemon.seenPaths, ['GET /health']); + assert.equal(stderrCapture.read(), '', 'no takeover notice is printed'); + assert.ok(fs.existsSync(paths.infoPath), 'the newer daemon keeps its metadata'); + } finally { + stderrCapture.restore(); + await closeLoopbackServer(newerDaemon.server); + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); diff --git a/src/daemon-client/__tests__/daemon-launch-spec.test.ts b/src/daemon-client/__tests__/daemon-launch-spec.test.ts index 9f31b6e823..7714d4d284 100644 --- a/src/daemon-client/__tests__/daemon-launch-spec.test.ts +++ b/src/daemon-client/__tests__/daemon-launch-spec.test.ts @@ -110,7 +110,8 @@ test('a source client fingerprints the source entry through the stat-validated c test('a reachable daemon newer than the client is refused, not replaced', async () => { const clientVersion = readVersion(); await assert.rejects( - () => resolveDaemonTakeoverReason(runningDaemon({ version: '999.0.0' }), true, '/tmp/state'), + () => + resolveDaemonTakeoverReason(runningDaemon({ version: '999.0.0' }), true, '/tmp/state dir'), (error: unknown) => { assert.ok(error instanceof AppError); assert.equal(error.code, 'COMMAND_FAILED'); @@ -122,7 +123,7 @@ test('a reachable daemon newer than the client is refused, not replaced', async assert.equal(error.details?.clientVersion, clientVersion); assert.match( String(error.details?.hint), - /agent-device daemon stop --state-dir \/tmp\/state/, + /agent-device daemon stop --state-dir '\/tmp\/state dir'/, ); return true; }, diff --git a/src/daemon-client/daemon-launch-spec.ts b/src/daemon-client/daemon-launch-spec.ts index d52e8557b4..6a3305d016 100644 --- a/src/daemon-client/daemon-launch-spec.ts +++ b/src/daemon-client/daemon-launch-spec.ts @@ -1,6 +1,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { AppError } from '@agent-device/kernel/errors'; +import { shellQuoteIfNeeded } from '@agent-device/kernel/device-shell'; import { DAEMON_SOURCE_ENTRY, findProjectRoot, @@ -182,7 +183,7 @@ function newerDaemonRefusedError( stateDir: string | undefined, ): AppError { const stopCommand = stateDir - ? `agent-device daemon stop --state-dir ${stateDir}` + ? `agent-device daemon stop --state-dir ${shellQuoteIfNeeded(stateDir)}` : 'agent-device daemon stop'; return new AppError( 'COMMAND_FAILED', From f13e49f43356c27d85702f2bcb7f47c8e13976e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Kwas=CC=81niewski?= Date: Wed, 23 Sep 2026 14:28:17 +0200 Subject: [PATCH 3/5] refactor(host-kit): split the SemVer comparator so each rule reads alone The Fallow complexity gate flagged compareVersions at cyclomatic 18; release, prerelease and per-field comparison are now separate functions. --- packages/host-kit/src/internal/version.ts | 41 +++++++++++++++-------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/packages/host-kit/src/internal/version.ts b/packages/host-kit/src/internal/version.ts index 04cd5cecbd..37469f314c 100644 --- a/packages/host-kit/src/internal/version.ts +++ b/packages/host-kit/src/internal/version.ts @@ -47,30 +47,43 @@ export function isNewerVersion(candidate: string, baseline: string): boolean { function compareVersions(left: string, right: string): number { const a = parseVersion(left); const b = parseVersion(right); + return compareRelease(a.release, b.release) || comparePrerelease(a.prerelease, b.prerelease); +} + +function compareRelease(a: number[], b: number[]): number { for (let i = 0; i < 3; i += 1) { - const x = a.release[i] ?? 0; - const y = b.release[i] ?? 0; + const x = a[i] ?? 0; + const y = b[i] ?? 0; if (x !== y) return x > y ? 1 : -1; } - if (a.prerelease.length === 0 || b.prerelease.length === 0) { - return Math.sign(b.prerelease.length - a.prerelease.length); - } - const fields = Math.max(a.prerelease.length, b.prerelease.length); + return 0; +} + +/** A release (no prerelease) sorts after every prerelease of the same base. */ +function comparePrerelease(a: string[], b: string[]): number { + if (a.length === 0 || b.length === 0) return Math.sign(b.length - a.length); + const fields = Math.max(a.length, b.length); for (let i = 0; i < fields; i += 1) { - const x = a.prerelease[i]; - const y = b.prerelease[i]; + const x = a[i]; + const y = b[i]; if (x === undefined) return -1; if (y === undefined) return 1; - if (x === y) continue; - const xNumeric = /^\d+$/.test(x); - const yNumeric = /^\d+$/.test(y); - if (xNumeric && yNumeric) return Number(x) > Number(y) ? 1 : -1; - if (xNumeric !== yNumeric) return xNumeric ? -1 : 1; - return x > y ? 1 : -1; + const order = comparePrereleaseField(x, y); + if (order !== 0) return order; } return 0; } +/** Numeric fields compare as numbers and sort below alphanumeric ones; the rest compare lexically. */ +function comparePrereleaseField(x: string, y: string): number { + if (x === y) return 0; + const xNumeric = /^\d+$/.test(x); + const yNumeric = /^\d+$/.test(y); + if (xNumeric && yNumeric) return Number(x) > Number(y) ? 1 : -1; + if (xNumeric !== yNumeric) return xNumeric ? -1 : 1; + return x > y ? 1 : -1; +} + function parseVersion(version: string): { release: number[]; prerelease: string[] } { const [core = '', prerelease = ''] = version.split('+', 1)[0]!.split(/-(.*)/s, 2); const release = core.split('.').map((part) => Number.parseInt(part, 10)); From 1bcdc8c84dacceaeae6fc1d329231baba167bd42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Kwas=CC=81niewski?= Date: Wed, 23 Sep 2026 14:49:13 +0200 Subject: [PATCH 4/5] refactor(daemon): takeover decision as a value, one SemVer comparator 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. --- .../host-kit/src/internal/version.test.ts | 9 +- packages/host-kit/src/internal/version.ts | 48 ++++--- packages/host-kit/src/version.ts | 7 +- .../test-utils/daemon-http-fixture.ts | 74 ++++++++++ src/__tests__/update-check.test.ts | 26 ++++ src/cli/update-check.ts | 5 +- .../__tests__/daemon-client-lifecycle.test.ts | 72 +--------- .../daemon-client-newer-daemon.test.ts | 45 +----- .../__tests__/daemon-launch-spec.test.ts | 129 ++++++++---------- src/daemon-client/daemon-client-lifecycle.ts | 37 +++-- src/daemon-client/daemon-launch-spec.ts | 62 +++------ 11 files changed, 261 insertions(+), 253 deletions(-) create mode 100644 src/__tests__/test-utils/daemon-http-fixture.ts diff --git a/packages/host-kit/src/internal/version.test.ts b/packages/host-kit/src/internal/version.test.ts index 5118b87f12..e46f17e3f3 100644 --- a/packages/host-kit/src/internal/version.test.ts +++ b/packages/host-kit/src/internal/version.test.ts @@ -5,7 +5,7 @@ import { afterEach, test, vi } from 'vitest'; import { mkdtempForTestSync } from './tmp-dir.fixtures.ts'; import { resetAllProcessMemosForTests } from '@agent-device/kernel/ttl-memo'; import { resolveAgentDeviceProjectRoot } from './project-root.ts'; -import { findProjectRoot, isNewerVersion, readVersion } from './version.ts'; +import { compareVersions, findProjectRoot, isNewerVersion, readVersion } from './version.ts'; afterEach(() => { vi.restoreAllMocks(); @@ -150,3 +150,10 @@ test('isNewerVersion ranks a release above the prerelease of the same base', () assert.equal(isNewerVersion('0.21.13-beta', '0.21.13-alpha.1'), true); assert.equal(isNewerVersion('0.21.13+build.2', '0.21.13+build.1'), false); }); + +test('compareVersions reads malformed or prefixed versions conservatively', () => { + assert.equal(compareVersions('v0.21.13', '0.21.13'), 0); + assert.equal(compareVersions('garbage', '0.0.1'), -1); + assert.equal(compareVersions('garbage', '0.0.0'), 0); + assert.equal(compareVersions('1.2.3.4', '1.2.3'), -1, 'four segments is not a version'); +}); diff --git a/packages/host-kit/src/internal/version.ts b/packages/host-kit/src/internal/version.ts index 37469f314c..82d9c14823 100644 --- a/packages/host-kit/src/internal/version.ts +++ b/packages/host-kit/src/internal/version.ts @@ -33,35 +33,49 @@ export function findProjectRoot(): string { } /** - * Whether `candidate` is a later release than `baseline` under SemVer ordering: numeric - * `major.minor.patch` first, then a release sorts after any prerelease of the same base - * (`0.21.13` > `0.21.13-dev`), and prerelease identifiers compare per dot-separated field, - * numerically when both are numbers and lexically otherwise. Build metadata is ignored. The daemon - * takeover decision needs exactly this to tell an upgrade from a downgrade across the `-dev` - * versions main carries between releases. + * Whether `candidate` is a later release than `baseline` (see {@link compareVersions}). */ export function isNewerVersion(candidate: string, baseline: string): boolean { return compareVersions(candidate, baseline) > 0; } -function compareVersions(left: string, right: string): number { +/** + * SemVer order for the versions this package publishes: numeric `major.minor.patch` first, then a + * release sorts after any prerelease of the same base (`0.21.13` > `0.21.13-dev`, the shape main + * carries between releases), and prerelease fields compare per dot-separated field, numerically + * when both are numbers and lexically otherwise. Build metadata is ignored. A string that is not a + * version at all reads as `0.0.0`, so a malformed version always compares as the oldest. + */ +export function compareVersions(left: string, right: string): number { const a = parseVersion(left); const b = parseVersion(right); return compareRelease(a.release, b.release) || comparePrerelease(a.prerelease, b.prerelease); } -function compareRelease(a: number[], b: number[]): number { +const SEMVER = /^v?(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z.-]+))?(?:\+[0-9A-Za-z.-]+)?$/; + +type ParsedVersion = { release: [number, number, number]; prerelease: string[] }; + +function parseVersion(version: string): ParsedVersion { + const match = SEMVER.exec(version.trim()); + if (!match) return { release: [0, 0, 0], prerelease: [] }; + return { + release: [Number(match[1]), Number(match[2]), Number(match[3])], + prerelease: match[4]?.split('.') ?? [], + }; +} + +function compareRelease(a: ParsedVersion['release'], b: ParsedVersion['release']): number { for (let i = 0; i < 3; i += 1) { - const x = a[i] ?? 0; - const y = b[i] ?? 0; - if (x !== y) return x > y ? 1 : -1; + if (a[i] !== b[i]) return a[i]! > b[i]! ? 1 : -1; } return 0; } /** A release (no prerelease) sorts after every prerelease of the same base. */ function comparePrerelease(a: string[], b: string[]): number { - if (a.length === 0 || b.length === 0) return Math.sign(b.length - a.length); + if (a.length === 0) return b.length === 0 ? 0 : 1; + if (b.length === 0) return -1; const fields = Math.max(a.length, b.length); for (let i = 0; i < fields; i += 1) { const x = a[i]; @@ -83,13 +97,3 @@ function comparePrereleaseField(x: string, y: string): number { if (xNumeric !== yNumeric) return xNumeric ? -1 : 1; return x > y ? 1 : -1; } - -function parseVersion(version: string): { release: number[]; prerelease: string[] } { - const [core = '', prerelease = ''] = version.split('+', 1)[0]!.split(/-(.*)/s, 2); - const release = core.split('.').map((part) => Number.parseInt(part, 10)); - while (release.length < 3) release.push(0); - return { - release: release.map((part) => (Number.isNaN(part) ? 0 : part)), - prerelease: prerelease ? prerelease.split('.') : [], - }; -} diff --git a/packages/host-kit/src/version.ts b/packages/host-kit/src/version.ts index 5c82b4fd48..2db3b9d7a0 100644 --- a/packages/host-kit/src/version.ts +++ b/packages/host-kit/src/version.ts @@ -1,2 +1,7 @@ -export { findProjectRoot, isNewerVersion, readVersion } from './internal/version.ts'; +export { + compareVersions, + findProjectRoot, + isNewerVersion, + readVersion, +} from './internal/version.ts'; export { DAEMON_SOURCE_ENTRY, isSourceCheckoutProjectRoot } from './internal/project-root.ts'; diff --git a/src/__tests__/test-utils/daemon-http-fixture.ts b/src/__tests__/test-utils/daemon-http-fixture.ts new file mode 100644 index 0000000000..65dba9ab23 --- /dev/null +++ b/src/__tests__/test-utils/daemon-http-fixture.ts @@ -0,0 +1,74 @@ +import http from 'node:http'; +import { listenOnLoopback } from './loopback.ts'; + +// A loopback stand-in for a running daemon: answers `GET /health`, echoes `responseData` as the +// result of every `POST /rpc`, and records what it was asked, for the daemon-client tests that +// decide which daemon a command keeps. + +export type HttpDaemonFixture = { + server: http.Server; + port: number; + seenPaths: string[]; + rpcRequests: Record[]; +}; + +export async function startHttpDaemonFixture( + responseData: Record, +): Promise { + const seenPaths: string[] = []; + const rpcRequests: Record[] = []; + const server = http.createServer((req, res) => { + const url = new URL(req.url || '/', 'http://127.0.0.1'); + seenPaths.push(`${req.method ?? 'GET'} ${url.pathname}`); + + if (req.method === 'GET' && url.pathname === '/health') { + res.writeHead(200); + res.end('ok'); + return; + } + + if (req.method === 'POST' && url.pathname === '/rpc') { + const chunks: Buffer[] = []; + req.on('data', (chunk) => { + chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + }); + req.on('end', () => { + const rpcRequest = JSON.parse(Buffer.concat(chunks).toString('utf8')) as Record< + string, + any + >; + rpcRequests.push(rpcRequest); + res.writeHead(200, { 'content-type': 'application/json' }); + res.end( + JSON.stringify({ + jsonrpc: '2.0', + id: rpcRequest.id, + result: { ok: true, data: responseData }, + }), + ); + }); + return; + } + + res.writeHead(404); + res.end('not found'); + }); + const port = await listenOnLoopback(server); + return { server, port, seenPaths, rpcRequests }; +} + +/** Swaps `process.stderr.write` for a buffer until `restore`, so a test can read what was printed. */ +export function captureStderr(): { read: () => string; restore: () => void } { + const originalWrite = process.stderr.write.bind(process.stderr); + let captured = ''; + (process.stderr as { write: typeof process.stderr.write }).write = ((chunk: unknown) => { + captured += String(chunk); + return true; + }) as typeof process.stderr.write; + return { + read: () => captured, + restore: () => { + process.stderr.write = originalWrite; + }, + }; +} diff --git a/src/__tests__/update-check.test.ts b/src/__tests__/update-check.test.ts index 903bc31091..58e2baf6b7 100644 --- a/src/__tests__/update-check.test.ts +++ b/src/__tests__/update-check.test.ts @@ -87,6 +87,32 @@ test('notifier prints cached upgrade notice once for a newly discovered version' assert.equal(cache.prompted, true); }); +test('notifier treats the release as newer than the -dev build of the same base', () => { + // main carries `-dev` between releases; the shared SemVer comparator ranks the release above it, + // where numeric string collation ranked it below and never prompted. + const stateDir = makeTempStateDir(); + cleanupPaths.push(stateDir); + writeCache(stateDir, { + latestVersion: '0.12.0', + checkedAt: '2026-03-25T10:00:00.000Z', + }); + + let stderr = ''; + vi.spyOn(process.stderr, 'write').mockImplementation(((chunk: unknown) => { + stderr += String(chunk); + return true; + }) as typeof process.stderr.write); + + maybeRunUpgradeNotifier({ + command: 'devices', + currentVersion: '0.12.0-dev', + stateDir, + flags: {}, + }); + + assert.match(stderr, /Update available: agent-device 0\.12\.0-dev -> 0\.12\.0/); +}); + test('notifier skips repeat prompts after the cached version was already shown', () => { const stateDir = makeTempStateDir(); cleanupPaths.push(stateDir); diff --git a/src/cli/update-check.ts b/src/cli/update-check.ts index e430e336a2..6a1e0a7039 100644 --- a/src/cli/update-check.ts +++ b/src/cli/update-check.ts @@ -2,6 +2,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { runCmdDetached } from '@agent-device/host-kit/command'; +import { compareVersions } from '@agent-device/host-kit/version'; const PACKAGE_NAME = 'agent-device'; const UPDATE_CHECK_INTERVAL_MS = 14 * 24 * 60 * 60 * 1000; @@ -168,10 +169,6 @@ function parseTimestamp(value: string | undefined): number | undefined { return Number.isNaN(parsed) ? undefined : parsed; } -function compareVersions(left: string, right: string): number { - return left.localeCompare(right, undefined, { numeric: true }); -} - export function readUpdateCheckWorkerArgs( argv: string[], ): { cachePath: string; currentVersion: string } | null { diff --git a/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts b/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts index 810672ff6b..073f789a58 100644 --- a/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts +++ b/src/daemon-client/__tests__/daemon-client-lifecycle.test.ts @@ -28,6 +28,11 @@ import { listenOnLoopback, supportsLoopbackBind, } from '../../__tests__/test-utils/loopback.ts'; +import { + captureStderr, + startHttpDaemonFixture, + type HttpDaemonFixture, +} from '../../__tests__/test-utils/daemon-http-fixture.ts'; import { AppError } from '@agent-device/kernel/errors'; import { runCmdDetachedMonitored, runCmdSync } from '@agent-device/host-kit/command'; import { shellQuoteIfNeeded } from '@agent-device/kernel/device-shell'; @@ -46,13 +51,6 @@ type DaemonInfoFixture = { processStartTime?: string; }; -type HttpDaemonFixture = { - server: http.Server; - port: number; - seenPaths: string[]; - rpcRequests: Record[]; -}; - const mockRunCmdDetached = vi.mocked(runCmdDetachedMonitored); const mockRunCmdSync = vi.mocked(runCmdSync); const mockSleep = vi.mocked(sleep); @@ -109,51 +107,6 @@ function writeDaemonLock( ); } -async function startHttpDaemonFixture( - responseData: Record, -): Promise { - const seenPaths: string[] = []; - const rpcRequests: Record[] = []; - const server = http.createServer((req, res) => { - const url = new URL(req.url || '/', 'http://127.0.0.1'); - seenPaths.push(`${req.method ?? 'GET'} ${url.pathname}`); - - if (req.method === 'GET' && url.pathname === '/health') { - res.writeHead(200); - res.end('ok'); - return; - } - - if (req.method === 'POST' && url.pathname === '/rpc') { - const chunks: Buffer[] = []; - req.on('data', (chunk) => { - chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); - }); - req.on('end', () => { - const rpcRequest = JSON.parse(Buffer.concat(chunks).toString('utf8')) as Record< - string, - any - >; - rpcRequests.push(rpcRequest); - res.writeHead(200, { 'content-type': 'application/json' }); - res.end( - JSON.stringify({ - jsonrpc: '2.0', - id: rpcRequest.id, - result: { ok: true, data: responseData }, - }), - ); - }); - return; - } - - res.writeHead(404); - res.end('not found'); - }); - const port = await listenOnLoopback(server); - return { server, port, seenPaths, rpcRequests }; -} - /** Like `startHttpDaemonFixture`, but every RPC call returns `errorResult` as an `{ok:false}` result. */ async function startHttpDaemonErrorFixture( errorResult: Record, @@ -649,21 +602,6 @@ test('sendToDaemon replaces socket-only daemon metadata when HTTP transport is r } }); -function captureStderr(): { read: () => string; restore: () => void } { - const originalWrite = process.stderr.write.bind(process.stderr); - let captured = ''; - (process.stderr as { write: typeof process.stderr.write }).write = ((chunk: unknown) => { - captured += String(chunk); - return true; - }) as typeof process.stderr.write; - return { - read: () => captured, - restore: () => { - process.stderr.write = originalWrite; - }, - }; -} - test('sendRequest timeout cleanup uses resolved daemon paths instead of request flags', async (t) => { if (!(await supportsLoopbackBind())) { t.skip('loopback listeners are not permitted in this environment'); diff --git a/src/daemon-client/__tests__/daemon-client-newer-daemon.test.ts b/src/daemon-client/__tests__/daemon-client-newer-daemon.test.ts index 9eba3b28cf..00eb7cd32c 100644 --- a/src/daemon-client/__tests__/daemon-client-newer-daemon.test.ts +++ b/src/daemon-client/__tests__/daemon-client-newer-daemon.test.ts @@ -1,6 +1,5 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; -import http from 'node:http'; import { afterEach, test, vi } from 'vitest'; import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; @@ -13,15 +12,15 @@ vi.mock('@agent-device/host-kit/command', async (importOriginal) => ({ import { resolveDaemonPaths } from '../../daemon-resolution.ts'; import { sendToDaemon } from '../daemon-client.ts'; +import { closeLoopbackServer, supportsLoopbackBind } from '../../__tests__/test-utils/loopback.ts'; import { - closeLoopbackServer, - listenOnLoopback, - supportsLoopbackBind, -} from '../../__tests__/test-utils/loopback.ts'; + captureStderr, + startHttpDaemonFixture, +} from '../../__tests__/test-utils/daemon-http-fixture.ts'; import { AppError } from '@agent-device/kernel/errors'; import { runCmdDetachedMonitored } from '@agent-device/host-kit/command'; -// The daemon-version half of the takeover ladder (`resolveDaemonTakeoverReason`): an older CLI +// The daemon-version half of the takeover ladder (`resolveDaemonTakeover`): an older CLI // hoisted onto PATH meets the daemon a newer install started, with live sessions attached. It must // neither spawn a replacement nor kill the daemon. @@ -32,38 +31,6 @@ afterEach(() => { vi.unstubAllEnvs(); }); -/** A reachable daemon that answers `/health` and records every path it was asked for. */ -async function startHealthyDaemon(): Promise<{ - server: http.Server; - port: number; - seenPaths: string[]; -}> { - const seenPaths: string[] = []; - const server = http.createServer((req, res) => { - const url = new URL(req.url || '/', 'http://127.0.0.1'); - seenPaths.push(`${req.method ?? 'GET'} ${url.pathname}`); - res.writeHead(url.pathname === '/health' ? 200 : 404); - res.end(url.pathname === '/health' ? 'ok' : 'not found'); - }); - const port = await listenOnLoopback(server); - return { server, port, seenPaths }; -} - -function captureStderr(): { read: () => string; restore: () => void } { - const originalWrite = process.stderr.write.bind(process.stderr); - let captured = ''; - (process.stderr as { write: typeof process.stderr.write }).write = ((chunk: unknown) => { - captured += String(chunk); - return true; - }) as typeof process.stderr.write; - return { - read: () => captured, - restore: () => { - process.stderr.write = originalWrite; - }, - }; -} - test('sendToDaemon refuses to replace a reachable daemon newer than the client', async (t) => { if (!(await supportsLoopbackBind())) { t.skip('loopback listeners are not permitted in this environment'); @@ -71,7 +38,7 @@ test('sendToDaemon refuses to replace a reachable daemon newer than the client', } const stateDir = mkdtempForTestSync('agent-device-daemon-newer-refused-'); const paths = resolveDaemonPaths(stateDir); - const newerDaemon = await startHealthyDaemon(); + const newerDaemon = await startHttpDaemonFixture({ via: 'newer-daemon' }); vi.stubEnv('AGENT_DEVICE_STATE_DIR', stateDir); fs.mkdirSync(paths.baseDir, { recursive: true }); fs.writeFileSync( diff --git a/src/daemon-client/__tests__/daemon-launch-spec.test.ts b/src/daemon-client/__tests__/daemon-launch-spec.test.ts index 7714d4d284..5223ecc02b 100644 --- a/src/daemon-client/__tests__/daemon-launch-spec.test.ts +++ b/src/daemon-client/__tests__/daemon-launch-spec.test.ts @@ -2,11 +2,10 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; import { afterEach, test, vi } from 'vitest'; -import { AppError } from '@agent-device/kernel/errors'; import { computeDaemonCodeSignature } from '@agent-device/host-kit/code-signature'; import { resolveDaemonLaunchSpec, - resolveDaemonTakeoverReason, + resolveDaemonTakeover, resolveLocalDaemonCodeIdentity, } from '../daemon-launch-spec.ts'; import { isSourceCheckoutProjectRoot, readVersion } from '@agent-device/host-kit/version'; @@ -101,49 +100,34 @@ test('a source client fingerprints the source entry through the stat-validated c } }); -/** - * Which daemon a command keeps. `daemon-client-lifecycle.test.ts` pins the same - * decision end to end from a source checkout, which is what this test process runs - * in; these cases flip the two inputs that separate the trees — which tree this client - * runs from, and which tree the running daemon says it was started from (#2458). - */ test('a reachable daemon newer than the client is refused, not replaced', async () => { - const clientVersion = readVersion(); - await assert.rejects( - () => - resolveDaemonTakeoverReason(runningDaemon({ version: '999.0.0' }), true, '/tmp/state dir'), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.code, 'COMMAND_FAILED'); - assert.equal( - error.message, - `Daemon (pid 999999, v999.0.0) is newer than this client (v${clientVersion}); refusing to replace it.`, - ); - assert.equal(error.details?.daemonVersion, '999.0.0'); - assert.equal(error.details?.clientVersion, clientVersion); - assert.match( - String(error.details?.hint), - /agent-device daemon stop --state-dir '\/tmp\/state dir'/, - ); - return true; - }, - ); + assert.deepEqual(await resolveDaemonTakeover(runningDaemon({ version: '999.0.0' }), true), { + kind: 'refuseNewer', + daemonVersion: '999.0.0', + clientVersion: readVersion(), + }); }); test('an unreachable newer daemon is replaced like any version mismatch', async () => { - assert.equal( - await resolveDaemonTakeoverReason(runningDaemon({ version: '999.0.0' }), false), - `version mismatch (client v${readVersion()})`, - ); + assert.deepEqual(await resolveDaemonTakeover(runningDaemon({ version: '999.0.0' }), false), { + kind: 'replace', + reason: `version mismatch (client v${readVersion()})`, + }); }); test('a reachable daemon older than the client is replaced', async () => { - assert.equal( - await resolveDaemonTakeoverReason(runningDaemon({ version: '0.0.1' }), true), - `version mismatch (client v${readVersion()})`, - ); + assert.deepEqual(await resolveDaemonTakeover(runningDaemon({ version: '0.0.1' }), true), { + kind: 'replace', + reason: `version mismatch (client v${readVersion()})`, + }); }); +/** + * Which daemon a command keeps. `daemon-client-lifecycle.test.ts` pins the same + * decision end to end from a source checkout, which is what this test process runs + * in; these cases flip the two inputs that separate the trees — which tree this client + * runs from, and which tree the running daemon says it was started from (#2458). + */ function useClientTree(sourceCheckout: boolean): void { vi.mocked(isSourceCheckoutProjectRoot).mockReturnValue(sourceCheckout); } @@ -180,12 +164,12 @@ test('an installed client keeps an installed daemon whose code signature differs // whole of the identity either can offer, and the session on the daemon stands. useClientTree(false); - assert.equal( - await resolveDaemonTakeoverReason( + assert.deepEqual( + await resolveDaemonTakeover( runningDaemon({ codeOrigin: 'installed', codeSignature: 'some-other-install' }), true, ), - undefined, + { kind: 'reuse' }, ); }); @@ -195,22 +179,22 @@ test('an installed client replaces a daemon that reports a source checkout (#245 // fingerprint of its own to notice. useClientTree(false); - assert.equal( - await resolveDaemonTakeoverReason( + assert.deepEqual( + await resolveDaemonTakeover( runningDaemon({ codeOrigin: 'checkout', codeSignature: 'edited-checkout' }), true, ), - 'code origin mismatch (daemon checkout, client installed)', + { kind: 'replace', reason: 'code origin mismatch (daemon checkout, client installed)' }, ); }); test('an installed client replaces a daemon that predates the code origin field (#2458)', async () => { useClientTree(false); - assert.equal( - await resolveDaemonTakeoverReason(runningDaemon({ codeSignature: 'any' }), true), - 'code origin mismatch (daemon unreported, client installed)', - ); + assert.deepEqual(await resolveDaemonTakeover(runningDaemon({ codeSignature: 'any' }), true), { + kind: 'replace', + reason: 'code origin mismatch (daemon unreported, client installed)', + }); }); test('a source checkout keeps a daemon that reports the same code signature', async () => { @@ -218,12 +202,12 @@ test('a source checkout keeps a daemon that reports the same code signature', as const ownCodeSignature = await ownCheckoutCodeSignature(); assert.ok(ownCodeSignature); - assert.equal( - await resolveDaemonTakeoverReason( + assert.deepEqual( + await resolveDaemonTakeover( runningDaemon({ codeOrigin: 'checkout', codeSignature: ownCodeSignature }), true, ), - undefined, + { kind: 'reuse' }, ); }); @@ -232,12 +216,15 @@ test('a source checkout replaces a daemon whose code signature differs', async ( // notice a daemon serving code its client no longer has. useClientTree(true); - assert.equal( - await resolveDaemonTakeoverReason( + assert.deepEqual( + await resolveDaemonTakeover( runningDaemon({ codeOrigin: 'checkout', codeSignature: 'an-older-build' }), true, ), - 'code-signature mismatch', + { + kind: 'replace', + reason: 'code-signature mismatch', + }, ); }); @@ -246,12 +233,15 @@ test('a source checkout replaces a daemon that reports an installed package', as // nor disprove what an install holds, and must not run it on faith. useClientTree(true); - assert.equal( - await resolveDaemonTakeoverReason( + assert.deepEqual( + await resolveDaemonTakeover( runningDaemon({ codeOrigin: 'installed', codeSignature: 'the-published-artifact' }), true, ), - 'code origin mismatch (daemon installed, client checkout)', + { + kind: 'replace', + reason: 'code origin mismatch (daemon installed, client checkout)', + }, ); }); @@ -260,19 +250,19 @@ test('a source checkout judges a daemon that predates the code origin field by i // comparison they were reused under until now. useClientTree(true); - assert.equal( - await resolveDaemonTakeoverReason(runningDaemon({}), true), - 'code-signature mismatch', - ); + assert.deepEqual(await resolveDaemonTakeover(runningDaemon({}), true), { + kind: 'replace', + reason: 'code-signature mismatch', + }); }); test('a mismatched version replaces the daemon whichever tree the client runs from', async () => { - const expected = `version mismatch (client v${readVersion()})`; + const expected = { kind: 'replace', reason: `version mismatch (client v${readVersion()})` }; for (const sourceCheckout of [false, true]) { useClientTree(sourceCheckout); - assert.equal( - await resolveDaemonTakeoverReason( + assert.deepEqual( + await resolveDaemonTakeover( runningDaemon({ version: '0.0.0-mismatch', codeOrigin: 'installed', codeSignature: 'any' }), true, ), @@ -284,14 +274,13 @@ test('a mismatched version replaces the daemon whichever tree the client runs fr test('a reachable daemon of a matching identity survives, an unreachable one does not', async () => { useClientTree(false); - assert.equal( - await resolveDaemonTakeoverReason(runningDaemon({ codeOrigin: 'installed' }), true), - undefined, - ); - assert.equal( - await resolveDaemonTakeoverReason(runningDaemon({ codeOrigin: 'installed' }), false), - 'unreachable', - ); + assert.deepEqual(await resolveDaemonTakeover(runningDaemon({ codeOrigin: 'installed' }), true), { + kind: 'reuse', + }); + assert.deepEqual(await resolveDaemonTakeover(runningDaemon({ codeOrigin: 'installed' }), false), { + kind: 'replace', + reason: 'unreachable', + }); }); test('the tree shape is asked per call, not memoized with the signature', async () => { diff --git a/src/daemon-client/daemon-client-lifecycle.ts b/src/daemon-client/daemon-client-lifecycle.ts index 464c013de6..e76d35d5d0 100644 --- a/src/daemon-client/daemon-client-lifecycle.ts +++ b/src/daemon-client/daemon-client-lifecycle.ts @@ -19,7 +19,11 @@ import { type DaemonServerMode, type DaemonTransportPreference, } from '../daemon-resolution.ts'; -import { resolveDaemonLaunchSpec, resolveDaemonTakeoverReason } from './daemon-launch-spec.ts'; +import { + resolveDaemonLaunchSpec, + resolveDaemonTakeover, + type DaemonTakeoverDecision, +} from './daemon-launch-spec.ts'; import { PUBLIC_COMMANDS } from '@agent-device/command-registry/catalog'; import { @@ -182,14 +186,13 @@ async function readReusableLocalDaemon(settings: DaemonClientSettings): Promise< if (!existing) return null; const existingReachable = await canConnectReusableDaemon(existing, settings.transportPreference); - const takeoverReason = await resolveDaemonTakeoverReason( - existing, - existingReachable, - settings.paths.baseDir, - ); - if (!takeoverReason) return existing; + const decision = await resolveDaemonTakeover(existing, existingReachable); + if (decision.kind === 'reuse') return existing; + if (decision.kind === 'refuseNewer') { + throw newerDaemonRefusedError(existing, decision, settings.paths.baseDir); + } - emitDaemonTakeoverNotice(existing, takeoverReason, settings.paths.baseDir); + emitDaemonTakeoverNotice(existing, decision.reason, settings.paths.baseDir); await stopDaemonProcessForTakeover(existing); removeDaemonInfo(settings.paths.infoPath); return null; @@ -216,6 +219,24 @@ function isDaemonTransportUnavailableError(error: unknown): boolean { ); } +function newerDaemonRefusedError( + info: DaemonInfo, + decision: Extract, + stateDir: string, +): AppError { + const { daemonVersion, clientVersion } = decision; + return new AppError( + 'COMMAND_FAILED', + `Daemon (pid ${info.pid}, v${daemonVersion}) is newer than this client (v${clientVersion}); refusing to replace it.`, + { + daemonPid: info.pid, + daemonVersion, + clientVersion, + hint: `Use the agent-device v${daemonVersion} CLI that started it, or stop it deliberately: agent-device daemon stop --state-dir ${shellQuoteIfNeeded(stateDir)}`, + }, + ); +} + function emitDaemonTakeoverNotice(info: DaemonInfo, reason: string, stateDir: string): void { try { const identity = info.version ? `pid ${info.pid}, v${info.version}` : `pid ${info.pid}`; diff --git a/src/daemon-client/daemon-launch-spec.ts b/src/daemon-client/daemon-launch-spec.ts index 6a3305d016..a88e74f979 100644 --- a/src/daemon-client/daemon-launch-spec.ts +++ b/src/daemon-client/daemon-launch-spec.ts @@ -1,7 +1,6 @@ import fs from 'node:fs'; import path from 'node:path'; import { AppError } from '@agent-device/kernel/errors'; -import { shellQuoteIfNeeded } from '@agent-device/kernel/device-shell'; import { DAEMON_SOURCE_ENTRY, findProjectRoot, @@ -108,37 +107,39 @@ export async function resolveLocalDaemonCodeIdentity(): Promise { +): Promise { const clientVersion = readVersion(); if (info.version !== clientVersion) { if (reachable && info.version && isNewerVersion(info.version, clientVersion)) { - throw newerDaemonRefusedError(info, info.version, clientVersion, stateDir); + return { kind: 'refuseNewer', daemonVersion: info.version, clientVersion }; } - return `version mismatch (client v${clientVersion})`; + return { kind: 'replace', reason: `version mismatch (client v${clientVersion})` }; } const localIdentity = await resolveLocalDaemonCodeIdentity(); const codeMismatch = resolveCodeIdentityMismatch(localIdentity, info); - if (codeMismatch) return codeMismatch; - if (!reachable) return 'unreachable'; - return undefined; + if (codeMismatch) return { kind: 'replace', reason: codeMismatch }; + if (!reachable) return { kind: 'replace', reason: 'unreachable' }; + return { kind: 'reuse' }; } /** @@ -175,24 +176,3 @@ function describeCodeOriginMismatch( ): `code origin mismatch (${string}, client ${string})` { return `code origin mismatch (daemon ${info.codeOrigin ?? 'unreported'}, client ${local.origin})`; } - -function newerDaemonRefusedError( - info: DaemonInfo, - daemonVersion: string, - clientVersion: string, - stateDir: string | undefined, -): AppError { - const stopCommand = stateDir - ? `agent-device daemon stop --state-dir ${shellQuoteIfNeeded(stateDir)}` - : 'agent-device daemon stop'; - return new AppError( - 'COMMAND_FAILED', - `Daemon (pid ${info.pid}, v${daemonVersion}) is newer than this client (v${clientVersion}); refusing to replace it.`, - { - daemonPid: info.pid, - daemonVersion, - clientVersion, - hint: `Run the agent-device v${daemonVersion} CLI that started this daemon (an older copy was probably hoisted onto PATH), or stop it first with: ${stopCommand}.`, - }, - ); -} From 2c8e0fa5aa584d56eddcfbee5b7a242e5c5d4485 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 23 Sep 2026 17:46:03 +0200 Subject: [PATCH 5/5] fix(daemon): refuse a live newer daemon on any transport it advertises 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 --- .../daemon-client-newer-daemon.test.ts | 50 +++++++++ .../__tests__/daemon-launch-spec.test.ts | 103 +++++++++++++----- src/daemon-client/daemon-client-lifecycle.ts | 8 +- src/daemon-client/daemon-launch-spec.ts | 22 +++- 4 files changed, 149 insertions(+), 34 deletions(-) diff --git a/src/daemon-client/__tests__/daemon-client-newer-daemon.test.ts b/src/daemon-client/__tests__/daemon-client-newer-daemon.test.ts index 00eb7cd32c..4229f8cc79 100644 --- a/src/daemon-client/__tests__/daemon-client-newer-daemon.test.ts +++ b/src/daemon-client/__tests__/daemon-client-newer-daemon.test.ts @@ -82,3 +82,53 @@ test('sendToDaemon refuses to replace a reachable daemon newer than the client', fs.rmSync(stateDir, { recursive: true, force: true }); } }); + +test('sendToDaemon refuses a newer socket-only daemon when the client prefers http', async (t) => { + if (!(await supportsLoopbackBind())) { + t.skip('loopback listeners are not permitted in this environment'); + return; + } + const stateDir = mkdtempForTestSync('agent-device-daemon-newer-socket-only-'); + const paths = resolveDaemonPaths(stateDir); + const newerDaemon = await startHttpDaemonFixture({ via: 'newer-socket-daemon' }); + vi.stubEnv('AGENT_DEVICE_STATE_DIR', stateDir); + fs.mkdirSync(paths.baseDir, { recursive: true }); + fs.writeFileSync( + paths.infoPath, + `${JSON.stringify({ + token: 'local-secret', + pid: 999_999, + version: '999.0.0', + port: newerDaemon.port, + transport: 'socket', + })}\n`, + 'utf8', + ); + const stderrCapture = captureStderr(); + + try { + await assert.rejects( + () => + sendToDaemon({ + session: 'default', + command: 'newer-daemon-smoke', + positionals: [], + flags: { stateDir, daemonTransport: 'http' }, + meta: { requestId: 'req-newer-socket-daemon' }, + }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.match(error.message, /v999\.0\.0\) is newer than this client/); + return true; + }, + ); + + assert.equal(mockRunCmdDetached.mock.calls.length, 0, 'no replacement daemon is spawned'); + assert.equal(stderrCapture.read(), '', 'no takeover notice is printed'); + assert.ok(fs.existsSync(paths.infoPath), 'the newer daemon keeps its metadata'); + } finally { + stderrCapture.restore(); + await closeLoopbackServer(newerDaemon.server); + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); diff --git a/src/daemon-client/__tests__/daemon-launch-spec.test.ts b/src/daemon-client/__tests__/daemon-launch-spec.test.ts index 5223ecc02b..37af703064 100644 --- a/src/daemon-client/__tests__/daemon-launch-spec.test.ts +++ b/src/daemon-client/__tests__/daemon-launch-spec.test.ts @@ -6,6 +6,7 @@ import { computeDaemonCodeSignature } from '@agent-device/host-kit/code-signatur import { resolveDaemonLaunchSpec, resolveDaemonTakeover, + type DaemonReachability, resolveLocalDaemonCodeIdentity, } from '../daemon-launch-spec.ts'; import { isSourceCheckoutProjectRoot, readVersion } from '@agent-device/host-kit/version'; @@ -101,22 +102,47 @@ test('a source client fingerprints the source entry through the stat-validated c }); test('a reachable daemon newer than the client is refused, not replaced', async () => { - assert.deepEqual(await resolveDaemonTakeover(runningDaemon({ version: '999.0.0' }), true), { - kind: 'refuseNewer', - daemonVersion: '999.0.0', - clientVersion: readVersion(), - }); + assert.deepEqual( + await resolveDaemonTakeover(runningDaemon({ version: '999.0.0' }), reachable()), + { + kind: 'refuseNewer', + daemonVersion: '999.0.0', + clientVersion: readVersion(), + }, + ); }); test('an unreachable newer daemon is replaced like any version mismatch', async () => { - assert.deepEqual(await resolveDaemonTakeover(runningDaemon({ version: '999.0.0' }), false), { - kind: 'replace', - reason: `version mismatch (client v${readVersion()})`, - }); + assert.deepEqual( + await resolveDaemonTakeover(runningDaemon({ version: '999.0.0' }), unreachable()), + { + kind: 'replace', + reason: `version mismatch (client v${readVersion()})`, + }, + ); +}); + +test('a newer daemon alive only on a transport the client does not prefer is still refused', async () => { + assert.deepEqual( + await resolveDaemonTakeover(runningDaemon({ version: '999.0.0' }), onlyOnAnotherTransport()), + { kind: 'refuseNewer', daemonVersion: '999.0.0', clientVersion: readVersion() }, + ); +}); + +test('a same-version daemon the client transport cannot reach is replaced', async () => { + useClientTree(false); + + assert.deepEqual( + await resolveDaemonTakeover( + runningDaemon({ codeOrigin: 'installed' }), + onlyOnAnotherTransport(), + ), + { kind: 'replace', reason: 'unreachable' }, + ); }); test('a reachable daemon older than the client is replaced', async () => { - assert.deepEqual(await resolveDaemonTakeover(runningDaemon({ version: '0.0.1' }), true), { + assert.deepEqual(await resolveDaemonTakeover(runningDaemon({ version: '0.0.1' }), reachable()), { kind: 'replace', reason: `version mismatch (client v${readVersion()})`, }); @@ -132,6 +158,18 @@ function useClientTree(sourceCheckout: boolean): void { vi.mocked(isSourceCheckoutProjectRoot).mockReturnValue(sourceCheckout); } +function reachable(): DaemonReachability { + return { viaClientTransport: true, onAnyAdvertisedTransport: async () => true }; +} + +function unreachable(): DaemonReachability { + return { viaClientTransport: false, onAnyAdvertisedTransport: async () => false }; +} + +function onlyOnAnotherTransport(): DaemonReachability { + return { viaClientTransport: false, onAnyAdvertisedTransport: async () => true }; +} + function runningDaemon(info: { version?: string; codeOrigin?: 'installed' | 'checkout'; @@ -167,7 +205,7 @@ test('an installed client keeps an installed daemon whose code signature differs assert.deepEqual( await resolveDaemonTakeover( runningDaemon({ codeOrigin: 'installed', codeSignature: 'some-other-install' }), - true, + reachable(), ), { kind: 'reuse' }, ); @@ -182,7 +220,7 @@ test('an installed client replaces a daemon that reports a source checkout (#245 assert.deepEqual( await resolveDaemonTakeover( runningDaemon({ codeOrigin: 'checkout', codeSignature: 'edited-checkout' }), - true, + reachable(), ), { kind: 'replace', reason: 'code origin mismatch (daemon checkout, client installed)' }, ); @@ -191,10 +229,13 @@ test('an installed client replaces a daemon that reports a source checkout (#245 test('an installed client replaces a daemon that predates the code origin field (#2458)', async () => { useClientTree(false); - assert.deepEqual(await resolveDaemonTakeover(runningDaemon({ codeSignature: 'any' }), true), { - kind: 'replace', - reason: 'code origin mismatch (daemon unreported, client installed)', - }); + assert.deepEqual( + await resolveDaemonTakeover(runningDaemon({ codeSignature: 'any' }), reachable()), + { + kind: 'replace', + reason: 'code origin mismatch (daemon unreported, client installed)', + }, + ); }); test('a source checkout keeps a daemon that reports the same code signature', async () => { @@ -205,7 +246,7 @@ test('a source checkout keeps a daemon that reports the same code signature', as assert.deepEqual( await resolveDaemonTakeover( runningDaemon({ codeOrigin: 'checkout', codeSignature: ownCodeSignature }), - true, + reachable(), ), { kind: 'reuse' }, ); @@ -219,7 +260,7 @@ test('a source checkout replaces a daemon whose code signature differs', async ( assert.deepEqual( await resolveDaemonTakeover( runningDaemon({ codeOrigin: 'checkout', codeSignature: 'an-older-build' }), - true, + reachable(), ), { kind: 'replace', @@ -236,7 +277,7 @@ test('a source checkout replaces a daemon that reports an installed package', as assert.deepEqual( await resolveDaemonTakeover( runningDaemon({ codeOrigin: 'installed', codeSignature: 'the-published-artifact' }), - true, + reachable(), ), { kind: 'replace', @@ -250,7 +291,7 @@ test('a source checkout judges a daemon that predates the code origin field by i // comparison they were reused under until now. useClientTree(true); - assert.deepEqual(await resolveDaemonTakeover(runningDaemon({}), true), { + assert.deepEqual(await resolveDaemonTakeover(runningDaemon({}), reachable()), { kind: 'replace', reason: 'code-signature mismatch', }); @@ -264,7 +305,7 @@ test('a mismatched version replaces the daemon whichever tree the client runs fr assert.deepEqual( await resolveDaemonTakeover( runningDaemon({ version: '0.0.0-mismatch', codeOrigin: 'installed', codeSignature: 'any' }), - true, + reachable(), ), expected, ); @@ -274,13 +315,19 @@ test('a mismatched version replaces the daemon whichever tree the client runs fr test('a reachable daemon of a matching identity survives, an unreachable one does not', async () => { useClientTree(false); - assert.deepEqual(await resolveDaemonTakeover(runningDaemon({ codeOrigin: 'installed' }), true), { - kind: 'reuse', - }); - assert.deepEqual(await resolveDaemonTakeover(runningDaemon({ codeOrigin: 'installed' }), false), { - kind: 'replace', - reason: 'unreachable', - }); + assert.deepEqual( + await resolveDaemonTakeover(runningDaemon({ codeOrigin: 'installed' }), reachable()), + { + kind: 'reuse', + }, + ); + assert.deepEqual( + await resolveDaemonTakeover(runningDaemon({ codeOrigin: 'installed' }), unreachable()), + { + kind: 'replace', + reason: 'unreachable', + }, + ); }); test('the tree shape is asked per call, not memoized with the signature', async () => { diff --git a/src/daemon-client/daemon-client-lifecycle.ts b/src/daemon-client/daemon-client-lifecycle.ts index e76d35d5d0..21c728798b 100644 --- a/src/daemon-client/daemon-client-lifecycle.ts +++ b/src/daemon-client/daemon-client-lifecycle.ts @@ -185,8 +185,12 @@ async function readReusableLocalDaemon(settings: DaemonClientSettings): Promise< const existing = readDaemonInfo(settings.paths.infoPath); if (!existing) return null; - const existingReachable = await canConnectReusableDaemon(existing, settings.transportPreference); - const decision = await resolveDaemonTakeover(existing, existingReachable); + const viaClientTransport = await canConnectReusableDaemon(existing, settings.transportPreference); + const decision = await resolveDaemonTakeover(existing, { + viaClientTransport, + onAnyAdvertisedTransport: async () => + viaClientTransport || (await canConnectReusableDaemon(existing, 'auto')), + }); if (decision.kind === 'reuse') return existing; if (decision.kind === 'refuseNewer') { throw newerDaemonRefusedError(existing, decision, settings.paths.baseDir); diff --git a/src/daemon-client/daemon-launch-spec.ts b/src/daemon-client/daemon-launch-spec.ts index a88e74f979..d0cd527bb3 100644 --- a/src/daemon-client/daemon-launch-spec.ts +++ b/src/daemon-client/daemon-launch-spec.ts @@ -113,24 +113,38 @@ export type DaemonTakeoverDecision = | { kind: 'replace'; reason: string } | { kind: 'refuseNewer'; daemonVersion: string; clientVersion: string }; +/** + * Reuse needs the daemon on the transport this client will route through; refusal needs only + * proof that the daemon is alive, on any transport its metadata advertises. A client whose + * transport preference the daemon does not serve must still see a live newer daemon. + */ +export type DaemonReachability = { + viaClientTransport: boolean; + onAnyAdvertisedTransport: () => Promise; +}; + /** * One ladder decides reuse, replace, or refuse, so a daemon can never be reused and announced as * replaced, or replaced without a reason to print. The version answers first because it is cheap * and decides alone for the common pair of installed trees; the code identity * (`resolveCodeIdentityMismatch`) answers next and unreachability last. * - * A reachable daemon NEWER than this client is neither reused nor replaced: it was started by a + * A live daemon NEWER than this client is neither reused nor replaced: it was started by a * newer install that may still own live sessions, and an older binary that a package manager * hoisted onto PATH must not kill it under that install. An unreachable newer daemon is dead and * replaced like any version mismatch. */ export async function resolveDaemonTakeover( info: DaemonInfo, - reachable: boolean, + reachability: DaemonReachability, ): Promise { const clientVersion = readVersion(); if (info.version !== clientVersion) { - if (reachable && info.version && isNewerVersion(info.version, clientVersion)) { + if ( + info.version && + isNewerVersion(info.version, clientVersion) && + (await reachability.onAnyAdvertisedTransport()) + ) { return { kind: 'refuseNewer', daemonVersion: info.version, clientVersion }; } return { kind: 'replace', reason: `version mismatch (client v${clientVersion})` }; @@ -138,7 +152,7 @@ export async function resolveDaemonTakeover( const localIdentity = await resolveLocalDaemonCodeIdentity(); const codeMismatch = resolveCodeIdentityMismatch(localIdentity, info); if (codeMismatch) return { kind: 'replace', reason: codeMismatch }; - if (!reachable) return { kind: 'replace', reason: 'unreachable' }; + if (!reachability.viaClientTransport) return { kind: 'replace', reason: 'unreachable' }; return { kind: 'reuse' }; }