feat(cli): commonly doctor, plus a stale-CLI notice on agent run and daemon run (TASK-660)

cli/__tests__/command-registration.test.mjs

@@ -22,13 +22,15 @@
 import { Command } from 'commander';
 import { cascadeOverridesFromOpts, registerAgent } from '../src/commands/agent.js';
 import { registerDaemon } from '../src/commands/daemon.js';
+import { registerDoctor } from '../src/commands/doctor.js';
 import { CASCADE_ENV_VARS } from '../src/lib/enforcement.js';
 
 const registerAll = () => {
   const program = new Command();
   program.exitOverride();
   registerAgent(program);
   registerDaemon(program);
+  registerDoctor(program);
   return program;
 };
 
@@ -112,3 +114,9 @@ describe('command registration', () => {
     }
   });
 });
+
+test('doctor is wired with its --instance option (TASK-660)', () => {
+  const doctor = findCommand(registerAll(), ['doctor']);
+  expect(doctor).toBeDefined();
+  expect(doctor.options.map((option) => option.long)).toContain('--instance');
+});

cli/__tests__/daemon-service.test.mjs

@@ -123,15 +123,16 @@ describe('install', () => {
     expect(deps.execCmd).toHaveBeenLastCalledWith(['launchctl', 'load', '-w', target.file]);
   });
 
-  test('linux: writes the unit, reloads, enables --now', async () => {
+  test('linux: writes the unit, reloads, enables, and restarts so a running unit takes the new env', async () => {
     const deps = makeDeps();
     const target = await installDaemonService({
       platform: 'linux', home, nodePath, cliPath, ...deps,
     });
     expect(target.kind).toBe('systemd');
     expect(deps.execCmd.mock.calls).toEqual([
       [['systemctl', '--user', 'daemon-reload']],
-      [['systemctl', '--user', 'enable', '--now', SYSTEMD_UNIT]],
+      [['systemctl', '--user', 'enable', SYSTEMD_UNIT]],
+      [['systemctl', '--user', 'restart', SYSTEMD_UNIT]],
     ]);
   });
 
@@ -188,6 +189,30 @@ describe('install', () => {
     expect(withoutKey.warn).toHaveBeenCalledWith(expect.stringContaining('COMMONLY_LITELLM_KEY is not set in this shell'));
   });
 
+  test('install carries a set opt-out into the unit, and says nothing when it is absent (TASK-660)', async () => {
+    const set = makeDeps();
+    set.warn = jest.fn();
+    await installDaemonService({
+      platform: 'linux', home, nodePath, cliPath,
+      optionalEnvNames: ['COMMONLY_NO_UPDATE_CHECK'],
+      env: { COMMONLY_NO_UPDATE_CHECK: '1' },
+      ...set,
+    });
+    expect(set.writeFile.mock.calls[0][1]).toContain('Environment="COMMONLY_NO_UPDATE_CHECK=1"');
+    expect(set.warn).not.toHaveBeenCalled();
+
+    const absent = makeDeps();
+    absent.warn = jest.fn();
+    await installDaemonService({
+      platform: 'darwin', home, nodePath, cliPath,
+      optionalEnvNames: ['COMMONLY_NO_UPDATE_CHECK'],
+      env: {},
+      ...absent,
+    });
+    expect(absent.writeFile.mock.calls[0][1]).not.toContain('COMMONLY_NO_UPDATE_CHECK');
+    expect(absent.warn).not.toHaveBeenCalled();
+  });
+
   // The mode is held by construction — a sibling file created 0600 and renamed
   // over the target — so the property to witness is the mode and the content,
   // not which call did it. Real file through the REAL default writer, with chmod

cli/__tests__/detach.test.mjs

@@ -29,6 +29,7 @@ const {
   saveAgentToken,
   loadAgentToken,
   listLocalAgents,
+  listUnreadableAgentTokens,
 } = await import('../src/commands/agent.js');
 const { setSession, getSession } = await import('../src/lib/session-store.js');
 
@@ -233,4 +234,39 @@ describe('listLocalAgents', () => {
     expect(agent.name).toBe('corrupt');
     expect(agent.lastTurn).toBeNull();
   });
+
+  test('a token file that is not JSON is dropped from the listing and named as unreadable', () => {
+    saveAgentToken('fine', {
+      agentName: 'fine', instanceId: 'default', podId: 'pod-f',
+      instanceUrl: 'http://localhost:5000', runtimeToken: 'cm_agent_f',
+      adapter: 'claude',
+    });
+    fs.writeFileSync(path.join(tmp, '.commonly', 'tokens', 'broken.json'), '{"agentName": ', 'utf8');
+
+    expect(listLocalAgents().map((a) => a.name)).toEqual(['fine']);
+    expect(listUnreadableAgentTokens()).toEqual(['broken']);
+  });
+
+  test('no tokens dir means nothing unreadable', () => {
+    expect(listUnreadableAgentTokens()).toEqual([]);
+  });
+
+  test('a tokens path that is a file means nothing unreadable', () => {
+    fs.mkdirSync(path.join(tmp, '.commonly'), { recursive: true });
+    fs.writeFileSync(path.join(tmp, '.commonly', 'tokens'), '', 'utf8');
+    expect(listUnreadableAgentTokens()).toEqual([]);
+  });
+
+  // Mode 000 stages the denial only on POSIX as a non-root user: root reads
+  // through it, and Windows chmod cannot remove a directory's read permission.
+  (process.platform === 'win32' || process.getuid?.() === 0 ? test.skip : test)('a tokens dir it cannot traverse throws instead of reading as empty', () => {
+    const state = path.join(tmp, '.commonly');
+    fs.mkdirSync(path.join(state, 'tokens'), { recursive: true });
+    fs.chmodSync(state, 0o000);
+    try {
+      expect(() => listUnreadableAgentTokens()).toThrow(/EACCES/);
+    } finally {
+      fs.chmodSync(state, 0o700);
+    }
+  });
 });

cli/__tests__/doctor-host-tokens.test.mjs

@@ -0,0 +1,36 @@
+/**
+ * The production wiring of doctor's agent listing (TASK-660): hostDoctorDeps
+ * must hand runDoctor the unreadable token files, or a malformed file reads as
+ * "no agent here" again. HOME is a temp dir, so nothing on this host is read.
+ */
+import { jest } from '@jest/globals';
+import os from 'os';
+import path from 'path';
+import fs from 'fs';
+
+const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-doctor-host-'));
+
+await jest.unstable_mockModule('os', () => ({
+  ...os,
+  default: { ...os, homedir: () => tmp },
+  homedir: () => tmp,
+}));
+
+const { hostDoctorDeps } = await import('../src/commands/doctor.js');
+const { saveAgentToken } = await import('../src/commands/agent.js');
+
+afterAll(() => {
+  fs.rmSync(tmp, { recursive: true, force: true });
+});
+
+test('hostDoctorDeps lists the agents that load and names the token file that does not', () => {
+  saveAgentToken('fine', {
+    agentName: 'fine', instanceId: 'default', podId: 'pod-f',
+    instanceUrl: 'http://localhost:5000', runtimeToken: 'cm_agent_f', adapter: 'claude',
+  });
+  fs.writeFileSync(path.join(tmp, '.commonly', 'tokens', 'broken.json'), '{"agentName": ', 'utf8');
+
+  const deps = hostDoctorDeps();
+  expect(deps.listAgents().map((a) => a.name)).toEqual(['fine']);
+  expect(deps.listUnreadable()).toEqual(['broken']);
+});

cli/__tests__/doctor-host.test.mjs

@@ -0,0 +1,70 @@
+import { loginTarget, timedClient, versionCommand } from '../src/commands/doctor.js';
+
+// The host wiring's two decisions, with config resolution faked. loginTarget
+// must agree with createClient (api.js): getToken prefers COMMONLY_TOKEN, and
+// resolveInstanceUrl falls back to the default URL when nothing is saved.
+
+const DEFAULT = 'https://api.commonly.me';
+const fakes = ({ saved = null, env = {} } = {}) => ({
+  env,
+  resolve: () => saved,
+  resolveUrl: () => saved?.url || DEFAULT,
+  tokenFor: () => (env.COMMONLY_TOKEN || saved?.token || null),
+});
+
+describe('loginTarget', () => {
+  test('a saved login sends the saved token', () => {
+    const saved = { key: 'default', url: DEFAULT, token: 'cm_x' };
+    expect(loginTarget(fakes({ saved }))).toEqual({
+      key: 'default', url: DEFAULT, tokenSource: 'saved', loginHint: 'commonly login',
+    });
+  });
+
+  test('COMMONLY_TOKEN with nothing saved is a usable login on the default URL, as it is for every command', () => {
+    expect(loginTarget(fakes({ env: { COMMONLY_TOKEN: 'cm_env' } }))).toMatchObject({
+      key: null, url: DEFAULT, tokenSource: 'env',
+    });
+  });
+
+  test('COMMONLY_TOKEN beside a saved login is the token sent, and is named as such', () => {
+    const saved = { key: 'default', url: DEFAULT, token: 'cm_saved' };
+    expect(loginTarget(fakes({ saved, env: { COMMONLY_TOKEN: 'cm_env' } })).tokenSource).toBe('env');
+  });
+
+  test('nothing saved and no env token is no token, and the hint keeps the --instance given', () => {
+    expect(loginTarget({ ...fakes(), instanceArg: 'foo' })).toMatchObject({
+      tokenSource: null, loginHint: 'commonly login --instance foo',
+    });
+  });
+
+  test('never carries the token itself', () => {
+    const saved = { key: 'default', url: DEFAULT, token: 'cm_secret' };
+    expect(JSON.stringify(loginTarget(fakes({ saved, env: { COMMONLY_TOKEN: 'cm_env_secret' } })))).not.toMatch(/cm_/);
+  });
+});
+
+describe('versionCommand', () => {
+  test('a Windows .cmd shim goes through the shell, quoted', () => {
+    expect(versionCommand('C:\\Program Files\\nodejs\\commonly.cmd', 'win32')).toEqual({
+      command: '"C:\\Program Files\\nodejs\\commonly.cmd" --version', args: [], shell: true,
+    });
+  });
+
+  test('everything else is spawned directly', () => {
+    expect(versionCommand('/opt/homebrew/bin/commonly', 'darwin')).toEqual({
+      command: '/opt/homebrew/bin/commonly', args: ['--version'], shell: false,
+    });
+    expect(versionCommand('C:\\npm\\commonly.exe', 'win32').shell).toBe(false);
+  });
+});
+
+describe('timedClient', () => {
+  test('every get carries a timeout signal, so a hung server is a fail line, not a hang', async () => {
+    const seen = [];
+    const client = timedClient({ get: async (path, params, options) => { seen.push({ path, params, options }); return {}; } });
+    await client.get('/api/auth/user');
+    expect(seen[0].path).toBe('/api/auth/user');
+    expect(seen[0].options.signal).toBeInstanceOf(AbortSignal);
+    expect(seen[0].options.signal.aborted).toBe(false);
+  });
+});