feat: persistent Gemini ACP provider + status spinner

Replace per-call gemini CLI spawning (~10s cold start each time) with persistent ACP (Agent Client Protocol) subprocess. First call absorbs the cold start, subsequent calls are near-instant over JSON-RPC stdio. - Add AcpClient: manages persistent gemini --experimental-acp subprocess with lazy init, auto-restart on crash/timeout, NDJSON framing - Add GeminiAcpProvider: LlmProvider wrapper with serial queue for concurrent calls, same interface as GeminiCliProvider - Add dispose() to LlmProvider interface + disposeAll() to registry - Wire provider disposal into mcplocal shutdown handler - Add status command spinner with progressive output and color-coded LLM health check results (green checkmark/red cross) - 25 new tests (17 ACP client + 8 provider) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-24 23:52:04 +00:00
parent 848868d45f
commit 11da8b1fbf
12 changed files with 1102 additions and 140 deletions
--- a/src/cli/src/commands/status.ts
+++ b/src/cli/src/commands/status.ts
@@ -11,12 +11,21 @@ import { APP_VERSION } from '@mcpctl/shared';

 const execFileAsync = promisify(execFile);

+// ANSI helpers
+const GREEN = '\x1b[32m';
+const RED = '\x1b[31m';
+const DIM = '\x1b[2m';
+const RESET = '\x1b[0m';
+const CLEAR_LINE = '\x1b[2K\r';
+
 export interface StatusCommandDeps {
  configDeps: Partial<ConfigLoaderDeps>;
  credentialsDeps: Partial<CredentialsDeps>;
  log: (...args: string[]) => void;
+  write: (text: string) => void;
  checkHealth: (url: string) => Promise<boolean>;
  checkLlm: (llm: LlmConfig) => Promise<string>;
+  isTTY: boolean;
 }

 function defaultCheckHealth(url: string): Promise<boolean> {
@@ -64,16 +73,20 @@ async function defaultCheckLlm(llm: LlmConfig): Promise<string> {
  return 'ok (key stored)';
 }

+const SPINNER_FRAMES = ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏'];
+
 const defaultDeps: StatusCommandDeps = {
  configDeps: {},
  credentialsDeps: {},
  log: (...args) => console.log(...args),
+  write: (text) => process.stdout.write(text),
  checkHealth: defaultCheckHealth,
  checkLlm: defaultCheckLlm,
+  isTTY: process.stdout.isTTY ?? false,
 };

 export function createStatusCommand(deps?: Partial<StatusCommandDeps>): Command {
-  const { configDeps, credentialsDeps, log, checkHealth, checkLlm } = { ...defaultDeps, ...deps };
+  const { configDeps, credentialsDeps, log, write, checkHealth, checkLlm, isTTY } = { ...defaultDeps, ...deps };

  return new Command('status')
    .description('Show mcpctl status and connectivity')
@@ -86,45 +99,81 @@ export function createStatusCommand(deps?: Partial<StatusCommandDeps>): Command
        ? `${config.llm.provider}${config.llm.model ? ` / ${config.llm.model}` : ''}`
        : null;

-      // Run health checks in parallel (include LLM check if configured)
-      const healthPromises: [Promise<boolean>, Promise<boolean>, Promise<string | null>] = [
+      if (opts.output !== 'table') {
+        // JSON/YAML: run everything in parallel, wait, output at once
+        const [mcplocalReachable, mcpdReachable, llmStatus] = await Promise.all([
+          checkHealth(config.mcplocalUrl),
+          checkHealth(config.mcpdUrl),
+          llmLabel ? checkLlm(config.llm!) : Promise.resolve(null),
+        ]);
+
+        const llm = llmLabel
+          ? llmStatus === 'ok' ? llmLabel : `${llmLabel} (${llmStatus})`
+          : null;
+
+        const status = {
+          version: APP_VERSION,
+          mcplocalUrl: config.mcplocalUrl,
+          mcplocalReachable,
+          mcpdUrl: config.mcpdUrl,
+          mcpdReachable,
+          auth: creds ? { user: creds.user } : null,
+          registries: config.registries,
+          outputFormat: config.outputFormat,
+          llm,
+          llmStatus,
+        };
+
+        log(opts.output === 'json' ? formatJson(status) : formatYaml(status));
+        return;
+      }
+
+      // Table format: print lines progressively, LLM last with spinner
+
+      // Fast health checks first
+      const [mcplocalReachable, mcpdReachable] = await Promise.all([
        checkHealth(config.mcplocalUrl),
        checkHealth(config.mcpdUrl),
-        config.llm && config.llm.provider !== 'none'
-          ? checkLlm(config.llm)
-          : Promise.resolve(null),
-      ];
-      const [mcplocalReachable, mcpdReachable, llmStatus] = await Promise.all(healthPromises);
+      ]);

-      const llm = llmLabel
-        ? llmStatus === 'ok' ? llmLabel : `${llmLabel} (${llmStatus})`
-        : null;
+      log(`mcpctl v${APP_VERSION}`);
+      log(`mcplocal:   ${config.mcplocalUrl} (${mcplocalReachable ? 'connected' : 'unreachable'})`);
+      log(`mcpd:       ${config.mcpdUrl} (${mcpdReachable ? 'connected' : 'unreachable'})`);
+      log(`Auth:       ${creds ? `logged in as ${creds.user}` : 'not logged in'}`);
+      log(`Registries: ${config.registries.join(', ')}`);
+      log(`Output:     ${config.outputFormat}`);

-      const status = {
-        version: APP_VERSION,
-        mcplocalUrl: config.mcplocalUrl,
-        mcplocalReachable,
-        mcpdUrl: config.mcpdUrl,
-        mcpdReachable,
-        auth: creds ? { user: creds.user } : null,
-        registries: config.registries,
-        outputFormat: config.outputFormat,
-        llm,
-        llmStatus,
-      };
+      if (!llmLabel) {
+        log(`LLM:        not configured (run 'mcpctl config setup')`);
+        return;
+      }

-      if (opts.output === 'json') {
-        log(formatJson(status));
-      } else if (opts.output === 'yaml') {
-        log(formatYaml(status));
+      // LLM check with spinner
+      const llmPromise = checkLlm(config.llm!);
+
+      if (isTTY) {
+        let frame = 0;
+        const interval = setInterval(() => {
+          write(`${CLEAR_LINE}LLM:        ${llmLabel} ${DIM}${SPINNER_FRAMES[frame % SPINNER_FRAMES.length]} checking...${RESET}`);
+          frame++;
+        }, 80);
+
+        const llmStatus = await llmPromise;
+        clearInterval(interval);
+
+        if (llmStatus === 'ok' || llmStatus === 'ok (key stored)') {
+          write(`${CLEAR_LINE}LLM:        ${llmLabel} ${GREEN}✓ ${llmStatus}${RESET}\n`);
+        } else {
+          write(`${CLEAR_LINE}LLM:        ${llmLabel} ${RED}✗ ${llmStatus}${RESET}\n`);
+        }
      } else {
-        log(`mcpctl v${status.version}`);
-        log(`mcplocal:   ${status.mcplocalUrl} (${mcplocalReachable ? 'connected' : 'unreachable'})`);
-        log(`mcpd:       ${status.mcpdUrl} (${mcpdReachable ? 'connected' : 'unreachable'})`);
-        log(`Auth:       ${creds ? `logged in as ${creds.user}` : 'not logged in'}`);
-        log(`Registries: ${status.registries.join(', ')}`);
-        log(`Output:     ${status.outputFormat}`);
-        log(`LLM:        ${status.llm ?? "not configured (run 'mcpctl config setup')"}`);
+        // Non-TTY: no spinner, just wait and print
+        const llmStatus = await llmPromise;
+        if (llmStatus === 'ok' || llmStatus === 'ok (key stored)') {
+          log(`LLM:        ${llmLabel} ✓ ${llmStatus}`);
+        } else {
+          log(`LLM:        ${llmLabel} ✗ ${llmStatus}`);
+        }
      }
    });
 }
--- a/src/cli/tests/commands/status.test.ts
+++ b/src/cli/tests/commands/status.test.ts
@@ -3,19 +3,38 @@ import { mkdtempSync, rmSync } from 'node:fs';
 import { join } from 'node:path';
 import { tmpdir } from 'node:os';
 import { createStatusCommand } from '../../src/commands/status.js';
+import type { StatusCommandDeps } from '../../src/commands/status.js';
 import { saveConfig, DEFAULT_CONFIG } from '../../src/config/index.js';
 import { saveCredentials } from '../../src/auth/index.js';

 let tempDir: string;
 let output: string[];
+let written: string[];

 function log(...args: string[]) {
  output.push(args.join(' '));
 }

+function write(text: string) {
+  written.push(text);
+}
+
+function baseDeps(overrides?: Partial<StatusCommandDeps>): Partial<StatusCommandDeps> {
+  return {
+    configDeps: { configDir: tempDir },
+    credentialsDeps: { configDir: tempDir },
+    log,
+    write,
+    checkHealth: async () => true,
+    isTTY: false,
+    ...overrides,
+  };
+}
+
 beforeEach(() => {
  tempDir = mkdtempSync(join(tmpdir(), 'mcpctl-status-test-'));
  output = [];
+  written = [];
 });

 afterEach(() => {
@@ -24,12 +43,7 @@ afterEach(() => {

 describe('status command', () => {
  it('shows status in table format', async () => {
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
-      checkHealth: async () => true,
-    });
+    const cmd = createStatusCommand(baseDeps());
    await cmd.parseAsync([], { from: 'user' });
    const out = output.join('\n');
    expect(out).toContain('mcpctl v');
@@ -39,46 +53,26 @@ describe('status command', () => {
  });

  it('shows unreachable when daemons are down', async () => {
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
-      checkHealth: async () => false,
-    });
+    const cmd = createStatusCommand(baseDeps({ checkHealth: async () => false }));
    await cmd.parseAsync([], { from: 'user' });
    expect(output.join('\n')).toContain('unreachable');
  });

  it('shows not logged in when no credentials', async () => {
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
-      checkHealth: async () => true,
-    });
+    const cmd = createStatusCommand(baseDeps());
    await cmd.parseAsync([], { from: 'user' });
    expect(output.join('\n')).toContain('not logged in');
  });

  it('shows logged in user when credentials exist', async () => {
    saveCredentials({ token: 'tok', mcpdUrl: 'http://x:3100', user: 'alice@example.com' }, { configDir: tempDir });
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
-      checkHealth: async () => true,
-    });
+    const cmd = createStatusCommand(baseDeps());
    await cmd.parseAsync([], { from: 'user' });
    expect(output.join('\n')).toContain('logged in as alice@example.com');
  });

  it('shows status in JSON format', async () => {
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
-      checkHealth: async () => true,
-    });
+    const cmd = createStatusCommand(baseDeps());
    await cmd.parseAsync(['-o', 'json'], { from: 'user' });
    const parsed = JSON.parse(output[0]) as Record<string, unknown>;
    expect(parsed['version']).toBe('0.1.0');
@@ -87,12 +81,7 @@ describe('status command', () => {
  });

  it('shows status in YAML format', async () => {
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
-      checkHealth: async () => false,
-    });
+    const cmd = createStatusCommand(baseDeps({ checkHealth: async () => false }));
    await cmd.parseAsync(['-o', 'yaml'], { from: 'user' });
    expect(output[0]).toContain('mcplocalReachable: false');
  });
@@ -100,15 +89,12 @@ describe('status command', () => {
  it('checks correct URLs from config', async () => {
    saveConfig({ ...DEFAULT_CONFIG, mcplocalUrl: 'http://local:3200', mcpdUrl: 'http://remote:3100' }, { configDir: tempDir });
    const checkedUrls: string[] = [];
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
+    const cmd = createStatusCommand(baseDeps({
      checkHealth: async (url) => {
        checkedUrls.push(url);
        return false;
      },
-    });
+    }));
    await cmd.parseAsync([], { from: 'user' });
    expect(checkedUrls).toContain('http://local:3200');
    expect(checkedUrls).toContain('http://remote:3100');
@@ -116,24 +102,14 @@ describe('status command', () => {

  it('shows registries from config', async () => {
    saveConfig({ ...DEFAULT_CONFIG, registries: ['official'] }, { configDir: tempDir });
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
-      checkHealth: async () => true,
-    });
+    const cmd = createStatusCommand(baseDeps());
    await cmd.parseAsync([], { from: 'user' });
    expect(output.join('\n')).toContain('official');
    expect(output.join('\n')).not.toContain('glama');
  });

  it('shows LLM not configured hint when no LLM is set', async () => {
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
-      checkHealth: async () => true,
-    });
+    const cmd = createStatusCommand(baseDeps());
    await cmd.parseAsync([], { from: 'user' });
    const out = output.join('\n');
    expect(out).toContain('LLM:');
@@ -141,71 +117,64 @@ describe('status command', () => {
    expect(out).toContain('mcpctl config setup');
  });

-  it('shows configured LLM provider and model when healthy', async () => {
+  it('shows green check when LLM is healthy (non-TTY)', async () => {
    saveConfig({ ...DEFAULT_CONFIG, llm: { provider: 'anthropic', model: 'claude-haiku-3-5-20241022' } }, { configDir: tempDir });
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
-      checkHealth: async () => true,
-      checkLlm: async () => 'ok',
-    });
+    const cmd = createStatusCommand(baseDeps({ checkLlm: async () => 'ok' }));
    await cmd.parseAsync([], { from: 'user' });
    const out = output.join('\n');
-    expect(out).toContain('LLM:');
    expect(out).toContain('anthropic / claude-haiku-3-5-20241022');
-    // Should NOT show error status when ok
-    expect(out).not.toContain('(ok)');
+    expect(out).toContain('✓ ok');
  });

-  it('shows LLM error status when check fails', async () => {
+  it('shows red cross when LLM check fails (non-TTY)', async () => {
    saveConfig({ ...DEFAULT_CONFIG, llm: { provider: 'gemini-cli', model: 'gemini-2.5-flash' } }, { configDir: tempDir });
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
-      checkHealth: async () => true,
-      checkLlm: async () => 'not authenticated',
-    });
+    const cmd = createStatusCommand(baseDeps({ checkLlm: async () => 'not authenticated' }));
    await cmd.parseAsync([], { from: 'user' });
    const out = output.join('\n');
-    expect(out).toContain('gemini-cli / gemini-2.5-flash (not authenticated)');
+    expect(out).toContain('✗ not authenticated');
  });

-  it('shows binary not found status', async () => {
+  it('shows binary not found error', async () => {
    saveConfig({ ...DEFAULT_CONFIG, llm: { provider: 'gemini-cli', model: 'gemini-2.5-flash' } }, { configDir: tempDir });
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
-      checkHealth: async () => true,
-      checkLlm: async () => 'binary not found',
-    });
+    const cmd = createStatusCommand(baseDeps({ checkLlm: async () => 'binary not found' }));
    await cmd.parseAsync([], { from: 'user' });
-    expect(output.join('\n')).toContain('(binary not found)');
+    expect(output.join('\n')).toContain('✗ binary not found');
+  });
+
+  it('uses spinner on TTY and writes final result', async () => {
+    saveConfig({ ...DEFAULT_CONFIG, llm: { provider: 'gemini-cli', model: 'gemini-2.5-flash' } }, { configDir: tempDir });
+    const cmd = createStatusCommand(baseDeps({
+      isTTY: true,
+      checkLlm: async () => 'ok',
+    }));
+    await cmd.parseAsync([], { from: 'user' });
+    // On TTY, the final LLM line goes through write(), not log()
+    const finalWrite = written[written.length - 1];
+    expect(finalWrite).toContain('gemini-cli / gemini-2.5-flash');
+    expect(finalWrite).toContain('✓ ok');
+  });
+
+  it('uses spinner on TTY and shows failure', async () => {
+    saveConfig({ ...DEFAULT_CONFIG, llm: { provider: 'gemini-cli', model: 'gemini-2.5-flash' } }, { configDir: tempDir });
+    const cmd = createStatusCommand(baseDeps({
+      isTTY: true,
+      checkLlm: async () => 'not authenticated',
+    }));
+    await cmd.parseAsync([], { from: 'user' });
+    const finalWrite = written[written.length - 1];
+    expect(finalWrite).toContain('✗ not authenticated');
  });

  it('shows not configured when LLM provider is none', async () => {
    saveConfig({ ...DEFAULT_CONFIG, llm: { provider: 'none' } }, { configDir: tempDir });
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
-      checkHealth: async () => true,
-    });
+    const cmd = createStatusCommand(baseDeps());
    await cmd.parseAsync([], { from: 'user' });
    expect(output.join('\n')).toContain('not configured');
  });

  it('includes llm and llmStatus in JSON output', async () => {
    saveConfig({ ...DEFAULT_CONFIG, llm: { provider: 'gemini-cli', model: 'gemini-2.5-flash' } }, { configDir: tempDir });
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
-      checkHealth: async () => true,
-      checkLlm: async () => 'ok',
-    });
+    const cmd = createStatusCommand(baseDeps({ checkLlm: async () => 'ok' }));
    await cmd.parseAsync(['-o', 'json'], { from: 'user' });
    const parsed = JSON.parse(output[0]) as Record<string, unknown>;
    expect(parsed['llm']).toBe('gemini-cli / gemini-2.5-flash');
@@ -213,12 +182,7 @@ describe('status command', () => {
  });

  it('includes null llm in JSON output when not configured', async () => {
-    const cmd = createStatusCommand({
-      configDeps: { configDir: tempDir },
-      credentialsDeps: { configDir: tempDir },
-      log,
-      checkHealth: async () => true,
-    });
+    const cmd = createStatusCommand(baseDeps());
    await cmd.parseAsync(['-o', 'json'], { from: 'user' });
    const parsed = JSON.parse(output[0]) as Record<string, unknown>;
    expect(parsed['llm']).toBeNull();