diff --git a/cli/__tests__/daemon-service.test.mjs b/cli/__tests__/daemon-service.test.mjs index 885f6e1c9f..5b3747d7aa 100644 --- a/cli/__tests__/daemon-service.test.mjs +++ b/cli/__tests__/daemon-service.test.mjs @@ -37,7 +37,7 @@ describe('unit content', () => { const plist = launchdPlist({ nodePath, cliPath, home }); expect(plist).toContain(`${LAUNCHD_LABEL}`); expect(plist).toContain(`${nodePath}\n\t\t${cliPath}\n\t\tdaemon\n\t\trun`); - expect(plist).toContain('KeepAlive\n\t'); + expect(plist).toContain('KeepAlive\n\t\n\t\tSuccessfulExit\n\t\t'); // Children spawn the user's claude/codex by name — homebrew must be on PATH. expect(plist).toContain('/opt/node/bin:/opt/homebrew/bin'); expect(plist).toContain(daemonLogPath(home)); @@ -47,6 +47,7 @@ describe('unit content', () => { const unit = systemdUnit({ nodePath, cliPath }); expect(unit).toContain(`ExecStart=${nodePath} ${cliPath} daemon run`); expect(unit).toContain('Restart=always'); + expect(unit).toContain('RestartPreventExitStatus=77 78'); }); // TASK-049: launchd and systemd start the daemon with a CLEAN environment, so diff --git a/cli/__tests__/daemon-supervisor.test.mjs b/cli/__tests__/daemon-supervisor.test.mjs index 5e88bc59eb..0b0a74082f 100644 --- a/cli/__tests__/daemon-supervisor.test.mjs +++ b/cli/__tests__/daemon-supervisor.test.mjs @@ -11,6 +11,8 @@ import { BACKOFF_BASE_MS, BACKOFF_MAX_MS, createDaemonSupervisor, + reconnectDelayMs, + MAX_CONCURRENT_AGENT_STARTS, } from '../src/lib/daemon-supervisor.js'; import { assertNoSandboxDeclared } from '../src/lib/adapters/pi.js'; import claude from '../src/lib/adapters/claude.js'; @@ -82,6 +84,49 @@ describe('backoff', () => { }); }); +test('reconnect delay is jittered and bounded', () => { + expect(reconnectDelayMs(1, () => 0)).toBe(1_000); + expect(reconnectDelayMs(1, () => 1)).toBe(2_000); + expect(reconnectDelayMs(20, () => 1)).toBe(60_000); +}); + +test('a revoked machine key ends supervision instead of polling forever', async () => { + const denied = Object.assign(new Error('Machine access revoked'), { status: 401 }); + const onTerminalAuthError = jest.fn(); + const client = { get: jest.fn().mockRejectedValue(denied), post: jest.fn() }; + const supervisor = createDaemonSupervisor({ + record, client, spawnChild: jest.fn(), loadToken: jest.fn(), saveToken: jest.fn(), + resolveAdapter: jest.fn(), isTerminalAuthError: (error) => error === denied, + onTerminalAuthError, + }); + await supervisor.tick(); + await supervisor.tick(); + expect(onTerminalAuthError).toHaveBeenCalledTimes(1); + expect(client.get).toHaveBeenCalledTimes(1); +}); + +test('initial and queued starts share a three-per-window limit', async () => { + const children = []; + const timers = []; + let time = 0; + const supervisor = createDaemonSupervisor({ + record, + client: { get: jest.fn(async () => ({ agents: Array.from({ length: 5 }, (_, i) => boundRow({ agentName: `seat-${i}` })) })), post: jest.fn() }, + spawnChild: jest.fn(() => { const child = makeChild(); children.push(child); return child; }), + loadToken: jest.fn((name) => ({ agentName: name, adapter: 'claude' })), + saveToken: jest.fn(), resolveAdapter: jest.fn(), + now: () => time, + setTimeoutFn: (fn, ms) => { timers.push({ fn, ms }); return timers.length; }, + }); + await supervisor.tick(); + expect(children).toHaveLength(MAX_CONCURRENT_AGENT_STARTS); + expect(timers).toHaveLength(2); + time = 5_000; + timers.forEach(({ fn }) => fn()); + expect(children).toHaveLength(5); + supervisor.stop(); +}); + describe('tick', () => { test('an owner reconnect rotates the bound local key and restarts exactly once', async () => { const tokens = { 'wren-test': { diff --git a/cli/__tests__/daemon.test.mjs b/cli/__tests__/daemon.test.mjs index eceb549471..b6bc9b8e89 100644 --- a/cli/__tests__/daemon.test.mjs +++ b/cli/__tests__/daemon.test.mjs @@ -2,6 +2,7 @@ import { jest } from '@jest/globals'; import fs from 'fs'; import os from 'os'; import path from 'path'; +import { spawnSync } from 'child_process'; const daemonTmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'commonly-daemon-test-')); @@ -16,11 +17,17 @@ await jest.unstable_mockModule('os', () => { const { daemonRecordPath, + daemonStopPath, + clearDaemonStop, + loadDaemonStop, loadDaemonRecord, removeDaemonRecord, saveDaemonRecord, + saveDaemonStop, } = await import('../src/lib/daemon-store.js'); const { + allowRevokedDaemonReplacement, + daemonStatusErrorMessage, getDaemonMachineStatus, heartbeatDaemonMachine, registerDaemonMachine, @@ -47,6 +54,34 @@ afterAll(() => { }); describe('daemon credential storage', () => { + test('a bad configuration exits 78 and a saved marker parks launchd on the next start', () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'commonly-daemon-run-')); + const entry = new URL('../src/index.js', import.meta.url).pathname; + try { + const run = () => spawnSync(process.execPath, [entry, 'daemon', 'run'], { + env: { ...process.env, HOME: home }, encoding: 'utf8', timeout: 10_000, + }); + const first = run(); + expect(first.status).toBe(78); + expect(first.stderr).toContain('No local daemon is registered'); + const marker = JSON.parse(fs.readFileSync(path.join(home, '.commonly', 'daemon', 'stopped.json'), 'utf8')); + expect(marker).toEqual({ machineDbId: 'unregistered', exitCode: 78 }); + const second = run(); + expect(second.status).toBe(process.platform === 'darwin' ? 0 : 78); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + test('persists a private terminal verdict without the bearer and clears it after registration', () => { + saveDaemonRecord(daemonRecord); + saveDaemonStop(daemonRecord, 77); + expect(loadDaemonStop(daemonRecord)).toEqual({ machineDbId: daemonRecord.machineDbId, exitCode: 77 }); + expect(fs.statSync(daemonStopPath()).mode & 0o777).toBe(0o600); + expect(fs.readFileSync(daemonStopPath(), 'utf8')).not.toContain(daemonRecord.daemonToken); + expect(loadDaemonStop({ machineDbId: 'another' })).toBeNull(); + clearDaemonStop(); + expect(loadDaemonStop(daemonRecord)).toBeNull(); + }); test('writes a private directory and 0600 bearer record', () => { saveDaemonRecord(daemonRecord); @@ -84,6 +119,77 @@ describe('daemon credential storage', () => { }); describe('daemon machine calls', () => { + const daemonError = (status, message) => Object.assign(new Error(message), { + status, + body: { message }, + }); + + test('re-registers over a revoked local record without deleting it before secure replacement', async () => { + saveDaemonRecord(daemonRecord); + const revoked = daemonError(401, 'Machine access revoked'); + await expect(allowRevokedDaemonReplacement({ + existing: daemonRecord, + client: { get: jest.fn().mockRejectedValue(revoked) }, + })).resolves.toBe('revoked'); + expect(loadDaemonRecord()).toEqual(daemonRecord); + + const replacement = { + ...daemonRecord, + machineDbId: '507f1f77bcf86cd799439022', + daemonToken: 'cm_daemon_replacement', + }; + const client = { + post: jest.fn().mockResolvedValue({ + machine: { + id: replacement.machineDbId, + machineId: replacement.machineId, + name: replacement.machineName, + }, + daemonToken: replacement.daemonToken, + }), + del: jest.fn(), + }; + await registerDaemonMachine({ + client, instanceUrl: replacement.instanceUrl, name: replacement.machineName, + }); + expect(loadDaemonRecord()).toEqual(expect.objectContaining({ + machineDbId: replacement.machineDbId, + daemonToken: replacement.daemonToken, + })); + }); + + test('does not replace a live record or one whose server status cannot be determined', async () => { + const live = { get: jest.fn().mockResolvedValue({ machine: { + id: daemonRecord.machineDbId, machineId: daemonRecord.machineId, + } }) }; + await expect(allowRevokedDaemonReplacement({ existing: daemonRecord, client: live })) + .rejects.toThrow(/already registered/i); + const unavailable = { get: jest.fn().mockRejectedValue(new Error('network unavailable')) }; + await expect(allowRevokedDaemonReplacement({ existing: daemonRecord, client: unavailable })) + .rejects.toThrow(/network unavailable/i); + const missing = { get: jest.fn().mockRejectedValue(daemonError(404, 'Machine not found')) }; + await expect(allowRevokedDaemonReplacement({ existing: daemonRecord, client: missing })) + .resolves.toBe('missing'); + }); + + test('a revoked status names the recovery command without exposing the bearer', () => { + const message = daemonStatusErrorMessage(daemonError(401, 'Machine access revoked')); + expect(message).toContain('commonly daemon register'); + expect(message).not.toContain(daemonRecord.daemonToken); + expect(daemonStatusErrorMessage(daemonError(404, 'Machine not found'))).toContain('commonly daemon register'); + }); + + test.each([ + ['HTML 404', { get: jest.fn().mockRejectedValue(daemonError(404, 'not found')) }], + ['JSON 200 without a machine', { get: jest.fn().mockResolvedValue({}) }], + ['HTML 200', { get: jest.fn().mockResolvedValue({ message: 'ok' }) }], + ['wrong machine', { get: jest.fn().mockResolvedValue({ machine: { id: 'other', machineId: 'other' } }) }], + ])('keeps the local key on ambiguous %s status', async (_label, client) => { + saveDaemonRecord(daemonRecord); + await expect(allowRevokedDaemonReplacement({ existing: daemonRecord, client })).rejects.toThrow(); + expect(loadDaemonRecord()).toEqual(daemonRecord); + }); + test('persists the one-time daemon bearer without returning it in the machine view', async () => { const client = { post: jest.fn().mockResolvedValue({ @@ -165,11 +271,11 @@ describe('daemon machine calls', () => { test('status uses the daemon-scoped self route, not an owner machine listing', async () => { const client = { get: jest.fn().mockResolvedValue({ - machine: { id: daemonRecord.machineDbId, status: 'online' }, + machine: { id: daemonRecord.machineDbId, machineId: daemonRecord.machineId, status: 'online' }, }), }; await expect(getDaemonMachineStatus({ client, record: daemonRecord })) - .resolves.toEqual({ id: daemonRecord.machineDbId, status: 'online' }); + .resolves.toEqual({ id: daemonRecord.machineDbId, machineId: daemonRecord.machineId, status: 'online' }); expect(client.get).toHaveBeenCalledWith('/api/machines/me'); }); @@ -194,13 +300,49 @@ describe('daemon machine calls', () => { test('cleans up a local record when its machine was already removed server-side', async () => { const remove = jest.fn(); - const error = Object.assign(new Error('Machine not found'), { status: 404 }); + const error = daemonError(404, 'Machine not found'); const client = { del: jest.fn().mockRejectedValue(error) }; await unregisterDaemonMachine({ client, record: daemonRecord, remove }); expect(remove).toHaveBeenCalledTimes(1); }); + + test.each([401, 403])('removes local state after owner DELETE %i only when the daemon key is revoked', async (status) => { + const remove = jest.fn(); + const client = { del: jest.fn().mockRejectedValue(daemonError(status, 'Access denied')) }; + const daemonClient = { get: jest.fn().mockRejectedValue(daemonError(401, 'Machine access revoked')) }; + + await expect(unregisterDaemonMachine({ client, daemonClient, record: daemonRecord, remove })) + .resolves.toEqual({ removedRemotely: false }); + expect(remove).toHaveBeenCalledTimes(1); + expect(daemonClient.get).toHaveBeenCalledWith('/api/machines/me'); + }); + + test.each([ + ['HTML 404', { get: jest.fn().mockRejectedValue(daemonError(404, 'not found')) }], + ['JSON 200 without a machine', { get: jest.fn().mockResolvedValue({}) }], + ['HTML 200', { get: jest.fn().mockResolvedValue({ message: 'ok' }) }], + ])('keeps the local key after owner refusal and ambiguous %s proof', async (_label, daemonClient) => { + saveDaemonRecord(daemonRecord); + const remove = jest.fn(); + const client = { del: jest.fn().mockRejectedValue(daemonError(403, 'Access denied')) }; + await expect(unregisterDaemonMachine({ client, daemonClient, record: daemonRecord, remove })) + .rejects.toThrow(); + expect(remove).not.toHaveBeenCalled(); + expect(loadDaemonRecord()).toEqual(daemonRecord); + }); + + test('keeps the local key if owner DELETE fails but the daemon key still works', async () => { + const remove = jest.fn(); + const client = { del: jest.fn().mockRejectedValue(daemonError(401, 'Unauthorized')) }; + const daemonClient = { get: jest.fn().mockResolvedValue({ machine: { + id: daemonRecord.machineDbId, machineId: daemonRecord.machineId, + } }) }; + await expect(unregisterDaemonMachine({ client, daemonClient, record: daemonRecord, remove })) + .rejects.toThrow(/Unauthorized/); + expect(remove).not.toHaveBeenCalled(); + }); }); // TASK-072: the status line is the only place a local seat's identity and its flap diff --git a/cli/package-lock.json b/cli/package-lock.json index 3a3444d4be..5e45570af7 100644 --- a/cli/package-lock.json +++ b/cli/package-lock.json @@ -1,12 +1,12 @@ { "name": "@commonlyai/cli", - "version": "0.1.142", + "version": "0.1.147", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@commonlyai/cli", - "version": "0.1.142", + "version": "0.1.147", "license": "Apache-2.0", "dependencies": { "commander": "^12.0.0" diff --git a/cli/package.json b/cli/package.json index 2b559b76b7..b4f3e410d9 100644 --- a/cli/package.json +++ b/cli/package.json @@ -1,6 +1,6 @@ { "name": "@commonlyai/cli", - "version": "0.1.142", + "version": "0.1.147", "license": "Apache-2.0", "description": "The Commonly CLI — connect agents, manage pods, iterate fast", "type": "module", diff --git a/cli/src/commands/daemon.js b/cli/src/commands/daemon.js index 746d27618b..cf64ea7663 100644 --- a/cli/src/commands/daemon.js +++ b/cli/src/commands/daemon.js @@ -13,7 +13,10 @@ import { } from 'fs'; import { createClient } from '../lib/api.js'; import { getToken, resolveInstanceUrl } from '../lib/config.js'; -import { loadDaemonRecord, removeDaemonRecord, saveDaemonRecord } from '../lib/daemon-store.js'; +import { + clearDaemonStop, loadDaemonRecord, loadDaemonStop, removeDaemonRecord, + saveDaemonRecord, saveDaemonStop, +} from '../lib/daemon-store.js'; import { createDaemonSupervisor, DEFAULT_HEARTBEAT_MS, @@ -117,21 +120,73 @@ export const heartbeatDaemonMachine = async ({ client, record }) => ( client.post(`/api/machines/${record.machineDbId}/heartbeat`) ); -export const getDaemonMachineStatus = async ({ client }) => { +export const getDaemonMachineStatus = async ({ client, record }) => { const response = await client.get('/api/machines/me'); - return response?.machine || null; + const machine = response?.machine; + if (!machine || typeof machine !== 'object' || Array.isArray(machine) + || !machine.id || !machine.machineId + || (record && (machine.id !== record.machineDbId || machine.machineId !== record.machineId))) { + throw new Error('Unexpected daemon status response; local credential kept.'); + } + return machine; +}; + +const isRevokedDaemonKey = (error) => error?.status === 401 && [ + 'Daemon token revoked or unknown', + 'Machine access revoked', +].includes(error?.body?.message); + +const isMissingDaemonMachine = (error) => error?.status === 404 + && error?.body?.message === 'Machine not found'; + +const storedDaemonState = async (client, record) => { + try { + await getDaemonMachineStatus({ client, record }); + return 'active'; + } catch (error) { + if (isRevokedDaemonKey(error)) return 'revoked'; + if (isMissingDaemonMachine(error)) return 'missing'; + throw error; + } }; -// Revoke remotely before unlinking locally. A failed network call must leave -// the bearer record intact so the operator can retry rather than orphaning a -// year-long daemon credential they can no longer address from the CLI. -export const unregisterDaemonMachine = async ({ client, record, remove = removeDaemonRecord }) => { +export const daemonStatusErrorMessage = (error) => (isRevokedDaemonKey(error) + ? "This computer's old key was revoked. Run: commonly daemon register" + : isMissingDaemonMachine(error) + ? 'This computer is no longer registered. Run: commonly daemon register' + : `Daemon status failed: ${error.message}`); + +// Keep the old record until a new registration is securely persisted. A +// network failure or a still-live daemon key must never silently mint another. +export const allowRevokedDaemonReplacement = async ({ existing, client }) => { + if (!existing) return null; + const state = await storedDaemonState(client, existing); + if (state === 'active') { + throw new Error(`A local daemon is already registered for ${existing.machineName}. Run: commonly daemon status`); + } + return state; +}; + +// Revoke remotely before unlinking locally. An owner-session 401/403 is not +// proof of revocation: check the stored daemon key before discarding it. +export const unregisterDaemonMachine = async ({ + client, daemonClient, record, remove = removeDaemonRecord, +}) => { + let removedRemotely = true; try { await client.del(`/api/machines/${record.machineDbId}`); } catch (error) { - if (error?.status !== 404) throw error; + if (isMissingDaemonMachine(error)) { + removedRemotely = false; + } else if ([401, 403].includes(error?.status) && daemonClient) { + if (await storedDaemonState(daemonClient, record) === 'active') throw error; + removedRemotely = false; + } else { + throw error; + } } remove(); + return { removedRemotely }; }; // The adapter names a binary on THIS machine — the one fact the server cannot @@ -173,7 +228,13 @@ Examples: try { const existing = loadDaemonRecord(); if (existing) { - throw new Error(`A local daemon is already registered for ${existing.machineName}. Run: commonly daemon status`); + const state = await allowRevokedDaemonReplacement({ + existing, + client: createClient({ instance: existing.instanceUrl, token: existing.daemonToken }), + }); + console.log(state === 'revoked' + ? "This computer's old key was revoked; registering it again." + : 'This computer is no longer registered; registering it again.'); } const instanceUrl = resolveInstanceUrl(opts.instance); const client = createClient({ instance: instanceUrl, token: requireUserToken(opts.instance || instanceUrl) }); @@ -182,6 +243,7 @@ Examples: instanceUrl, name: String(opts.name || hostname()).trim(), }); + clearDaemonStop(); console.log(`Registered ${machine.name}. Daemon credential stored securely.`); try { @@ -195,6 +257,9 @@ Examples: console.error('Retry with: commonly daemon heartbeat'); process.exitCode = 1; } + if (existing && existsSync(servicePaths(process.platform, homedir()).file)) { + console.log('Restart the installed daemon to use the new key: commonly daemon restart'); + } } catch (error) { console.error(`Daemon registration failed: ${error.message}`); process.exitCode = 1; @@ -211,8 +276,14 @@ Examples: instance: record.instanceUrl, token: requireUserToken(record.instanceUrl), }); - await unregisterDaemonMachine({ client, record }); - console.log(`Unregistered ${record.machineName} and removed its local daemon credential.`); + const { removedRemotely } = await unregisterDaemonMachine({ + client, + daemonClient: createClient({ instance: record.instanceUrl, token: record.daemonToken }), + record, + }); + console.log(removedRemotely + ? `Unregistered ${record.machineName} and removed its local daemon credential.` + : `Removed ${record.machineName}'s local daemon credential. Server access was already revoked or the machine was removed.`); } catch (error) { console.error(`Daemon unregister failed: ${error.message}`); process.exitCode = 1; @@ -343,8 +414,23 @@ Examples: .option('--heartbeat ', 'Heartbeat interval in ms', String(DEFAULT_HEARTBEAT_MS)) .option('--foreground', 'Keep the supervisor attached to this terminal (default for direct invocation)') .action(async (opts) => { + let record; try { - const record = requireDaemonRecord(); + const terminal = loadDaemonStop(); + if (terminal) { + console.error('Daemon stopped: no valid local registration. Run: commonly daemon register'); + process.exitCode = process.platform === 'darwin' ? 0 : terminal.exitCode; + return; + } + record = requireDaemonRecord(); + const stopped = loadDaemonStop(record); + if (stopped) { + console.error("Daemon stopped: this computer's key was revoked. Run: commonly daemon register"); + // launchd restarts the first nonzero terminal exit once; a saved + // verdict makes that second invocation successful and ends the loop. + process.exitCode = process.platform === 'darwin' ? 0 : stopped.exitCode; + return; + } const client = createClient({ instance: record.instanceUrl, token: record.daemonToken }); const logsDir = daemonLogsDir(); if (!existsSync(logsDir)) mkdirSync(logsDir, { recursive: true }); @@ -365,9 +451,34 @@ Examples: stampLog(`WARNING: ${name} is not set in this environment — a seat whose adapter needs it will fail to start. Export it and re-run: commonly daemon install (docs/agents/daemon-service-environment.md)`); } - const supervisor = createDaemonSupervisor({ + let pollTimer; + let heartbeatTimer; + let supervisor; + let terminalEnded = false; + const terminalAuth = (error) => isRevokedDaemonKey(error) + || isMissingDaemonMachine(error); + const onTerminalAuthError = () => { + terminalEnded = true; + let saved = false; + try { + saveDaemonStop(record, 77); + saved = true; + } catch (error) { + stampLog(`could not save revoked-machine stop marker: ${error.message}`); + } + stampLog("machine key revoked; stopping supervised agents. Run: commonly daemon register"); + clearInterval(pollTimer); + clearInterval(heartbeatTimer); + supervisor.stop(); + // Without the marker launchd would restart forever. A successful + // exit parks it on macOS; systemd honors 77 directly. + process.exitCode = saved || process.platform !== 'darwin' ? 77 : 0; + }; + supervisor = createDaemonSupervisor({ record, client, + isTerminalAuthError: terminalAuth, + onTerminalAuthError, // One child per agent, logging to its own file. The child is the // ordinary `commonly agent run ` — the daemon is its // supervisor, never its replacement (D6). @@ -396,9 +507,20 @@ Examples: stampLog(`daemon supervising for ${record.machineName} — poll ${opts.poll}ms, heartbeat ${opts.heartbeat}ms (ctrl+c to stop)`); await supervisor.tick(); + if (terminalEnded) return; await supervisor.heartbeat(); - const pollTimer = setInterval(() => supervisor.tick(), Number(opts.poll) || DEFAULT_POLL_MS); - const heartbeatTimer = setInterval(() => supervisor.heartbeat(), Number(opts.heartbeat) || DEFAULT_HEARTBEAT_MS); + if (terminalEnded) return; + const runtimeFailure = (error) => { + if (terminalEnded) return; + terminalEnded = true; + stampLog(`daemon runtime failed: ${error.message}`); + clearInterval(pollTimer); + clearInterval(heartbeatTimer); + supervisor.stop(); + process.exitCode = 75; + }; + pollTimer = setInterval(() => { void supervisor.tick().catch(runtimeFailure); }, Number(opts.poll) || DEFAULT_POLL_MS); + heartbeatTimer = setInterval(() => { void supervisor.heartbeat().catch(runtimeFailure); }, Number(opts.heartbeat) || DEFAULT_HEARTBEAT_MS); const shutdown = () => { stampLog('daemon stopping — terminating supervised agents'); clearInterval(pollTimer); @@ -410,7 +532,19 @@ Examples: process.on('SIGTERM', shutdown); } catch (error) { console.error(`Daemon run failed: ${error.message}`); - process.exitCode = 1; + const badConfig = !record || error.message.startsWith('Daemon credential file'); + if (badConfig) { + try { + saveDaemonStop(record || { machineDbId: 'unregistered' }, 78); + } catch (markerError) { + console.error(`Could not save daemon stop marker: ${markerError.message}`); + if (process.platform === 'darwin') { + process.exitCode = 0; + return; + } + } + } + process.exitCode = badConfig ? 78 : 75; } }); @@ -423,11 +557,8 @@ Examples: const record = requireDaemonRecord(); const machine = await getDaemonMachineStatus({ client: createClient({ instance: record.instanceUrl, token: record.daemonToken }), + record, }); - if (!machine) { - console.log(`${record.machineName}: no longer registered on the server.`); - return; - } const lastSeen = machine.lastSeenAt ? new Date(machine.lastSeenAt).toLocaleString() : 'never'; console.log(`${machine.name}: ${machine.status} (last heartbeat: ${lastSeen})`); if (opts.verbose) { @@ -442,7 +573,7 @@ Examples: } } } catch (error) { - console.error(`Daemon status failed: ${error.message}`); + console.error(daemonStatusErrorMessage(error)); process.exitCode = 1; } }); diff --git a/cli/src/lib/daemon-service.js b/cli/src/lib/daemon-service.js index 3e95be9729..36cdefdb4b 100644 --- a/cli/src/lib/daemon-service.js +++ b/cli/src/lib/daemon-service.js @@ -1,8 +1,7 @@ /** * ADR-026 D1: install-once service registration for the resident daemon. * - * macOS gets a launchd LaunchAgent (KeepAlive — launchd itself restarts a - * dead daemon), Linux a systemd user unit (Restart=always). Both run the + * macOS gets a launchd LaunchAgent, Linux a systemd user unit. Both run the * exact interpreter + CLI entry that executed `daemon install`, resolved at * install time — a PATH lookup at boot would race version managers and * silently run a different install than the one the user tested. @@ -165,7 +164,10 @@ ${plistEnvPairs(providerEnv)}\t \tRunAtLoad \t \tKeepAlive -\t +\t +\t\tSuccessfulExit +\t\t +\t \tStandardOutPath \t${xmlEscape(daemonLogPath(home))} \tStandardErrorPath @@ -185,6 +187,7 @@ After=network-online.target [Service] ExecStart=${nodePath} ${cliPath} daemon run --foreground Restart=always +RestartPreventExitStatus=77 78 RestartSec=5 Environment=PATH=${childPath(nodePath)}${providerLines} diff --git a/cli/src/lib/daemon-store.js b/cli/src/lib/daemon-store.js index 1adc2de86f..a06212287d 100644 --- a/cli/src/lib/daemon-store.js +++ b/cli/src/lib/daemon-store.js @@ -21,6 +21,35 @@ import { join } from 'path'; const daemonDir = () => join(homedir(), '.commonly', 'daemon'); export const daemonRecordPath = () => join(daemonDir(), 'machine.json'); +export const daemonStopPath = () => join(daemonDir(), 'stopped.json'); + +// A terminal machine-key verdict survives launchd's one unavoidable restart. +// It contains no bearer; registration clears it only after the new key is safe. +export const loadDaemonStop = (record = { machineDbId: 'unregistered' }) => { + const path = daemonStopPath(); + if (!existsSync(path)) return null; + const value = JSON.parse(readFileSync(path, 'utf8')); + return value.machineDbId === record.machineDbId ? value : null; +}; + +export const saveDaemonStop = (record, exitCode) => { + const dir = daemonDir(); + if (!existsSync(dir)) mkdirSync(dir, { recursive: true, mode: 0o700 }); + chmodSync(dir, 0o700); + const path = daemonStopPath(); + const tmp = `${path}.${process.pid}.tmp`; + try { + writeFileSync(tmp, `${JSON.stringify({ machineDbId: record.machineDbId, exitCode })}\n`, { mode: 0o600, flag: 'wx' }); + renameSync(tmp, path); + } catch (error) { + if (existsSync(tmp)) unlinkSync(tmp); + throw error; + } +}; + +export const clearDaemonStop = () => { + if (existsSync(daemonStopPath())) unlinkSync(daemonStopPath()); +}; const validateRecord = (record) => { if (!record diff --git a/cli/src/lib/daemon-supervisor.js b/cli/src/lib/daemon-supervisor.js index 7553ed7cd0..d3d55314bd 100644 --- a/cli/src/lib/daemon-supervisor.js +++ b/cli/src/lib/daemon-supervisor.js @@ -33,6 +33,14 @@ export const DEFAULT_POLL_MS = 30_000; export const DEFAULT_HEARTBEAT_MS = 30_000; export const BACKOFF_BASE_MS = 5_000; export const BACKOFF_MAX_MS = 60_000; +export const MAX_CONCURRENT_AGENT_STARTS = 3; +export const AGENT_START_WINDOW_MS = 5_000; +export const RECONNECT_MAX_MS = 60_000; + +export const reconnectDelayMs = (failures, random = Math.random) => { + const ceiling = Math.min(RECONNECT_MAX_MS, 1_000 * 2 ** Math.min(failures, 6)); + return Math.floor(1_000 + random() * (ceiling - 1_000)); +}; const workspacePathFor = (environment) => { const declared = environment?.workspace?.path; @@ -59,12 +67,35 @@ export const createDaemonSupervisor = ({ resolveAdapter, // async (runtime) => adapter name for THIS machine persistState = () => {}, // (agentStates) => void; must not persist secrets log = () => {}, + isTerminalAuthError = () => false, + onTerminalAuthError = () => {}, + now = Date.now, + random = Math.random, setTimeoutFn = setTimeout, clearTimeoutFn = clearTimeout, }) => { // key → { agentName, instanceId, child, state, restarts, backoffTimer, desired } const seats = new Map(); let stopped = false; + let reconnectFailures = 0; + let reconnectAfter = 0; + const recentStarts = []; + + const connectionFailure = (error, label) => { + if (isTerminalAuthError(error)) { + stopped = true; + onTerminalAuthError(error); + return; + } + reconnectFailures += 1; + reconnectAfter = now() + reconnectDelayMs(reconnectFailures, random); + log(`${label}: ${error.message}; reconnect after ${Math.round((reconnectAfter - now()) / 1000)}s`); + }; + + const connectionRestored = () => { + reconnectFailures = 0; + reconnectAfter = 0; + }; const persist = () => { try { @@ -90,7 +121,19 @@ export const createDaemonSupervisor = ({ })); const startChild = (seat) => { - if (stopped || !seat.desired || seat.child) return; + if (stopped || !seat.desired || seat.child || seat.startDelayTimer) return; + const at = now(); + while (recentStarts.length && recentStarts[0] + AGENT_START_WINDOW_MS <= at) { + recentStarts.shift(); + } + if (recentStarts.length >= MAX_CONCURRENT_AGENT_STARTS) { + seat.startDelayTimer = setTimeoutFn(() => { + seat.startDelayTimer = null; + startChild(seat); + }, Math.max(1, recentStarts[0] + AGENT_START_WINDOW_MS - at)); + return; + } + recentStarts.push(at); seat.child = spawnChild(seat.agentName); seat.state = 'running'; seat.pid = Number.isInteger(seat.child?.pid) ? seat.child.pid : null; @@ -121,6 +164,10 @@ export const createDaemonSupervisor = ({ const stopSeat = (seat) => { seat.desired = false; + if (seat.startDelayTimer) { + clearTimeoutFn(seat.startDelayTimer); + seat.startDelayTimer = null; + } if (seat.backoffTimer) { clearTimeoutFn(seat.backoffTimer); seat.backoffTimer = null; @@ -411,14 +458,15 @@ export const createDaemonSupervisor = ({ }; const tick = async () => { - if (stopped) return; + if (stopped || now() < reconnectAfter) return; let assigned; try { assigned = await client.get('/api/agent-binding/assigned'); } catch (error) { - log(`work-list fetch failed: ${error.message}`); + connectionFailure(error, 'work-list fetch failed'); return; } + connectionRestored(); const rows = Array.isArray(assigned?.agents) ? assigned.agents : []; const bound = []; @@ -453,6 +501,7 @@ export const createDaemonSupervisor = ({ state: 'stopped', restarts: 0, backoffTimer: null, + startDelayTimer: null, desired: true, adapter: null, model: null, @@ -501,11 +550,12 @@ export const createDaemonSupervisor = ({ }; const heartbeat = async () => { - if (stopped) return; + if (stopped || now() < reconnectAfter) return; try { await client.post(`/api/machines/${record.machineDbId}/heartbeat`, { agents: agentStates() }); + connectionRestored(); } catch (error) { - log(`heartbeat failed: ${error.message}`); + connectionFailure(error, 'heartbeat failed'); } }; diff --git a/docs/cli/README.md b/docs/cli/README.md index 1f3d56d6fb..f088fd751f 100644 --- a/docs/cli/README.md +++ b/docs/cli/README.md @@ -182,8 +182,14 @@ The two `list` modes answer different questions — backend mode is "who is inst | `commonly daemon register --name [--instance ]` | Register this laptop and securely store its machine-scoped daemon credential. | | `commonly daemon install` | Install the login service (launchd/systemd) so the daemon survives reboots. | | `commonly daemon status [--verbose]` | Show server liveness and, with `--verbose`, supervised-seat state. | +| `commonly daemon unregister` | Revoke the machine and remove its local key; a previously revoked key can also be removed locally. | | `commonly daemon logs [--seat ] [--follow]` | Read daemon or per-seat logs. | +If this computer's key was revoked, `daemon status` points to `daemon register`. +Registration replaces the old local record only after the server confirms that +key cannot be used. Restart an installed daemon service with `commonly daemon +restart` so it reads the new key. + Registration and installation do not replace agent installation. For a persistent seat, use the web app's Bring your own agent → On my computer flow to place that seat on this computer; the daemon then adopts the server-marked