diff --git a/src/cli.ts b/src/cli.ts index d01beb5..00d9cbb 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1,5 +1,6 @@ #!/usr/bin/env node +import chalk from 'chalk'; import { Command } from 'commander'; import { registerDaemon } from './commands/daemon-cmd.js'; import { registerMcp } from './commands/mcp.js'; @@ -9,6 +10,7 @@ import { registerStatus } from './commands/status.js'; import { registerUpdate } from './commands/update.js'; import { registerVault } from './commands/vault.js'; import { disableDaemon } from './lib/daemon-pref.js'; +import { refreshUpdateCache, updateHint } from './lib/update-cache.js'; import { VERSION } from './utils/version.js'; const program = new Command(); @@ -23,6 +25,20 @@ program.hook('preAction', () => { if (program.opts().daemon === false) disableDaemon(); }); +// After any command (except the update-aware ones, which report richer state themselves), print a +// one-line hint from the cache, then kick a fire-and-forget refresh. Never awaited, never throws - +// the command output is already done and the cache-only read keeps this instant. The hint is +// suppressed under --json so it never corrupts machine-readable output. +program.hook('postAction', (_thisCommand, actionCommand) => { + const name = actionCommand.name(); + if (name === 'status' || name === 'update') return; + if (!actionCommand.opts()['json']) { + const hint = updateHint(); + if (hint) console.log(chalk.dim(hint)); + } + void refreshUpdateCache(); +}); + registerSetup(program); registerStatus(program); registerVault(program); diff --git a/src/commands/update.test.ts b/src/commands/update.test.ts index 4c34fcd..3c12736 100644 --- a/src/commands/update.test.ts +++ b/src/commands/update.test.ts @@ -1,24 +1,30 @@ import { describe, expect, it, vi } from 'vitest'; import type { UpdateInfo } from '../lib/update-check.js'; -import { runUpdate, type UpdateDeps } from './update.js'; +import { restartDaemonIfRunning, runUpdate, type UpdateDeps } from './update.js'; -const makeDeps = (status: UpdateInfo['status']) => { +const makeDeps = (status: UpdateInfo['status'], over: Partial = {}) => { const logs: string[] = []; - const install = vi.fn(async () => {}); + const install = over.install ?? vi.fn(async () => {}); + const restartDaemon = over.restartDaemon ?? vi.fn(async () => 'not-running' as const); + const releaseLock = over.releaseLock ?? vi.fn(() => {}); const deps: UpdateDeps = { check: async () => ({ status, message: null }), install, + restartDaemon, + acquireLock: over.acquireLock ?? (() => true), + releaseLock, log: (m) => logs.push(m), }; - return { deps, logs, install }; + return { deps, logs, install, restartDaemon, releaseLock }; }; describe('update', () => { - it('installs when an update is available', async () => { + it('installs (and releases the lock) when an update is available', async () => { const h = makeDeps({ kind: 'update-available', latest: '0.0.9' }); await runUpdate({}, h.deps); expect(h.install).toHaveBeenCalledOnce(); expect(h.logs.join()).toContain('Updated'); + expect(h.releaseLock).toHaveBeenCalledOnce(); }); it('installs when the running version is unsupported', async () => { @@ -46,4 +52,82 @@ describe('update', () => { expect(h.install).not.toHaveBeenCalled(); expect(h.logs.join()).toContain('Update available'); }); + + it('announces the restart after installing when the daemon was running', async () => { + const h = makeDeps( + { kind: 'update-available', latest: '0.0.9' }, + { restartDaemon: vi.fn(async () => 'restarted' as const) } + ); + await runUpdate({}, h.deps); + expect(h.restartDaemon).toHaveBeenCalledOnce(); + expect(h.logs.join()).toContain('Restarted the daemon'); + }); + + it('reports honestly when the daemon did not come back up', async () => { + const h = makeDeps( + { kind: 'update-available', latest: '0.0.9' }, + { restartDaemon: vi.fn(async () => 'failed' as const) } + ); + await runUpdate({}, h.deps); + expect(h.logs.join()).not.toContain('Restarted the daemon'); + expect(h.logs.join()).toContain('did not restart cleanly'); + }); + + it('does not announce a restart when the daemon was not running', async () => { + const h = makeDeps({ kind: 'update-available', latest: '0.0.9' }); + await runUpdate({}, h.deps); + expect(h.restartDaemon).toHaveBeenCalledOnce(); + expect(h.logs.join()).not.toContain('Restarted the daemon'); + }); + + it('refuses to install when another update holds the lock', async () => { + const h = makeDeps({ kind: 'update-available', latest: '0.0.9' }, { acquireLock: () => false }); + await runUpdate({}, h.deps); + expect(h.install).not.toHaveBeenCalled(); + expect(h.logs.join()).toContain('in progress'); + expect(h.releaseLock).not.toHaveBeenCalled(); + }); + + it('releases the lock when the install throws (finally path)', async () => { + const h = makeDeps( + { kind: 'update-available', latest: '0.0.9' }, + { + install: vi.fn(async () => { + throw new Error('npm exploded'); + }), + } + ); + await expect(runUpdate({}, h.deps)).rejects.toThrow('npm exploded'); + expect(h.releaseLock).toHaveBeenCalledOnce(); + }); +}); + +describe('restartDaemonIfRunning', () => { + it('stops the old daemon BEFORE starting the new one', async () => { + const order: string[] = []; + const stop = vi.fn(async () => { + order.push('stop'); + return true; + }); + const start = vi.fn(async () => { + order.push('start'); + return true; + }); + expect(await restartDaemonIfRunning({ running: () => true, stop, start })).toBe('restarted'); + expect(order).toEqual(['stop', 'start']); + }); + + it('returns failed when the new daemon does not come up', async () => { + const stop = vi.fn(async () => true); + const start = vi.fn(async () => false); + expect(await restartDaemonIfRunning({ running: () => true, stop, start })).toBe('failed'); + }); + + it('is a no-op when the daemon is not running', async () => { + const stop = vi.fn(async () => true); + const start = vi.fn(async () => true); + expect(await restartDaemonIfRunning({ running: () => false, stop, start })).toBe('not-running'); + expect(stop).not.toHaveBeenCalled(); + expect(start).not.toHaveBeenCalled(); + }); }); diff --git a/src/commands/update.ts b/src/commands/update.ts index 4fed056..943f1e5 100644 --- a/src/commands/update.ts +++ b/src/commands/update.ts @@ -2,15 +2,39 @@ import { execFile } from 'node:child_process'; import { promisify } from 'node:util'; import chalk from 'chalk'; import { type Command } from 'commander'; -import { links, siteFqdn } from '../lib/origins.js'; +import { isDaemonRunning, resolvePort, stopDaemonAndWait } from '../daemon/lifecycle.js'; +import { spawnDaemon } from '../lib/daemon-client.js'; import { checkForUpdate, INSTALL_HINT, type UpdateInfo } from '../lib/update-check.js'; +import { acquireUpdateLock, releaseUpdateLock } from '../lib/update-lock.js'; import { VERSION } from '../utils/version.js'; const pexec = promisify(execFile); +export type RestartOutcome = 'restarted' | 'failed' | 'not-running'; + +export interface RestartDeps { + running?: () => boolean; + stop?: () => Promise; + start?: (port: number) => Promise; +} + +// Restart a running daemon so it picks up the freshly installed binary; a stopped daemon is left +// stopped. Waits for the old process to exit (port free) before spawning, and reports honestly +// when the new daemon did not come up. +export const restartDaemonIfRunning = async (deps: RestartDeps = {}): Promise => { + const running = deps.running ?? isDaemonRunning; + if (!running()) return 'not-running'; + await (deps.stop ?? stopDaemonAndWait)(); + const up = await (deps.start ?? spawnDaemon)(resolvePort()); + return up ? 'restarted' : 'failed'; +}; + export interface UpdateDeps { - check: (apiUrl: string, installed: string) => Promise; + check: (installed: string) => Promise; install: () => Promise; + restartDaemon: () => Promise; + acquireLock: () => boolean; + releaseLock: () => void; log: (msg: string) => void; } @@ -19,6 +43,9 @@ const defaultDeps: UpdateDeps = { install: async () => { await pexec('npm', ['install', '-g', '@agentage/cli@latest']); }, + restartDaemon: () => restartDaemonIfRunning(), + acquireLock: () => acquireUpdateLock(), + releaseLock: releaseUpdateLock, log: (msg) => console.log(msg), }; @@ -42,19 +69,37 @@ const describe = (info: UpdateInfo): string => { } }; -// --check reports the verdict and never installs. Otherwise, install only when the -// registry says we're behind (available/unsupported); current/unreachable just report. +// Install, then restart a running daemon so its in-process engine runs the new version. +const install = async (deps: UpdateDeps): Promise => { + deps.log('Installing the latest @agentage/cli...'); + await deps.install(); + deps.log(chalk.green('Updated. Run `agentage status` to confirm.')); + const outcome = await deps.restartDaemon(); + if (outcome === 'restarted') deps.log(chalk.green('Restarted the daemon on the new version.')); + if (outcome === 'failed') + deps.log(chalk.yellow('Daemon did not restart cleanly - run `agentage daemon start`.')); +}; + +// --check reports the verdict and never installs. Otherwise, install only when the registry says +// we're behind (available/unsupported), guarded by a single-writer lock; current/unreachable just +// report. export const runUpdate = async ( opts: UpdateOptions, deps: UpdateDeps = defaultDeps ): Promise => { - const info = await deps.check(links(siteFqdn()).api, VERSION); + const info = await deps.check(VERSION); deps.log(describe(info)); const behind = info.status.kind === 'update-available' || info.status.kind === 'unsupported'; if (opts.check || !behind) return; - deps.log('Installing the latest @agentage/cli...'); - await deps.install(); - deps.log(chalk.green('Updated. Run `agentage status` to confirm.')); + if (!deps.acquireLock()) { + deps.log(chalk.yellow('Another update is already in progress; skipping.')); + return; + } + try { + await install(deps); + } finally { + deps.releaseLock(); + } }; export const registerUpdate = (program: Command): void => { diff --git a/src/daemon/lifecycle.test.ts b/src/daemon/lifecycle.test.ts index 8e5dab8..3cc232b 100644 --- a/src/daemon/lifecycle.test.ts +++ b/src/daemon/lifecycle.test.ts @@ -2,6 +2,7 @@ import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { spawn } from 'node:child_process'; import { DEFAULT_DAEMON_PORT, isDaemonRunning, @@ -10,6 +11,7 @@ import { removePortFile, resolvePort, stopDaemon, + stopDaemonAndWait, writePidFile, writePortFile, } from './lifecycle.js'; @@ -101,3 +103,20 @@ describe('stopDaemon', () => { expect(isDaemonRunning()).toBe(false); }); }); + +describe('stopDaemonAndWait', () => { + it('returns true immediately when nothing is running', async () => { + expect(await stopDaemonAndWait()).toBe(true); + writePidFile(DEAD_PID); + expect(await stopDaemonAndWait()).toBe(true); + }); + + it('signals a real process and waits for it to be gone', async () => { + const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { + stdio: 'ignore', + }); + writePidFile(child.pid!); + expect(await stopDaemonAndWait(5000)).toBe(true); + expect(isProcessAlive(child.pid!)).toBe(false); + }); +}); diff --git a/src/daemon/lifecycle.ts b/src/daemon/lifecycle.ts index ce8f7f6..a122703 100644 --- a/src/daemon/lifecycle.ts +++ b/src/daemon/lifecycle.ts @@ -69,3 +69,17 @@ export const stopDaemon = (): boolean => { removePortFile(); return alive; }; + +// Stop, then wait (bounded) for the old process to actually exit so a restart can rebind the +// port without an EADDRINUSE window. Returns whether the process is confirmed gone. +export const stopDaemonAndWait = async (timeoutMs = 2000): Promise => { + const pid = readPid(); + const signalled = stopDaemon(); + if (!signalled || pid === null) return true; + const deadline = Date.now() + timeoutMs; + while (isProcessAlive(pid)) { + if (Date.now() >= deadline) return false; + await new Promise((r) => setTimeout(r, 50)); + } + return true; +}; diff --git a/src/lib/config.test.ts b/src/lib/config.test.ts index 5b80e0b..effca78 100644 --- a/src/lib/config.test.ts +++ b/src/lib/config.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; @@ -42,6 +42,14 @@ describe('config store', () => { expect(mode).toBe(0o600); }); + it('writes atomically, leaving no temp file behind', () => { + saveAuth(sample); + saveAuth({ ...sample, clientId: 'client-2' }); + expect(existsSync(join(getConfigDir(), 'auth.json.tmp'))).toBe(false); + expect(readAuth()?.clientId).toBe('client-2'); + expect(statSync(join(getConfigDir(), 'auth.json')).mode & 0o777).toBe(0o600); + }); + it('returns null for corrupt auth files', () => { ensureConfigDir(); writeFileSync(join(getConfigDir(), 'auth.json'), 'not json'); diff --git a/src/lib/config.ts b/src/lib/config.ts index da7bc94..ca8df37 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -1,4 +1,12 @@ -import { chmodSync, existsSync, mkdirSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs'; +import { + chmodSync, + existsSync, + mkdirSync, + readFileSync, + renameSync, + unlinkSync, + writeFileSync, +} from 'node:fs'; import { homedir } from 'node:os'; import { join } from 'node:path'; @@ -36,11 +44,15 @@ export const readAuth = (): AuthState | null => { } }; +// Atomic 0600 write: a token file is never left half-written or briefly world-readable. Write a +// sibling temp, chmod it 0600 BEFORE the rename, then rename over the target in one step. export const saveAuth = (state: AuthState): void => { ensureConfigDir(); const path = authPath(); - writeFileSync(path, JSON.stringify(state, null, 2) + '\n', { encoding: 'utf-8', mode: 0o600 }); - chmodSync(path, 0o600); + const tmp = `${path}.tmp`; + writeFileSync(tmp, JSON.stringify(state, null, 2) + '\n', { encoding: 'utf-8', mode: 0o600 }); + chmodSync(tmp, 0o600); + renameSync(tmp, path); }; export const deleteAuth = (): void => { diff --git a/src/lib/status-info.test.ts b/src/lib/status-info.test.ts index 81b4a78..bffbca5 100644 --- a/src/lib/status-info.test.ts +++ b/src/lib/status-info.test.ts @@ -12,10 +12,12 @@ const jsonResponse = (status: number, body: unknown): Response => new Response(JSON.stringify(body), { status, headers: { 'content-type': 'application/json' } }); const stubFetch = (routes: Record Response>): void => { + // The update check now reads the npm registry; default it to "current" so it never adds noise. + const all = { 'cli/latest': () => jsonResponse(200, { version: '0.0.0' }), ...routes }; vi.stubGlobal( 'fetch', vi.fn((url: string) => { - for (const [suffix, response] of Object.entries(routes)) { + for (const [suffix, response] of Object.entries(all)) { if (url.endsWith(suffix)) return Promise.resolve(response()); } return Promise.reject(new Error(`unmatched url: ${url}`)); diff --git a/src/lib/status-info.ts b/src/lib/status-info.ts index 8f1f78f..22f3e0d 100644 --- a/src/lib/status-info.ts +++ b/src/lib/status-info.ts @@ -24,11 +24,11 @@ const checkEndpoint = async (apiUrl: string): Promise => { export const gatherStatus = async (auth: AuthState | null, fqdn: string): Promise => { const target = links(fqdn); - // Reachability + update check share the api base and don't depend on each other - run - // them together so `status` stays snappy. + // Endpoint reachability and the (npm-registry) update check are independent - run them + // together so `status` stays snappy. const [reachable, update] = await Promise.all([ checkEndpoint(target.api), - checkForUpdate(target.api, VERSION), + checkForUpdate(VERSION), ]); const report: StatusReport = { version: VERSION, diff --git a/src/lib/update-cache.test.ts b/src/lib/update-cache.test.ts new file mode 100644 index 0000000..8492714 --- /dev/null +++ b/src/lib/update-cache.test.ts @@ -0,0 +1,127 @@ +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { getConfigDir } from './config.js'; +import { + fetchLatestVersion, + refreshUpdateCache, + updateHint, + type UpdateCache, +} from './update-cache.js'; +import { VERSION } from '../utils/version.js'; + +const cacheFile = (): string => join(getConfigDir(), 'update-check.json'); + +const writeCache = (c: UpdateCache): void => { + mkdirSync(getConfigDir(), { recursive: true }); + writeFileSync(cacheFile(), JSON.stringify(c), 'utf-8'); +}; + +describe('update cache', () => { + let dir: string; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'agentage-updcache-')); + process.env['AGENTAGE_CONFIG_DIR'] = join(dir, 'cfg'); + }); + + afterEach(() => { + delete process.env['AGENTAGE_CONFIG_DIR']; + rmSync(dir, { recursive: true, force: true }); + }); + + describe('updateHint', () => { + it('prints a hint only when the cached latest is newer', () => { + writeCache({ checkedAt: Date.now(), latest: '99.0.0' }); + expect(updateHint()).toContain('99.0.0'); + expect(updateHint()).toContain('npm i -g @agentage/cli'); + }); + + it('is silent when the cached latest is not newer', () => { + writeCache({ checkedAt: Date.now(), latest: VERSION }); + expect(updateHint()).toBeNull(); + }); + + it('is silent when there is no cache or no known version', () => { + expect(updateHint()).toBeNull(); + writeCache({ checkedAt: Date.now(), latest: null }); + expect(updateHint()).toBeNull(); + }); + }); + + describe('refreshUpdateCache', () => { + const fresh: UpdateCache = { checkedAt: 1_000_000, latest: '0.0.1' }; + + it('does not fetch when the cache is fresh (TTL respected)', async () => { + const fetch = vi.fn(async () => '9.9.9'); + const write = vi.fn(); + await refreshUpdateCache({ now: () => 1_000_000 + 60_000, read: () => fresh, fetch, write }); + expect(fetch).not.toHaveBeenCalled(); + expect(write).not.toHaveBeenCalled(); + }); + + it('refreshes when the cache is stale', async () => { + const now = 1_000_000 + 2 * 60 * 60 * 1000; // 2h later + const fetch = vi.fn(async () => '9.9.9'); + const write = vi.fn(); + await refreshUpdateCache({ now: () => now, read: () => fresh, fetch, write }); + expect(fetch).toHaveBeenCalledOnce(); + expect(write).toHaveBeenCalledWith('9.9.9', now); + }); + + it('refreshes when there is no cache', async () => { + const fetch = vi.fn(async () => '9.9.9'); + const write = vi.fn(); + await refreshUpdateCache({ now: () => 5, read: () => null, fetch, write }); + expect(fetch).toHaveBeenCalledOnce(); + expect(write).toHaveBeenCalledWith('9.9.9', 5); + }); + + it('a failed fetch still advances checkedAt, keeping the prior version', async () => { + const now = 1_000_000 + 2 * 60 * 60 * 1000; + const write = vi.fn(); + await refreshUpdateCache({ + now: () => now, + read: () => fresh, + fetch: async () => { + throw new Error('net'); + }, + write, + }); + expect(write).toHaveBeenCalledWith('0.0.1', now); // prior latest kept, retry throttled + }); + + it('a failed first-ever fetch advances checkedAt with a null version', async () => { + const write = vi.fn(); + await refreshUpdateCache({ now: () => 5, read: () => null, fetch: async () => null, write }); + expect(write).toHaveBeenCalledWith(null, 5); + }); + + it('persists a fetched version to the cache file end to end', async () => { + await refreshUpdateCache({ read: () => null, fetch: async () => '9.9.9' }); + const cache = JSON.parse(readFileSync(cacheFile(), 'utf-8')) as UpdateCache; + expect(cache.latest).toBe('9.9.9'); + expect(typeof cache.checkedAt).toBe('number'); + expect(updateHint()).toContain('9.9.9'); + }); + + it('an on-disk failure record throttles the next refresh (offline retries hourly)', async () => { + await refreshUpdateCache({ now: () => 7, fetch: async () => null }); + const fetch = vi.fn(async () => '9.9.9'); + await refreshUpdateCache({ now: () => 7 + 60_000, fetch }); + expect(fetch).not.toHaveBeenCalled(); + }); + }); + + describe('fetchLatestVersion', () => { + it('resolves null within the timeout bound on an unreachable host', async () => { + const started = Date.now(); + // 192.0.2.1 (TEST-NET-1) either blackholes (the destroy timer fires) or refuses fast. + const result = await fetchLatestVersion(500, 'https://192.0.2.1/x'); + expect(result).toBeNull(); + // Generous CI bound (timers stretch under load); still well under undici's ~10s floor. + expect(Date.now() - started).toBeLessThan(8000); + }); + }); +}); diff --git a/src/lib/update-cache.ts b/src/lib/update-cache.ts new file mode 100644 index 0000000..8be5ba1 --- /dev/null +++ b/src/lib/update-cache.ts @@ -0,0 +1,100 @@ +import { readFileSync, writeFileSync } from 'node:fs'; +import { get } from 'node:https'; +import { join } from 'node:path'; +import { ensureConfigDir, getConfigDir } from './config.js'; +import { compareVersions, INSTALL_HINT, REGISTRY_URL } from './update-check.js'; +import { VERSION } from '../utils/version.js'; + +const CACHE_FILE = 'update-check.json'; +const TTL_MS = 60 * 60 * 1000; // passive checks refresh at most once an hour +const BG_TIMEOUT_MS = 3000; // hard cap on the whole background attempt, connect included + +export interface UpdateCache { + checkedAt: number; // epoch ms of the last attempt, success or failure (throttles retries) + latest: string | null; // latest published version, or null when never fetched +} + +const cachePath = (): string => join(getConfigDir(), CACHE_FILE); + +export const readUpdateCache = (): UpdateCache | null => { + try { + const c = JSON.parse(readFileSync(cachePath(), 'utf-8')) as UpdateCache; + return typeof c.checkedAt === 'number' ? c : null; + } catch { + return null; + } +}; + +const writeUpdateCache = (latest: string | null, now: number): void => { + ensureConfigDir(); + writeFileSync(cachePath(), JSON.stringify({ checkedAt: now, latest }) + '\n', 'utf-8'); +}; + +// The one dim hint line printed after a command, read purely from the cache (never the network), +// only when the cached latest is newer than us. null = print nothing. +export const updateHint = (): string | null => { + const c = readUpdateCache(); + if (!c?.latest) return null; + return compareVersions(VERSION, c.latest) < 0 + ? `update available: ${c.latest} -> ${INSTALL_HINT}` + : null; +}; + +// node:https instead of global fetch: undici keeps a ref'd ~10s connect timer alive even after +// its AbortSignal fires, stalling process exit on an unreachable network. req.destroy() from an +// unref'd timer caps the whole attempt and frees the event loop the moment it settles. +export const fetchLatestVersion = ( + timeoutMs: number, + url: string = REGISTRY_URL +): Promise => + new Promise((resolve) => { + const req = get(url, { headers: { Accept: 'application/json' } }, (res) => { + let body = ''; + res.setEncoding('utf8'); + res.on('data', (chunk: string) => { + body += chunk; + }); + res.on('end', () => { + if (res.statusCode !== 200) return resolve(null); + try { + const v = (JSON.parse(body) as { version?: unknown }).version; + resolve(typeof v === 'string' ? v : null); + } catch { + resolve(null); + } + }); + }); + const timer = setTimeout(() => req.destroy(), timeoutMs); + timer.unref(); + req.once('error', () => resolve(null)); + req.once('close', () => { + clearTimeout(timer); + resolve(null); // no-op when already resolved + }); + }); + +export interface RefreshDeps { + now?: () => number; + read?: () => UpdateCache | null; + fetch?: () => Promise; + write?: (latest: string | null, now: number) => void; +} + +// Fire-and-forget refresh, throttled by the TTL. checkedAt advances on FAILURE too (keeping any +// previously known version), so an offline machine retries once an hour, not on every command. +export const refreshUpdateCache = async (deps: RefreshDeps = {}): Promise => { + const now = (deps.now ?? Date.now)(); + const cached = (deps.read ?? readUpdateCache)(); + if (cached && now - cached.checkedAt < TTL_MS) return; + let version: string | null = null; + try { + version = await (deps.fetch ?? (() => fetchLatestVersion(BG_TIMEOUT_MS)))(); + } catch { + version = null; + } + try { + (deps.write ?? writeUpdateCache)(version ?? cached?.latest ?? null, now); + } catch { + // total silence: a background check never disrupts a command + } +}; diff --git a/src/lib/update-check.test.ts b/src/lib/update-check.test.ts index c154774..1b3397a 100644 --- a/src/lib/update-check.test.ts +++ b/src/lib/update-check.test.ts @@ -22,50 +22,44 @@ describe('compareVersions', () => { }); describe('fetchCliLatest', () => { - it('parses the data envelope', async () => { + it('reads .version from the npm registry (no server floor or notice)', async () => { vi.stubGlobal( 'fetch', - vi.fn(async () => - jsonResponse(200, { - success: true, - data: { version: '0.2.0', minSupported: '0.1.0', message: 'note' }, - }) - ) + vi.fn(async () => jsonResponse(200, { name: '@agentage/cli', version: '0.2.0' })) ); - expect(await fetchCliLatest('https://x/api')).toEqual({ + expect(await fetchCliLatest()).toEqual({ version: '0.2.0', - minSupported: '0.1.0', - message: 'note', + minSupported: '0.0.0', + message: null, }); }); - it('returns null on a non-2xx, a throw, or a malformed body', async () => { + it('hits the public npm registry with a JSON Accept header', async () => { + const spy = vi.fn(async (_url: string, _init?: RequestInit) => + jsonResponse(200, { version: '0.2.0' }) + ); + vi.stubGlobal('fetch', spy); + await fetchCliLatest(); + const [url, init] = spy.mock.calls[0]!; + expect(url).toBe('https://registry.npmjs.org/@agentage/cli/latest'); + expect(init?.headers).toMatchObject({ Accept: 'application/json' }); + }); + + it('returns null on a non-2xx, a throw, or a body without a version', async () => { vi.stubGlobal( 'fetch', vi.fn(async () => jsonResponse(503, {})) ); - expect(await fetchCliLatest('https://x/api')).toBeNull(); + expect(await fetchCliLatest()).toBeNull(); vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('net'))); - expect(await fetchCliLatest('https://x/api')).toBeNull(); + expect(await fetchCliLatest()).toBeNull(); vi.stubGlobal( 'fetch', vi.fn(async () => jsonResponse(200, { nope: true })) ); - expect(await fetchCliLatest('https://x/api')).toBeNull(); - }); - - it('defaults missing fields (floor 0.0.0, no message) when version is present', async () => { - vi.stubGlobal( - 'fetch', - vi.fn(async () => jsonResponse(200, { data: { version: '0.2.0' } })) - ); - expect(await fetchCliLatest('https://x/api')).toEqual({ - version: '0.2.0', - minSupported: '0.0.0', - message: null, - }); + expect(await fetchCliLatest()).toBeNull(); }); }); diff --git a/src/lib/update-check.ts b/src/lib/update-check.ts index 4bd5ac0..f8a14e6 100644 --- a/src/lib/update-check.ts +++ b/src/lib/update-check.ts @@ -1,9 +1,11 @@ -// Self-update check for `setup`/`status`. Hits the PUBLIC GET {api}/cli/latest (no token - -// it's the one server call that works without auth) to learn the latest published version -// and the server-set support floor. Never throws: an unreachable endpoint = 'unknown'. +// Self-update check. Reads the latest published version straight from the public npm registry +// (the canonical source of truth for a published package). Never throws: an unreachable registry +// or any malformed payload yields 'unknown'. export const INSTALL_HINT = 'npm i -g @agentage/cli@latest'; +export const REGISTRY_URL = 'https://registry.npmjs.org/@agentage/cli/latest'; + export interface CliLatest { version: string | null; minSupported: string; @@ -38,21 +40,18 @@ export const compareVersions = (a: string, b: string): number => { return 0; }; -export const fetchCliLatest = async ( - apiUrl: string, - timeoutMs = 3000 -): Promise => { +export const fetchCliLatest = async (timeoutMs = 5000): Promise => { try { - const res = await fetch(`${apiUrl}/cli/latest`, { signal: AbortSignal.timeout(timeoutMs) }); + const res = await fetch(REGISTRY_URL, { + headers: { Accept: 'application/json' }, + signal: AbortSignal.timeout(timeoutMs), + }); if (!res.ok) return null; - const body = (await res.json().catch(() => null)) as { data?: Partial } | null; - const d = body?.data; - if (!d) return null; - return { - version: typeof d.version === 'string' ? d.version : null, - minSupported: typeof d.minSupported === 'string' ? d.minSupported : '0.0.0', - message: typeof d.message === 'string' ? d.message : null, - }; + const body = (await res.json().catch(() => null)) as { version?: unknown } | null; + const version = typeof body?.version === 'string' ? body.version : null; + if (!version) return null; + // The registry carries no support floor or notice, so neither gates an update hint. + return { version, minSupported: '0.0.0', message: null }; } catch { return null; } @@ -73,5 +72,5 @@ export const evaluateUpdate = (installed: string, latest: CliLatest | null): Upd return { status: { kind: 'current' }, message }; }; -export const checkForUpdate = async (apiUrl: string, installed: string): Promise => - evaluateUpdate(installed, await fetchCliLatest(apiUrl)); +export const checkForUpdate = async (installed: string): Promise => + evaluateUpdate(installed, await fetchCliLatest()); diff --git a/src/lib/update-lock.test.ts b/src/lib/update-lock.test.ts new file mode 100644 index 0000000..d08545a --- /dev/null +++ b/src/lib/update-lock.test.ts @@ -0,0 +1,95 @@ +import { execFile } from 'node:child_process'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { basename, join } from 'node:path'; +import { pathToFileURL } from 'node:url'; +import ts from 'typescript'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { getConfigDir } from './config.js'; +import { acquireUpdateLock, releaseUpdateLock } from './update-lock.js'; + +const lockFile = (): string => join(getConfigDir(), 'update.lock'); + +describe('update lock', () => { + let dir: string; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'agentage-lock-')); + process.env['AGENTAGE_CONFIG_DIR'] = join(dir, 'cfg'); + }); + + afterEach(() => { + delete process.env['AGENTAGE_CONFIG_DIR']; + rmSync(dir, { recursive: true, force: true }); + }); + + it('acquires when no lock is held', () => { + expect(acquireUpdateLock()).toBe(true); + expect(existsSync(lockFile())).toBe(true); + }); + + it('refuses when a fresh lock is already held', () => { + const now = 1_000_000; + expect(acquireUpdateLock(now)).toBe(true); + expect(acquireUpdateLock(now + 60_000)).toBe(false); // 1 min later, still within TTL + }); + + it('overrides a stale lock (older than the 10-minute TTL)', () => { + const now = 1_000_000; + expect(acquireUpdateLock(now)).toBe(true); + expect(acquireUpdateLock(now + 11 * 60_000)).toBe(true); // 11 min later, stale + }); + + it('releases idempotently', () => { + acquireUpdateLock(); + releaseUpdateLock(); + releaseUpdateLock(); + expect(existsSync(lockFile())).toBe(false); + }); + + // The TOCTOU proof: real concurrent PROCESSES running the actual module, transpiled on the fly + // (a check-then-write lock lets most of them win; the O_EXCL create admits exactly one). + const raceProcesses = async (count: number, now: number): Promise => { + const libDir = join(dir, 'lib'); + mkdirSync(libDir, { recursive: true }); + for (const src of ['config.ts', 'update-lock.ts']) { + const out = ts.transpileModule(readFileSync(join(import.meta.dirname, src), 'utf-8'), { + compilerOptions: { module: ts.ModuleKind.ESNext, target: ts.ScriptTarget.ES2022 }, + }).outputText; + writeFileSync(join(libDir, basename(src).replace(/\.ts$/, '.js')), out, 'utf-8'); + } + const script = join(dir, 'race.mjs'); + const mod = pathToFileURL(join(libDir, 'update-lock.js')).href; + writeFileSync( + script, + `import { acquireUpdateLock } from '${mod}';\n` + + `process.stdout.write(acquireUpdateLock(${now}) ? '1' : '0');\n`, + 'utf-8' + ); + const env = { ...process.env, AGENTAGE_CONFIG_DIR: join(dir, 'cfg') }; + return Promise.all( + Array.from( + { length: count }, + () => + new Promise((resolve) => { + execFile(process.execPath, [script], { env }, (_err, stdout) => resolve(stdout)); + }) + ) + ); + }; + + it('exactly one of many concurrent processes wins the lock', async () => { + const runs = await raceProcesses(12, 1_000_000); + expect(runs.filter((r) => r === '1')).toHaveLength(1); + }, 20_000); + + it('exactly one of many concurrent processes takes over a STALE lock', async () => { + // The ABA proof: a naive unlink-takeover lets a taker delete the fresh winner's lock and + // re-win; the rename takeover admits exactly one. + const now = 1_000_000; + mkdirSync(join(dir, 'cfg'), { recursive: true }); + writeFileSync(lockFile(), String(now - 11 * 60_000), 'utf-8'); // >10min old = stale + const runs = await raceProcesses(12, now); + expect(runs.filter((r) => r === '1')).toHaveLength(1); + }, 20_000); +}); diff --git a/src/lib/update-lock.ts b/src/lib/update-lock.ts new file mode 100644 index 0000000..0a99898 --- /dev/null +++ b/src/lib/update-lock.ts @@ -0,0 +1,73 @@ +import { existsSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { ensureConfigDir, getConfigDir } from './config.js'; + +const LOCK_FILE = 'update.lock'; +const LOCK_TTL_MS = 10 * 60 * 1000; // a lock older than this is treated as stale (crashed run) + +const lockPath = (): string => join(getConfigDir(), LOCK_FILE); + +const heldAt = (path: string): number | null => { + try { + const n = Number.parseInt(readFileSync(path, 'utf-8').trim(), 10); + return Number.isNaN(n) ? null : n; + } catch { + return null; + } +}; + +const unlinkQuiet = (path: string): void => { + try { + unlinkSync(path); + } catch { + // already gone + } +}; + +// Serialize stale-lock takeover behind its own exclusive create, so no taker can ever delete a +// lock that turned FRESH mid-takeover (the ABA race: reader sees the stale timestamp, a winner +// re-creates the lock, the reader deletes the winner's file). Under the guard the staleness is +// re-checked and only a genuinely stale lock is cleared; losers simply refuse. A guard left by a +// crashed taker expires like the lock itself. Returns whether to re-race the create. +const takeOverStale = (now: number): boolean => { + const guard = `${lockPath()}.takeover`; + const guardHeld = heldAt(guard); + if (guardHeld !== null && now - guardHeld >= LOCK_TTL_MS) unlinkQuiet(guard); + try { + writeFileSync(guard, String(now), { flag: 'wx' }); + } catch { + return false; // another taker is mid-takeover; let it win + } + try { + const held = heldAt(lockPath()); + if (held !== null && now - held < LOCK_TTL_MS) return false; // became fresh meanwhile + unlinkQuiet(lockPath()); + return true; + } finally { + unlinkQuiet(guard); + } +}; + +// Acquire the single-writer update lock. The O_EXCL ('wx') create IS the mutex - concurrent runs +// cannot both win it, unlike a check-then-write. A fresh holder (younger than the TTL) refuses; +// a stale one (crashed run) is cleared under the takeover guard and re-raced once. `now` is +// injectable. +export const acquireUpdateLock = (now: number = Date.now()): boolean => { + ensureConfigDir(); + for (let attempt = 0; attempt < 2; attempt++) { + try { + writeFileSync(lockPath(), String(now), { flag: 'wx' }); + return true; + } catch { + const held = heldAt(lockPath()); + if (held !== null && now - held < LOCK_TTL_MS) return false; + if (!takeOverStale(now)) return false; + } + } + return false; +}; + +export const releaseUpdateLock = (): void => { + const path = lockPath(); + if (existsSync(path)) unlinkSync(path); +}; diff --git a/src/sync/cycle.test.ts b/src/sync/cycle.test.ts index f918f60..cc415a9 100644 --- a/src/sync/cycle.test.ts +++ b/src/sync/cycle.test.ts @@ -116,6 +116,118 @@ describe('runSyncCycle', () => { // Remote: the push landed, both sides preserved there too. expect(g(bare, ['show', 'main:note.md'])).toContain('LOCAL-CHANGE'); expect(g(bare, ['show', 'main:note.conflict.md'])).toContain('REMOTE-CHANGE'); + + // Crash-window closed: the conflict copy is committed in the SAME (merge) commit, never a + // follow-up. HEAD is a merge (two parents) and its own diff lists the conflict file. + expect(g(work, ['rev-list', '--parents', '-n', '1', 'HEAD']).trim().split(' ')).toHaveLength(3); + const mergeFiles = g(work, ['show', '--name-only', '--format=', 'HEAD']).trim().split('\n'); + expect(mergeFiles).toContain('note.conflict.md'); + }); + + it('auto-commit message is an ISO-stamped `sync:` line', async () => { + writeFile(work, 'a.md', 'x'); + await runSyncCycle(target({ path: work, remote: bare })); + expect(g(work, ['log', '-1', '--format=%s']).trim()).toMatch(/^sync: \d{4}-/); + }); + + it('keeps syncing a file that is tracked before being added to ignore', async () => { + writeFile(work, 'tracked.md', 'v1\n'); + await runSyncCycle(target({ path: work, remote: bare })); + // gitignore semantics only affect untracked paths: a tracked file's edits still commit + push. + writeFile(work, 'tracked.md', 'v2\n'); + const result = await runSyncCycle(target({ path: work, remote: bare, ignore: ['tracked.md'] })); + expect(result.ok).toBe(true); + expect(result.committed).toBe(true); + expect(g(bare, ['show', 'main:tracked.md'])).toContain('v2'); + }); + + // Set up a diverged pair: remote holds REMOTE-CHANGE on note.md + extra.md, local holds + // LOCAL-CHANGE on note.md; both committed. Used by the crash-recovery tests below. + const diverge = async (): Promise => { + writeFile(work, 'note.md', 'base\n'); + await runSyncCycle(target({ path: work, remote: bare })); + const other = join(root, 'other'); + g(root, ['clone', bare, other]); + writeFile(other, 'note.md', 'REMOTE-CHANGE\n'); + writeFile(other, 'extra.md', 'remote extra\n'); + g(other, ['add', '-A']); + g(other, ['commit', '-m', 'remote change']); + g(other, ['push', 'origin', 'HEAD:main']); + writeFile(work, 'note.md', 'LOCAL-CHANGE\n'); + g(work, ['commit', '-am', 'local change']); + }; + + it('recovers when a previous cycle died between `merge --no-commit` and its commit', async () => { + await diverge(); + // Simulate the crash: the merge is staged (MERGE_HEAD present) but never committed. + g(work, ['fetch', 'sync']); + g(work, ['merge', '--no-commit', '-X', 'ours', 'sync/main']); + expect(existsSync(join(work, '.git', 'MERGE_HEAD'))).toBe(true); + + const result = await runSyncCycle(target({ path: work, remote: bare })); + expect(result.ok).toBe(true); + expect(result.pushed).toBe(true); + expect(result.conflicts).toEqual(['note.conflict.md']); + expect(existsSync(join(work, '.git', 'MERGE_HEAD'))).toBe(false); + // No bogus half-merge minted as a `sync:` auto-commit: HEAD is a proper merge commit + // carrying the conflict copy, and both sides survived to the remote. + expect(g(work, ['rev-list', '--parents', '-n', '1', 'HEAD']).trim().split(' ')).toHaveLength(3); + expect(g(bare, ['show', 'main:note.md'])).toContain('LOCAL-CHANGE'); + expect(g(bare, ['show', 'main:note.conflict.md'])).toContain('REMOTE-CHANGE'); + expect(g(bare, ['show', 'main:extra.md'])).toContain('remote extra'); + + // Pushes resume: the next cycle is a clean no-op, not a wedge. + const next = await runSyncCycle(target({ path: work, remote: bare })); + expect(next.ok).toBe(true); + expect(next.committed).toBe(false); + expect(next.pushed).toBe(true); + }); + + it('recovers when a user edited a merge-touched file after a mid-merge crash', async () => { + await diverge(); + // Crash state + a user edit to a file the staged merge touched: `merge --abort` refuses + // ("entry not uptodate"), so recovery must complete the merge instead of leaking MERGE_HEAD. + g(work, ['fetch', 'sync']); + g(work, ['merge', '--no-commit', '-X', 'ours', 'sync/main']); + writeFile(work, 'note.md', 'USER-EDIT\n'); + + const results = []; + for (let i = 0; i < 3; i++) + results.push(await runSyncCycle(target({ path: work, remote: bare }))); + // No permanent ok:false loop. + expect(results.map((r) => r.ok)).toEqual([true, true, true]); + expect(results.map((r) => r.pushed)).toEqual([true, true, true]); + expect(existsSync(join(work, '.git', 'MERGE_HEAD'))).toBe(false); + + // No phantom conflict-file accumulation across cycles. + const conflictFiles = g(work, ['ls-files']) + .split('\n') + .filter((f) => f.includes('.conflict')); + expect(conflictFiles.length).toBeLessThanOrEqual(1); + + // The user edit survived and reached the remote; the remote side stays reachable in history. + expect(readFileSync(join(work, 'note.md'), 'utf8')).toContain('USER-EDIT'); + expect(g(bare, ['show', 'main:note.md'])).toContain('USER-EDIT'); + expect(g(bare, ['show', 'main:extra.md'])).toContain('remote extra'); + }); + + it('recovers when a previous cycle died mid-rebase', async () => { + await diverge(); + g(work, ['fetch', 'sync']); + try { + g(work, ['rebase', 'sync/main']); // conflicts -> leaves .git/rebase-merge behind + } catch { + // expected: conflicted rebase exits non-zero + } + expect(existsSync(join(work, '.git', 'rebase-merge'))).toBe(true); + + const result = await runSyncCycle(target({ path: work, remote: bare })); + expect(result.ok).toBe(true); + expect(result.pushed).toBe(true); + expect(result.conflicts).toEqual(['note.conflict.md']); + expect(existsSync(join(work, '.git', 'rebase-merge'))).toBe(false); + expect(g(bare, ['show', 'main:note.md'])).toContain('LOCAL-CHANGE'); + expect(g(bare, ['show', 'main:note.conflict.md'])).toContain('REMOTE-CHANGE'); }); it('merges remote non-conflicting changes cleanly', async () => { diff --git a/src/sync/cycle.ts b/src/sync/cycle.ts index 788f624..b964aae 100644 --- a/src/sync/cycle.ts +++ b/src/sync/cycle.ts @@ -56,10 +56,26 @@ const ensureRemote = async (git: SyncGit, name: string, url: string): Promise => { + const gitDir = join(path, '.git'); + if (existsSync(join(gitDir, 'MERGE_HEAD'))) { + await git.exec(['merge', '--abort']); + // The abort refuses when the user edited a merge-touched file meanwhile ("entry not uptodate"); + // complete the staged merge instead so MERGE_HEAD never leaks into commitIfDirty. + if (existsSync(join(gitDir, 'MERGE_HEAD'))) await git.exec(['commit', '--no-edit']); + } + if (existsSync(join(gitDir, 'rebase-merge')) || existsSync(join(gitDir, 'rebase-apply'))) + await git.exec(['rebase', '--abort']); +}; + // Reconcile a diverged history keeping BOTH sides. A rebase probes for a true conflict; on a clean // replay history stays linear. On conflict the local files are kept as-is and each conflicted -// file's remote copy is written alongside as `.conflict.md`, so no write is lost on either -// side. Returns the conflict-copy paths written. +// file's remote copy is written alongside as `.conflict.md`. The `.conflict.md` copies are +// staged into the SAME merge commit (`merge --no-commit`), so no single commit boundary ever +// leaves the remote side merged-away yet unsurfaced. Returns the conflict-copy paths written. const reconcile = async ( git: SyncGit, cwd: string, @@ -81,13 +97,13 @@ const reconcile = async ( if (show.code === 0) remoteSides.set(p, show.stdout); } - // Merge remote in, auto-resolving content conflicts to the local side; keeps remote's - // non-conflicting changes and yields a push-able (descendant-of-remote) history. - const merge = await git.exec(['merge', '-X', 'ours', '--no-edit', ref]); + // Stage the merge WITHOUT committing (auto-resolving content conflicts to the local side); an + // unresolvable conflict (add/add, modify/delete) falls back to keeping ours. Everything stays + // staged so the conflict copies land in the very same commit. + const merge = await git.exec(['merge', '--no-commit', '-X', 'ours', '--no-edit', ref]); if (merge.code !== 0) { await git.exec(['checkout', '--ours', '--', '.']); await git.run(['add', '-A']); - await git.run(['commit', '--no-edit']); } const written: string[] = []; @@ -98,14 +114,15 @@ const reconcile = async ( await writeFile(abs, content, 'utf8'); written.push(name); } - if (written.length) { - await git.run(['add', '-A']); - await git.run([ - 'commit', - '-m', - `sync: preserve remote copy of ${written.length} conflicted file(s) (${now})`, - ]); - } + await git.run(['add', '-A']); + // Self-sufficient close: nothing staged and no merge to conclude = nothing to commit (a bare + // `commit --no-edit` would abort on the empty message and wedge the cycle); an absent MERGE_MSG + // (recovered state) falls back to an explicit -m. + const staged = await git.exec(['diff', '--cached', '--quiet']); + const merging = existsSync(join(cwd, '.git', 'MERGE_HEAD')); + if (staged.code === 0 && !merging) return written; + if (existsSync(join(cwd, '.git', 'MERGE_MSG'))) await git.run(['commit', '--no-edit']); + else await git.run(['commit', '-m', `sync: merge ${ref} (${now})`]); return written; }; @@ -132,6 +149,7 @@ export const runSyncCycle = async ( if (!existsSync(target.path)) return { ...base, ok: true }; // nothing on disk yet if (!existsSync(join(target.path, '.git'))) await git.run(['init', '-b', 'main']); + await abortInterrupted(git, target.path); await writeExclude(target.path, target.ignore); await ensureRemote(git, target.remoteName, target.remote); committed = await commitIfDirty(git, `sync: ${now}`);