From a95e2a03ac7a30f4ad9a65ae26393596786daab6 Mon Sep 17 00:00:00 2001 From: "credence-the-bot[bot]" <277301654+credence-the-bot[bot]@users.noreply.github.com> Date: Sat, 9 May 2026 21:10:09 +0000 Subject: [PATCH 1/6] =?UTF-8?q?feat(cli):=20registerCommand=20helper=20+?= =?UTF-8?q?=20transport=20tags=20for=2021=20commands=20+=20=C2=A73.7=20dae?= =?UTF-8?q?mon-down=20message=20(#30156)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(cli): registerCommand helper + transport tags for 21 commands + §3.7 daemon-down message * fix(lint): sort imports in cache.ts, clients.ts, register-command.test.ts * fix(tests): use console.log in status.ts; update exit-helper test for §3.7 message * fix(tests): use process.stdout.write in status.ts for testable output * fix: revert daemon terminology per AGENTS.md; use 'the assistant' in user-facing messages; 'Assistant: running/down' in status --------- Co-authored-by: credence-the-bot[bot] --- .../src/cli/commands/__tests__/cache.test.ts | 2 +- .../src/cli/commands/__tests__/status.test.ts | 211 ++++++++++++++++++ assistant/src/cli/commands/attachment.ts | 11 +- assistant/src/cli/commands/autonomy.ts | 14 +- assistant/src/cli/commands/bash.ts | 14 +- assistant/src/cli/commands/browser.ts | 12 +- assistant/src/cli/commands/cache.ts | 13 +- assistant/src/cli/commands/clients.ts | 11 +- assistant/src/cli/commands/completions.ts | 14 +- assistant/src/cli/commands/config.ts | 9 +- .../src/cli/commands/conversations-defer.ts | 12 +- .../src/cli/commands/credential-execution.ts | 15 +- assistant/src/cli/commands/gateway.ts | 9 +- assistant/src/cli/commands/keys.ts | 11 +- assistant/src/cli/commands/mcp.ts | 11 +- assistant/src/cli/commands/memory-v2.ts | 11 +- assistant/src/cli/commands/notifications.ts | 15 +- assistant/src/cli/commands/pending.ts | 16 +- assistant/src/cli/commands/status.ts | 82 ++++--- assistant/src/cli/commands/task.ts | 11 +- assistant/src/cli/commands/trust.ts | 11 +- assistant/src/cli/commands/watchers.ts | 11 +- .../lib/__tests__/register-command.test.ts | 85 +++++++ .../src/cli/{commands => lib}/cache-fs.ts | 0 assistant/src/cli/lib/register-command.ts | 19 ++ 25 files changed, 523 insertions(+), 107 deletions(-) create mode 100644 assistant/src/cli/commands/__tests__/status.test.ts create mode 100644 assistant/src/cli/lib/__tests__/register-command.test.ts rename assistant/src/cli/{commands => lib}/cache-fs.ts (100%) create mode 100644 assistant/src/cli/lib/register-command.ts diff --git a/assistant/src/cli/commands/__tests__/cache.test.ts b/assistant/src/cli/commands/__tests__/cache.test.ts index 012ab51201f..ca58afb0756 100644 --- a/assistant/src/cli/commands/__tests__/cache.test.ts +++ b/assistant/src/cli/commands/__tests__/cache.test.ts @@ -87,7 +87,7 @@ mock.module("../../../util/logger.js", () => ({ }), })); -mock.module("../cache-fs.js", () => ({ +mock.module("../../lib/cache-fs.js", () => ({ readFileSync: (path: string, encoding?: BufferEncoding) => { if (path === "/dev/stdin") { if (mockStdinContent === null) { diff --git a/assistant/src/cli/commands/__tests__/status.test.ts b/assistant/src/cli/commands/__tests__/status.test.ts new file mode 100644 index 00000000000..11c4c429267 --- /dev/null +++ b/assistant/src/cli/commands/__tests__/status.test.ts @@ -0,0 +1,211 @@ +/** + * Tests for the `assistant status` CLI command. + * + * Validates: + * - Successful IPC response shows version, workspace, and runtime health + * - When IPC fails (daemon down), prints "Daemon: down" or "Daemon: running" + * and exits with code 0 (not 1) + */ + +import { beforeEach, describe, expect, mock, test } from "bun:test"; + +import { Command } from "commander"; + +// --------------------------------------------------------------------------- +// Mock state +// --------------------------------------------------------------------------- + +let ipcCalls: Array<{ method: string }> = []; +let mockResponse: { ok: boolean; result?: unknown; error?: string } = { + ok: false, + error: "ENOENT", +}; + +let socketExists = false; + +// --------------------------------------------------------------------------- +// Mocks +// --------------------------------------------------------------------------- + +mock.module("../../../ipc/cli-client.js", () => ({ + cliIpcCall: async (method: string) => { + ipcCalls.push({ method }); + return mockResponse; + }, +})); + +mock.module("../../../ipc/socket-path.js", () => ({ + getAssistantSocketPath: () => "/tmp/test-assistant.sock", +})); + +mock.module("node:fs", () => ({ + existsSync: (_path: string) => socketExists, +})); + +mock.module("../../../util/platform.js", () => ({ + getWorkspaceDirDisplay: () => "~/.vellum/workspace", +})); + +mock.module("../../../util/logger.js", () => ({ + getLogger: () => ({ + info: () => {}, + warn: () => {}, + error: () => {}, + debug: () => {}, + }), + getCliLogger: () => ({ + info: () => {}, + warn: () => {}, + error: () => {}, + debug: () => {}, + }), +})); + +// --------------------------------------------------------------------------- +// Import module under test (after mocks) +// --------------------------------------------------------------------------- + +const { registerStatusCommand } = await import("../status.js"); + +// --------------------------------------------------------------------------- +// Test helper +// --------------------------------------------------------------------------- + +async function runStatusCommand(): Promise<{ + stdout: string; + stderr: string; + exitCode: number; +}> { + const stdoutChunks: string[] = []; + const stderrChunks: string[] = []; + let capturedExitCode = 0; + let exitCalled = false; + + const originalStdoutWrite = process.stdout.write.bind(process.stdout); + const originalStderrWrite = process.stderr.write.bind(process.stderr); + const originalExit = process.exit.bind(process); + + process.stdout.write = ((chunk: unknown) => { + stdoutChunks.push(typeof chunk === "string" ? chunk : String(chunk)); + return true; + }) as typeof process.stdout.write; + + process.stderr.write = ((chunk: unknown) => { + stderrChunks.push(typeof chunk === "string" ? chunk : String(chunk)); + return true; + }) as typeof process.stderr.write; + + // Override process.exit to capture the code instead of terminating + (process as { exit: (code?: number) => never }).exit = ((code?: number) => { + capturedExitCode = code ?? 0; + exitCalled = true; + throw new Error(`process.exit(${code})`); + }) as typeof process.exit; + + process.exitCode = 0; + + try { + const program = new Command(); + program.exitOverride(); + program.configureOutput({ + writeErr: () => {}, + writeOut: (str: string) => stdoutChunks.push(str), + }); + registerStatusCommand(program); + await program.parseAsync(["node", "assistant", "status"]); + } catch (err) { + // Swallow process.exit() throws and commander exitOverride errors + if (err instanceof Error && !err.message.startsWith("process.exit(")) { + // Commander parse errors; ignore + } + } finally { + process.stdout.write = originalStdoutWrite; + process.stderr.write = originalStderrWrite; + (process as { exit: (code?: number) => never }).exit = originalExit; + } + + const exitCode = exitCalled ? capturedExitCode : (process.exitCode ?? 0); + process.exitCode = 0; + + return { + exitCode, + stdout: stdoutChunks.join(""), + stderr: stderrChunks.join(""), + }; +} + +// --------------------------------------------------------------------------- +// Setup +// --------------------------------------------------------------------------- + +beforeEach(() => { + ipcCalls = []; + socketExists = false; + process.exitCode = 0; +}); + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("status command — daemon down", () => { + test("exits with code 0 (not 1) when IPC call fails", async () => { + mockResponse = { ok: false, error: "ENOENT" }; + socketExists = false; + + const { exitCode } = await runStatusCommand(); + + expect(exitCode).toBe(0); + }); + + test('prints "Daemon: down" when socket file does not exist', async () => { + mockResponse = { ok: false, error: "ENOENT" }; + socketExists = false; + + const { stdout, stderr } = await runStatusCommand(); + const combined = stdout + stderr; + + expect(combined).toContain("Assistant: down"); + }); + + test('prints "Daemon: running" when socket file exists but IPC fails', async () => { + mockResponse = { ok: false, error: "Connection closed before response" }; + socketExists = true; + + const { stdout, stderr } = await runStatusCommand(); + const combined = stdout + stderr; + + expect(combined).toContain("Assistant: running"); + }); + + test("does not print version or memory when daemon is down", async () => { + mockResponse = { ok: false, error: "ENOENT" }; + socketExists = false; + + const { stdout, stderr } = await runStatusCommand(); + const combined = stdout + stderr; + + expect(combined).not.toContain("Version"); + expect(combined).not.toContain("Memory"); + }); +}); + +describe("status command — daemon up", () => { + test("shows version and memory when IPC succeeds", async () => { + mockResponse = { + ok: true, + result: { + version: "1.2.3", + memory: { currentMb: 100, maxMb: 500 }, + disk: null, + }, + }; + + const { stdout, stderr } = await runStatusCommand(); + const combined = stdout + stderr; + + expect(combined).toContain("1.2.3"); + expect(combined).toContain("100"); + expect(combined).toContain("500"); + }); +}); diff --git a/assistant/src/cli/commands/attachment.ts b/assistant/src/cli/commands/attachment.ts index 518603bc363..7064aae424f 100644 --- a/assistant/src/cli/commands/attachment.ts +++ b/assistant/src/cli/commands/attachment.ts @@ -8,15 +8,18 @@ import type { Command } from "commander"; import { cliIpcCall } from "../../ipc/cli-client.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; import { shouldOutputJson, writeOutput } from "../output.js"; // ── Registration ────────────────────────────────────────────────────── export function registerAttachmentCommand(program: Command): void { - const attachment = program - .command("attachment") - .description("Manage file attachments for conversations"); + registerCommand(program, { + name: "attachment", + transport: "ipc", + description: "Manage file attachments for conversations", + build: (attachment) => { attachment.addHelpText( "after", @@ -183,4 +186,6 @@ Examples: } }, ); + }, + }); } diff --git a/assistant/src/cli/commands/autonomy.ts b/assistant/src/cli/commands/autonomy.ts index 7d97eff360c..54ba2e862bd 100644 --- a/assistant/src/cli/commands/autonomy.ts +++ b/assistant/src/cli/commands/autonomy.ts @@ -4,6 +4,7 @@ import { dirname, join } from "node:path"; import type { Command } from "commander"; import { getWorkspaceDir } from "../../util/platform.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; // --------------------------------------------------------------------------- @@ -173,10 +174,13 @@ function formatConfigForHuman(config: AutonomyConfig): string { // --------------------------------------------------------------------------- export function registerAutonomyCommand(program: Command): void { - const autonomy = program - .command("autonomy") - .description("View and configure autonomy tiers") - .option("--json", "Machine-readable JSON output"); + // JARVIS-745: autonomy.json is file-write-only; no daemon consumer. Tagged local until resolved. + registerCommand(program, { + name: "autonomy", + transport: "local", + description: "View and configure autonomy tiers", + build: (autonomy) => { + autonomy.option("--json", "Machine-readable JSON output"); autonomy.addHelpText( "after", @@ -362,4 +366,6 @@ Examples: process.exitCode = 1; }, ); + }, + }); } diff --git a/assistant/src/cli/commands/bash.ts b/assistant/src/cli/commands/bash.ts index 00bcd9da011..59a4c45ad20 100644 --- a/assistant/src/cli/commands/bash.ts +++ b/assistant/src/cli/commands/bash.ts @@ -1,6 +1,7 @@ import type { Command } from "commander"; import { cliIpcCall } from "../../ipc/cli-client.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; const DEFAULT_TIMEOUT_MS = 30_000; @@ -14,11 +15,12 @@ interface DebugBashResult { } export function registerBashCommand(program: Command): void { - program - .command("bash ") - .description( - "Execute a shell command through the assistant process for debugging", - ) + registerCommand(program, { + name: "bash ", + transport: "ipc", + description: "Execute a shell command through the assistant process for debugging", + build: (cmd) => { + cmd .option( "-t, --timeout ", "Timeout in milliseconds for command execution", @@ -100,4 +102,6 @@ Examples: process.exitCode = data.exitCode ?? 1; }); + }, + }); } diff --git a/assistant/src/cli/commands/browser.ts b/assistant/src/cli/commands/browser.ts index 37ef96bc34a..56fc0d4a192 100644 --- a/assistant/src/cli/commands/browser.ts +++ b/assistant/src/cli/commands/browser.ts @@ -17,6 +17,7 @@ import type { OperationField, } from "../../browser/types.js"; import { cliIpcCall } from "../../ipc/cli-client.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; // ── Naming helpers ─────────────────────────────────────────────────── @@ -357,9 +358,12 @@ const BROWSER_MODES = [ ] as const; export function registerBrowserCommand(program: Command): void { - const browser = program - .command("browser") - .description("Control the browser via the running assistant.") + registerCommand(program, { + name: "browser", + transport: "ipc", + description: "Control the browser via the running assistant.", + build: (browser) => { + browser .option( "--session ", "Session ID to preserve browser state across invocations.", @@ -408,4 +412,6 @@ Examples: for (const meta of BROWSER_OPERATION_META) { buildSubcommand(browser, meta); } + }, + }); } diff --git a/assistant/src/cli/commands/cache.ts b/assistant/src/cli/commands/cache.ts index d0c746341b7..08cedeec791 100644 --- a/assistant/src/cli/commands/cache.ts +++ b/assistant/src/cli/commands/cache.ts @@ -8,8 +8,9 @@ import type { Command } from "commander"; import { cliIpcCall } from "../../ipc/cli-client.js"; +import { existsSync, readFileSync } from "../lib/cache-fs.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; -import { existsSync, readFileSync } from "./cache-fs.js"; // ── Constants ───────────────────────────────────────────────────────── @@ -167,9 +168,11 @@ function resolvePayload(opts: { value?: string; file?: string }): unknown { // ── Registration ────────────────────────────────────────────────────── export function registerCacheCommand(program: Command): void { - const cache = program - .command("cache") - .description("Interact with the assistant's in-memory key/value cache"); + registerCommand(program, { + name: "cache", + transport: "ipc", + description: "Interact with the assistant's in-memory key/value cache", + build: (cache) => { cache.addHelpText( "after", @@ -411,4 +414,6 @@ Examples: } } }); + }, + }); } diff --git a/assistant/src/cli/commands/clients.ts b/assistant/src/cli/commands/clients.ts index 1e70326c19d..40b5e06a66c 100644 --- a/assistant/src/cli/commands/clients.ts +++ b/assistant/src/cli/commands/clients.ts @@ -2,6 +2,7 @@ import type { Command } from "commander"; import { cliIpcCall } from "../../ipc/cli-client.js"; import { optsToQueryParams } from "../lib/ipc-params.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; import { writeOutput } from "../output.js"; @@ -23,9 +24,11 @@ interface DisconnectClientResponse { } export function registerClientsCommand(program: Command): void { - const clients = program - .command("clients") - .description("Discover and manage connected clients"); + registerCommand(program, { + name: "clients", + transport: "ipc", + description: "Discover and manage connected clients", + build: (clients) => { clients.addHelpText( "after", @@ -178,6 +181,8 @@ $ assistant clients disconnect a1a30bde-6679-406c-bc32-d5a0d2a7a99e --json`, ); }, ); + }, + }); } function formatRelativeTime(iso: string): string { diff --git a/assistant/src/cli/commands/completions.ts b/assistant/src/cli/commands/completions.ts index 89f5040cdf4..39ea3dcd0e7 100644 --- a/assistant/src/cli/commands/completions.ts +++ b/assistant/src/cli/commands/completions.ts @@ -1,14 +1,16 @@ import type { Command } from "commander"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; export function registerCompletionsCommand(program: Command): void { - program - .command("completions") + registerCommand(program, { + name: "completions", + transport: "local", + description: "Generate shell completion script (e.g. assistant completions bash >> ~/.bashrc)", + build: (cmd) => { + cmd .argument("", "Shell type: bash, zsh, or fish") - .description( - "Generate shell completion script (e.g. assistant completions bash >> ~/.bashrc)", - ) .addHelpText( "after", ` @@ -73,6 +75,8 @@ Examples: process.exit(1); } }); + }, + }); } function generateBashCompletion( diff --git a/assistant/src/cli/commands/config.ts b/assistant/src/cli/commands/config.ts index a7b91e2e8e1..23af1af0ce9 100644 --- a/assistant/src/cli/commands/config.ts +++ b/assistant/src/cli/commands/config.ts @@ -9,6 +9,7 @@ import { } from "../../config/loader.js"; import { AssistantConfigSchema } from "../../config/schema.js"; import { getSchemaAtPath } from "../../config/schema-utils.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; import { requirePlatformConnection } from "./oauth/shared.js"; @@ -39,7 +40,11 @@ function flattenConfig( const SERVICE_MODE_PATH_RE = /^services\.[^.]+\.mode$/; export function registerConfigCommand(program: Command): void { - const config = program.command("config").description("Manage configuration"); + registerCommand(program, { + name: "config", + transport: "local", + description: "Manage configuration", + build: (config) => { config.addHelpText( "after", @@ -277,4 +282,6 @@ Examples: process.exit(1); } }); + }, + }); } diff --git a/assistant/src/cli/commands/conversations-defer.ts b/assistant/src/cli/commands/conversations-defer.ts index 0bbf84dedc5..106c344d182 100644 --- a/assistant/src/cli/commands/conversations-defer.ts +++ b/assistant/src/cli/commands/conversations-defer.ts @@ -1,6 +1,7 @@ import type { Command } from "commander"; import { cliIpcCall } from "../../ipc/cli-client.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; import { resolveConversationId } from "../utils/conversation-id.js"; import { parseDuration } from "../utils/parse-duration.js"; @@ -12,9 +13,12 @@ export { parseDuration }; // --------------------------------------------------------------------------- export function registerConversationsDeferCommand(parent: Command): void { - const defer = parent - .command("defer [conversationId]") - .description("Create a deferred wake for a conversation") + registerCommand(parent, { + name: "defer [conversationId]", + transport: "ipc", + description: "Create a deferred wake for a conversation", + build: (defer) => { + defer .option("--in ", "Delay before firing (e.g. 60, 60s, 5m, 1h)") .option("--at ", "Absolute ISO 8601 fire time") .option("--hint ", "Hint message for the wake") @@ -303,4 +307,6 @@ Examples: } }, ); + }, + }); } diff --git a/assistant/src/cli/commands/credential-execution.ts b/assistant/src/cli/commands/credential-execution.ts index fef4905533b..d618d792861 100644 --- a/assistant/src/cli/commands/credential-execution.ts +++ b/assistant/src/cli/commands/credential-execution.ts @@ -29,6 +29,7 @@ import { } from "../../credential-execution/client.js"; import { isCesGrantAuditEnabled } from "../../credential-execution/feature-gates.js"; import { createCesProcessManager } from "../../credential-execution/process-manager.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; import { shouldOutputJson, writeOutput } from "../output.js"; @@ -122,12 +123,12 @@ function printAuditRecordHuman(record: AuditRecordSummary): void { // --------------------------------------------------------------------------- export function registerCredentialExecutionCommand(program: Command): void { - const ce = program - .command("credential-execution") - .description( - "Inspect and manage Credential Execution Service (CES) grants and audit records", - ) - .option("--json", "Machine-readable compact JSON output"); + registerCommand(program, { + name: "credential-execution", + transport: "local", + description: "Inspect and manage Credential Execution Service (CES) grants and audit records", + build: (ce) => { + ce.option("--json", "Machine-readable compact JSON output"); ce.addHelpText( "after", @@ -356,4 +357,6 @@ Examples: } }, ); + }, + }); } diff --git a/assistant/src/cli/commands/gateway.ts b/assistant/src/cli/commands/gateway.ts index dbe66f0b8cc..a015fd1c8b5 100644 --- a/assistant/src/cli/commands/gateway.ts +++ b/assistant/src/cli/commands/gateway.ts @@ -8,6 +8,7 @@ import type { Command } from "commander"; import { cliIpcCall } from "../../ipc/cli-client.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; // -- Types -------------------------------------------------------------------- @@ -56,7 +57,11 @@ function colorLevel(name: string, levelNum: number): string { // -- Registration ------------------------------------------------------------- export function registerGatewayCommand(program: Command): void { - const gateway = program.command("gateway").description("Gateway management"); + registerCommand(program, { + name: "gateway", + transport: "ipc", + description: "Gateway management", + build: (gateway) => { gateway.addHelpText( "after", @@ -180,4 +185,6 @@ Examples: process.stdout.write(dim + "\n"); } }); + }, + }); } diff --git a/assistant/src/cli/commands/keys.ts b/assistant/src/cli/commands/keys.ts index ef2a8d3aa0f..74f7215ac15 100644 --- a/assistant/src/cli/commands/keys.ts +++ b/assistant/src/cli/commands/keys.ts @@ -7,6 +7,7 @@ import { deleteSecureKeyViaDaemon, setSecureKeyViaDaemon, } from "../lib/daemon-credential-client.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; // --------------------------------------------------------------------------- @@ -28,9 +29,11 @@ const UNTRUSTED_SHELL_ERROR = "API key management is restricted when running under CES shell lockdown."; export function registerKeysCommand(program: Command): void { - const keys = program - .command("keys") - .description("Manage API keys in secure storage"); + registerCommand(program, { + name: "keys", + transport: "local", + description: "Manage API keys in secure storage", + build: (keys) => { keys.addHelpText( "after", @@ -147,4 +150,6 @@ Examples: process.exit(1); } }); + }, + }); } diff --git a/assistant/src/cli/commands/mcp.ts b/assistant/src/cli/commands/mcp.ts index ac460b40c85..bf9d8aebbf6 100644 --- a/assistant/src/cli/commands/mcp.ts +++ b/assistant/src/cli/commands/mcp.ts @@ -6,6 +6,7 @@ import { cliIpcCall } from "../../ipc/cli-client.js"; import { McpClient } from "../../mcp/client.js"; import { deleteMcpOAuthCredentials } from "../../mcp/mcp-oauth-provider.js"; import { openInHostBrowser } from "../../util/browser.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; const HEALTH_CHECK_TIMEOUT_MS = 10_000; @@ -102,9 +103,11 @@ export async function checkServerHealth( } export function registerMcpCommand(program: Command): void { - const mcp = program - .command("mcp") - .description("Manage MCP (Model Context Protocol) servers"); + registerCommand(program, { + name: "mcp", + transport: "ipc", + description: "Manage MCP (Model Context Protocol) servers", + build: (mcp) => { mcp.addHelpText( "after", @@ -616,4 +619,6 @@ Examples: "Or run 'vellum mcp reload' to apply now.", ); }); + }, + }); } diff --git a/assistant/src/cli/commands/memory-v2.ts b/assistant/src/cli/commands/memory-v2.ts index ddc777439e1..6cc48d14bdc 100644 --- a/assistant/src/cli/commands/memory-v2.ts +++ b/assistant/src/cli/commands/memory-v2.ts @@ -43,6 +43,7 @@ import type { MemoryV2ReembedSkillsResult, MemoryV2ValidateResult, } from "../../runtime/routes/memory-v2-routes.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; // --------------------------------------------------------------------------- @@ -163,9 +164,11 @@ export function registerMemoryV2Command(program: Command): void { ); } - const v2 = memory - .command("v2") - .description("Memory v2 subsystem operations (concept-page model)"); + registerCommand(memory, { + name: "v2", + transport: "ipc", + description: "Memory v2 subsystem operations (concept-page model)", + build: (v2) => { v2.addHelpText( "after", @@ -595,4 +598,6 @@ Examples: } } }); + }, + }); } diff --git a/assistant/src/cli/commands/notifications.ts b/assistant/src/cli/commands/notifications.ts index 4d24bf31ea3..6a157887a8f 100644 --- a/assistant/src/cli/commands/notifications.ts +++ b/assistant/src/cli/commands/notifications.ts @@ -1,6 +1,7 @@ import type { Command } from "commander"; import { cliIpcCall } from "../../ipc/cli-client.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; import { shouldOutputJson, writeOutput } from "../output.js"; @@ -9,12 +10,12 @@ import { shouldOutputJson, writeOutput } from "../output.js"; // --------------------------------------------------------------------------- export function registerNotificationsCommand(program: Command): void { - const notifications = program - .command("notifications") - .description( - "Send and inspect notifications through the unified notification router", - ) - .option("--json", "Machine-readable compact JSON output"); + registerCommand(program, { + name: "notifications", + transport: "ipc", + description: "Send and inspect notifications through the unified notification router", + build: (notifications) => { + notifications.option("--json", "Machine-readable compact JSON output"); notifications.addHelpText( "after", @@ -396,4 +397,6 @@ Examples: } }, ); + }, + }); } diff --git a/assistant/src/cli/commands/pending.ts b/assistant/src/cli/commands/pending.ts index 64f89525ebe..d41bbaf942c 100644 --- a/assistant/src/cli/commands/pending.ts +++ b/assistant/src/cli/commands/pending.ts @@ -1,6 +1,7 @@ import type { Command } from "commander"; import { cliIpcCall } from "../../ipc/cli-client.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; interface PendingInteractionEntry { @@ -16,13 +17,12 @@ interface PendingInteractionsResponse { } export function registerPendingCommand(program: Command): void { - const pending = program - .command("pending") - .description( - "Inspect pending interactions (confirmations, secrets, host proxy requests)", - ); - - pending + registerCommand(program, { + name: "pending", + transport: "ipc", + description: "Inspect pending interactions (confirmations, secrets, host proxy requests)", + build: (pending) => { + pending .command("list") .alias("ls") .description("List all pending interactions across all conversations") @@ -99,4 +99,6 @@ export function registerPendingCommand(program: Command): void { } }, ); + }, + }); } diff --git a/assistant/src/cli/commands/status.ts b/assistant/src/cli/commands/status.ts index 3bb402d5103..53ac9e551fa 100644 --- a/assistant/src/cli/commands/status.ts +++ b/assistant/src/cli/commands/status.ts @@ -1,8 +1,11 @@ +import { existsSync } from "node:fs"; + import type { Command } from "commander"; import { cliIpcCall } from "../../ipc/cli-client.js"; +import { getAssistantSocketPath } from "../../ipc/socket-path.js"; import { getWorkspaceDirDisplay } from "../../util/platform.js"; -import { log } from "../logger.js"; +import { registerCommand } from "../lib/register-command.js"; interface HealthResponse { version: string; @@ -16,42 +19,47 @@ function fmtMb(mb: number): string { } export function registerStatusCommand(program: Command): void { - program - .command("status") - .description("Show assistant version, workspace, and runtime health") - .action(async () => { - const result = await cliIpcCall("health"); - - if (!result.ok || !result.result) { - log.error( - result.error ?? - "Assistant not running — could not connect to IPC socket.", + registerCommand(program, { + name: "status", + transport: "ipc", + description: "Show assistant version, workspace, and runtime health", + build: (cmd) => { + cmd.action(async () => { + const result = await cliIpcCall("health"); + + if (!result.ok || !result.result) { + const socketPath = getAssistantSocketPath(); + const socketExists = existsSync(socketPath); + const workspace = getWorkspaceDirDisplay(); + process.stdout.write((socketExists ? "Assistant: running" : "Assistant: down") + "\n"); + process.stdout.write(`Workspace: ${workspace}\n`); + process.exit(0); + } + + const h = result.result; + const workspace = getWorkspaceDirDisplay(); + + const rows: [string, string][] = [ + ["Version", h.version], + ["Workspace", workspace], + ["", ""], + ["Memory", `${fmtMb(h.memory.currentMb)} / ${fmtMb(h.memory.maxMb)}`], + ...(h.disk + ? ([["Disk", `${fmtMb(h.disk.freeMb)} free`]] as [string, string][]) + : []), + ]; + + const labelWidth = Math.max( + ...rows.filter(([l]) => l).map(([l]) => l.length), ); - process.exit(1); - } - - const h = result.result; - const workspace = getWorkspaceDirDisplay(); - - const rows: [string, string][] = [ - ["Version", h.version], - ["Workspace", workspace], - ["", ""], - ["Memory", `${fmtMb(h.memory.currentMb)} / ${fmtMb(h.memory.maxMb)}`], - ...(h.disk - ? ([["Disk", `${fmtMb(h.disk.freeMb)} free`]] as [string, string][]) - : []), - ]; - - const labelWidth = Math.max( - ...rows.filter(([l]) => l).map(([l]) => l.length), - ); - for (const [label, value] of rows) { - if (!label) { - log.info(""); - continue; + for (const [label, value] of rows) { + if (!label) { + process.stdout.write("\n"); + continue; + } + process.stdout.write(`${label.padEnd(labelWidth)} ${value}\n`); } - log.info(`${label.padEnd(labelWidth)} ${value}`); - } - }); + }); + }, + }); } diff --git a/assistant/src/cli/commands/task.ts b/assistant/src/cli/commands/task.ts index 36c65f14f3a..3d89daf97f9 100644 --- a/assistant/src/cli/commands/task.ts +++ b/assistant/src/cli/commands/task.ts @@ -9,15 +9,18 @@ import type { Command } from "commander"; import { cliIpcCall } from "../../ipc/cli-client.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; import { resolveConversationId } from "../utils/conversation-id.js"; // ── Registration ────────────────────────────────────────────────────── export function registerTaskCommand(program: Command): void { - const task = program - .command("task") - .description("Manage task templates and work queue items"); + registerCommand(program, { + name: "task", + transport: "ipc", + description: "Manage task templates and work queue items", + build: (task) => { task.addHelpText( "after", @@ -763,4 +766,6 @@ Examples: } }, ); + }, + }); } diff --git a/assistant/src/cli/commands/trust.ts b/assistant/src/cli/commands/trust.ts index 32da4e5ca01..f2934d8ea97 100644 --- a/assistant/src/cli/commands/trust.ts +++ b/assistant/src/cli/commands/trust.ts @@ -8,6 +8,7 @@ import type { Command } from "commander"; import { cliIpcCall } from "../../ipc/cli-client.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; // -- Types -------------------------------------------------------------------- @@ -25,9 +26,11 @@ interface TrustRule { // -- Registration ------------------------------------------------------------- export function registerTrustCommand(program: Command): void { - const trust = program - .command("trust") - .description("View tool trust rules (allow-list patterns for tool use)"); + registerCommand(program, { + name: "trust", + transport: "ipc", + description: "View tool trust rules (allow-list patterns for tool use)", + build: (trust) => { trust.addHelpText( "after", @@ -139,4 +142,6 @@ Examples: ); } }); + }, + }); } diff --git a/assistant/src/cli/commands/watchers.ts b/assistant/src/cli/commands/watchers.ts index 7b01152698b..d2c3c83b773 100644 --- a/assistant/src/cli/commands/watchers.ts +++ b/assistant/src/cli/commands/watchers.ts @@ -9,6 +9,7 @@ import type { Command } from "commander"; import { cliIpcCall } from "../../ipc/cli-client.js"; +import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; // -- Types for IPC results ---------------------------------------------------- @@ -37,9 +38,11 @@ interface WatcherEvent { // -- Registration ------------------------------------------------------------- export function registerWatchersCommand(program: Command): void { - const watchers = program - .command("watchers") - .description("Manage polling watchers that monitor external services"); + registerCommand(program, { + name: "watchers", + transport: "ipc", + description: "Manage polling watchers that monitor external services", + build: (watchers) => { watchers.addHelpText( "after", @@ -506,4 +509,6 @@ Examples: } }, ); + }, + }); } diff --git a/assistant/src/cli/lib/__tests__/register-command.test.ts b/assistant/src/cli/lib/__tests__/register-command.test.ts new file mode 100644 index 00000000000..2527df68f00 --- /dev/null +++ b/assistant/src/cli/lib/__tests__/register-command.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, test } from "bun:test"; + +import { Command } from "commander"; + +import { type CommandTransport, registerCommand } from "../register-command.js"; + +describe("registerCommand", () => { + test("registers a command with the correct name and description", () => { + const parent = new Command(); + const result = registerCommand(parent, { + name: "test-cmd", + transport: "ipc", + description: "A test command", + build: () => {}, + }); + + expect(result.name()).toBe("test-cmd"); + expect(result.description()).toBe("A test command"); + }); + + test("calls build with the command instance", () => { + const parent = new Command(); + let receivedCmd: Command | undefined; + + registerCommand(parent, { + name: "my-cmd", + transport: "local", + description: "My command", + build: (cmd) => { + receivedCmd = cmd; + }, + }); + + expect(receivedCmd).toBeDefined(); + expect(receivedCmd!.name()).toBe("my-cmd"); + }); + + test("returns the registered command", () => { + const parent = new Command(); + const result = registerCommand(parent, { + name: "ret-cmd", + transport: "bootstrap", + description: "Return test", + build: () => {}, + }); + + expect(result).toBeInstanceOf(Command); + expect(result.name()).toBe("ret-cmd"); + }); + + test.each([ + ["ipc" as CommandTransport], + ["local" as CommandTransport], + ["bootstrap" as CommandTransport], + ])("accepts transport value: %s", (transport) => { + const parent = new Command(); + let buildCalled = false; + + const result = registerCommand(parent, { + name: `cmd-${transport}`, + transport, + description: `Command with transport ${transport}`, + build: () => { + buildCalled = true; + }, + }); + + expect(buildCalled).toBe(true); + expect(result.name()).toBe(`cmd-${transport}`); + expect(result.description()).toBe(`Command with transport ${transport}`); + }); + + test("attaches the command as a subcommand of parent", () => { + const parent = new Command("program"); + registerCommand(parent, { + name: "sub", + transport: "ipc", + description: "Subcommand", + build: () => {}, + }); + + const names = parent.commands.map((c) => c.name()); + expect(names).toContain("sub"); + }); +}); diff --git a/assistant/src/cli/commands/cache-fs.ts b/assistant/src/cli/lib/cache-fs.ts similarity index 100% rename from assistant/src/cli/commands/cache-fs.ts rename to assistant/src/cli/lib/cache-fs.ts diff --git a/assistant/src/cli/lib/register-command.ts b/assistant/src/cli/lib/register-command.ts new file mode 100644 index 00000000000..255e3f6d752 --- /dev/null +++ b/assistant/src/cli/lib/register-command.ts @@ -0,0 +1,19 @@ +import type { Command } from "commander"; + +export type CommandTransport = "ipc" | "local" | "bootstrap"; + +interface RegisterCommandOpts { + name: string; + transport: CommandTransport; + description: string; + build: (cmd: Command) => void; +} + +export function registerCommand( + parent: Command, + opts: RegisterCommandOpts, +): Command { + const cmd = parent.command(opts.name).description(opts.description); + opts.build(cmd); + return cmd; +} From a686a944786417f0cd5f3c15a81a2c6947108e7c Mon Sep 17 00:00:00 2001 From: "credence-the-bot[bot]" <277301654+credence-the-bot[bot]@users.noreply.github.com> Date: Sat, 9 May 2026 21:33:15 +0000 Subject: [PATCH 2/6] feat(cli): advisory ESLint rule cli/no-daemon-internals (PR #30157) * feat(cli): advisory ESLint rule cli/no-daemon-internals * fix(lint): convert ESLint rule to plain JS so Node.js ESM loader can find it Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: credence-the-bot[bot] Co-authored-by: Claude Sonnet 4.6 --- .../__tests__/cli-no-daemon-internals.test.ts | 170 +++++++++++++++++ .../eslint-rules/cli-no-daemon-internals.js | 176 ++++++++++++++++++ assistant/eslint.config.mjs | 8 + 3 files changed, 354 insertions(+) create mode 100644 assistant/eslint-rules/__tests__/cli-no-daemon-internals.test.ts create mode 100644 assistant/eslint-rules/cli-no-daemon-internals.js diff --git a/assistant/eslint-rules/__tests__/cli-no-daemon-internals.test.ts b/assistant/eslint-rules/__tests__/cli-no-daemon-internals.test.ts new file mode 100644 index 00000000000..8ca234071d8 --- /dev/null +++ b/assistant/eslint-rules/__tests__/cli-no-daemon-internals.test.ts @@ -0,0 +1,170 @@ +import tsParser from "@typescript-eslint/parser"; +import { RuleTester } from "eslint"; + +import rule from "../cli-no-daemon-internals.js"; + +const tester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: "module", + parser: tsParser, + }, +}); + +tester.run("cli/no-daemon-internals", rule, { + valid: [ + // ipc-tagged file importing only allowed sources + { + code: ` + import type { Command } from "commander"; + import { cliIpcCall } from "../../ipc/cli-client.js"; + import { log } from "../logger.js"; + import { printTable } from "../output.js"; + + registerCommand(program, { + name: "example", + transport: "ipc", + build: () => {}, + }); + `, + }, + // local-tagged file importing allowed sources + { + code: ` + import type { Command } from "commander"; + import { loadRawConfig } from "../../config/loader.js"; + import { getWorkspaceDir } from "../../util/platform.js"; + + registerCommand(program, { + name: "local-example", + transport: "local", + build: () => {}, + }); + `, + }, + // bootstrap-tagged file importing allowed sources + { + code: ` + import type { Command } from "commander"; + import { AssistantConfigSchema } from "../../config/schema.js"; + + registerCommand(program, { + name: "bootstrap-example", + transport: "bootstrap", + build: () => {}, + }); + `, + }, + // ipc-tagged file importing from ../lib/ prefix (shared lib) + { + code: ` + import type { Command } from "commander"; + import { cliIpcCall } from "../../ipc/cli-client.js"; + import { readFileSync } from "../lib/daemon-credential-client.js"; + + registerCommand(program, { + name: "lib-example", + transport: "ipc", + build: () => {}, + }); + `, + }, + // File with zero imports and no registerCommand — utility file + { + code: ` + function utilHelper() { + return 42; + } + export { utilHelper }; + `, + }, + ], + + invalid: [ + // File with imports but no registerCommand call + { + code: ` + import type { Command } from "commander"; + import { cliIpcCall } from "../../ipc/cli-client.js"; + + export function registerMyCommand(program) { + // forgot to call registerCommand + } + `, + errors: [ + { + messageId: "missingTransport", + }, + ], + }, + // ipc-tagged file importing a forbidden runtime route + { + code: ` + import type { Command } from "commander"; + import { cliIpcCall } from "../../ipc/cli-client.js"; + import { healthRoutes } from "../../runtime/routes/health-routes.js"; + + registerCommand(program, { + name: "bad-ipc", + transport: "ipc", + build: () => {}, + }); + `, + errors: [ + { + messageId: "forbiddenImport", + data: { + transport: "ipc", + source: "../../runtime/routes/health-routes.js", + }, + }, + ], + }, + // ipc-tagged file importing a skills catalog module + { + code: ` + import type { Command } from "commander"; + import { cliIpcCall } from "../../ipc/cli-client.js"; + import { SkillsCatalog } from "../../skills/catalog.js"; + + registerCommand(program, { + name: "bad-ipc-skills", + transport: "ipc", + build: () => {}, + }); + `, + errors: [ + { + messageId: "forbiddenImport", + data: { + transport: "ipc", + source: "../../skills/catalog.js", + }, + }, + ], + }, + // local-tagged file importing a forbidden runtime route + { + code: ` + import type { Command } from "commander"; + import { loadRawConfig } from "../../config/loader.js"; + import { healthRoutes } from "../../runtime/routes/health-routes.js"; + + registerCommand(program, { + name: "bad-local", + transport: "local", + build: () => {}, + }); + `, + errors: [ + { + messageId: "forbiddenImport", + data: { + transport: "local", + source: "../../runtime/routes/health-routes.js", + }, + }, + ], + }, + ], +}); diff --git a/assistant/eslint-rules/cli-no-daemon-internals.js b/assistant/eslint-rules/cli-no-daemon-internals.js new file mode 100644 index 00000000000..60572ba9029 --- /dev/null +++ b/assistant/eslint-rules/cli-no-daemon-internals.js @@ -0,0 +1,176 @@ +const ALLOWED_PREFIXES = { + ipc: [ + "node:", + "bun:", + "commander", + "../../ipc/cli-client", + "../logger", + "../output", + "../lib/", + ], + local: [ + "node:", + "bun:", + "commander", + "../../config/loader", + "../../config/schema", + "../../util/platform", + "../lib/", + ], + bootstrap: [ + "node:", + "bun:", + "commander", + "../../config/loader", + "../../config/schema", + "../../util/platform", + "../lib/", + ], +}; + +function findTransport(program) { + const worklist = [...program.body]; + const seen = new WeakSet(); + + while (worklist.length > 0) { + const node = worklist.pop(); + + if (!node || typeof node !== "object" || seen.has(node)) { + continue; + } + seen.add(node); + + if ( + node.type === "CallExpression" && + node.callee.type === "Identifier" && + node.callee.name === "registerCommand" + ) { + for (const arg of node.arguments) { + if (arg.type === "ObjectExpression") { + for (const prop of arg.properties) { + if ( + prop.type === "Property" && + prop.key.type === "Identifier" && + prop.key.name === "transport" && + prop.value.type === "Literal" && + typeof prop.value.value === "string" + ) { + return prop.value.value; + } + } + } + } + } + + switch (node.type) { + case "ExpressionStatement": + worklist.push(node.expression); + break; + case "CallExpression": + for (const arg of node.arguments) { + worklist.push(arg); + } + break; + case "FunctionDeclaration": + case "FunctionExpression": + case "ArrowFunctionExpression": + if (node.body) worklist.push(node.body); + break; + case "BlockStatement": + for (const stmt of node.body) { + worklist.push(stmt); + } + break; + case "ReturnStatement": + if (node.argument) worklist.push(node.argument); + break; + case "ExportNamedDeclaration": + case "ExportDefaultDeclaration": + if (node.declaration) worklist.push(node.declaration); + break; + case "VariableDeclaration": + for (const decl of node.declarations) { + if (decl.init) worklist.push(decl.init); + } + break; + case "ObjectExpression": + for (const prop of node.properties) { + if (prop.type === "Property") { + worklist.push(prop.value); + } + } + break; + default: + break; + } + } + + return null; +} + +const rule = { + meta: { + type: "suggestion", + docs: { + description: + "Enforce import allowlists for CLI commands by transport class", + }, + messages: { + missingTransport: + "CLI command file must call registerCommand({ transport: ... }) to declare its transport class.", + forbiddenImport: + "'{{transport}}'-tagged CLI command imports forbidden module '{{source}}'. See DESIGN.md §3.1 for allowed imports.", + }, + schema: [], + }, + + create(context) { + const importNodes = []; + + return { + ImportDeclaration(node) { + importNodes.push(node); + }, + + "Program:exit"(program) { + if (importNodes.length === 0) { + return; + } + + const transport = findTransport(program); + + if (transport === null) { + context.report({ + node: program, + messageId: "missingTransport", + }); + return; + } + + const allowedPrefixes = ALLOWED_PREFIXES[transport]; + if (!allowedPrefixes) { + return; + } + + for (const importNode of importNodes) { + const source = importNode.source.value; + const allowed = allowedPrefixes.some((prefix) => + source.startsWith(prefix), + ); + if (!allowed) { + context.report({ + node: importNode, + messageId: "forbiddenImport", + data: { + transport, + source, + }, + }); + } + } + }, + }; + }, +}; + +export default rule; diff --git a/assistant/eslint.config.mjs b/assistant/eslint.config.mjs index fd5efa4cef8..5ed8b08912c 100644 --- a/assistant/eslint.config.mjs +++ b/assistant/eslint.config.mjs @@ -2,6 +2,8 @@ import { defineConfig, globalIgnores } from "eslint/config"; import simpleImportSort from "eslint-plugin-simple-import-sort"; import tseslint from "typescript-eslint"; +import cliNoDaemonInternals from "./eslint-rules/cli-no-daemon-internals.js"; + const eslintConfig = defineConfig([ ...tseslint.configs.recommended, globalIgnores(["dist/**", "drizzle/**"]), @@ -43,6 +45,12 @@ const eslintConfig = defineConfig([ "@typescript-eslint/no-explicit-any": "off", }, }, + { + files: ["src/cli/commands/**/*.ts"], + ignores: ["src/cli/commands/**/__tests__/**"], + plugins: { cli: { rules: { "no-daemon-internals": cliNoDaemonInternals } } }, + rules: { "cli/no-daemon-internals": "warn" }, + }, ]); export default eslintConfig; From 8869493c1303ad4e4510ff75c6ac3ab5946d0079 Mon Sep 17 00:00:00 2001 From: "credence-the-bot[bot]" <277301654+credence-the-bot[bot]@users.noreply.github.com> Date: Sat, 9 May 2026 21:37:08 +0000 Subject: [PATCH 3/6] feat(cli): COMMAND_INVENTORY.md + bidirectional CI check + AGENTS.md canonical example (PR #30158) * feat(cli): COMMAND_INVENTORY.md + bidirectional CI check + AGENTS.md canonical example Co-Authored-By: Claude Sonnet 4.6 * fix(lint): sort named imports alphabetically in check-cli-inventory.ts Co-Authored-By: Claude Sonnet 4.6 * fix(lint): correct named import sort order in check-cli-inventory.ts Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: credence-the-bot[bot] Co-authored-by: Claude Sonnet 4.6 --- assistant/package.json | 1 + assistant/scripts/check-cli-inventory.ts | 181 +++++++++++++++++++++++ assistant/src/cli/AGENTS.md | 34 ++++- assistant/src/cli/COMMAND_INVENTORY.md | 76 ++++++++++ 4 files changed, 288 insertions(+), 4 deletions(-) create mode 100644 assistant/scripts/check-cli-inventory.ts create mode 100644 assistant/src/cli/COMMAND_INVENTORY.md diff --git a/assistant/package.json b/assistant/package.json index 632313ea531..518e900bd55 100644 --- a/assistant/package.json +++ b/assistant/package.json @@ -20,6 +20,7 @@ "lint": "eslint", "lint:unused": "knip --include files,dependencies,unlisted", "lint:circular": "bun run scripts/check-circular-deps.ts", + "lint:inventory": "bun run scripts/check-cli-inventory.ts", "lint:unused:production": "knip --production --include exports", "typecheck": "bunx tsc --noEmit", "test": "bash scripts/test.sh", diff --git a/assistant/scripts/check-cli-inventory.ts b/assistant/scripts/check-cli-inventory.ts new file mode 100644 index 00000000000..e1f2062e5f1 --- /dev/null +++ b/assistant/scripts/check-cli-inventory.ts @@ -0,0 +1,181 @@ +#!/usr/bin/env bun +/** + * Bidirectional check: every .ts file under src/cli/commands/ (excl. __tests__/) + * must appear in COMMAND_INVENTORY.md and vice versa. + * + * Usage: + * bun run lint:inventory # exits 1 when mismatch exists + */ + +import { existsSync, readdirSync, readFileSync } from "node:fs"; +import { join, relative, resolve } from "node:path"; + +const assistantRoot = resolve(import.meta.dirname, ".."); +const commandsDir = resolve(assistantRoot, "src/cli/commands"); +const inventoryPath = resolve(assistantRoot, "src/cli/COMMAND_INVENTORY.md"); + +// --------------------------------------------------------------------------- +// 1. Check that COMMAND_INVENTORY.md exists +// --------------------------------------------------------------------------- + +if (!existsSync(inventoryPath)) { + console.error( + "✗ COMMAND_INVENTORY.md not found at src/cli/COMMAND_INVENTORY.md", + ); + console.error( + " Create the file and populate it with all commands under src/cli/commands/", + ); + process.exit(1); +} + +// --------------------------------------------------------------------------- +// 2. Find all .ts files on disk (excluding __tests__/) +// --------------------------------------------------------------------------- + +function findTsFiles(dir: string, base: string): string[] { + const results: string[] = []; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (entry.name === "__tests__") continue; + const full = join(dir, entry.name); + if (entry.isDirectory()) { + results.push(...findTsFiles(full, base)); + } else if (entry.isFile() && entry.name.endsWith(".ts")) { + // Derive command name: strip base prefix and .ts suffix + // e.g. /abs/path/commands/oauth/connect.ts -> oauth/connect + const rel = relative(base, full); + const commandName = rel.replace(/\.ts$/, ""); + results.push(commandName); + } + } + return results; +} + +const diskCommands = new Set(findTsFiles(commandsDir, commandsDir)); + +// --------------------------------------------------------------------------- +// 3. Parse COMMAND_INVENTORY.md to extract command names from the Commands table +// --------------------------------------------------------------------------- + +const inventoryContent = readFileSync(inventoryPath, "utf-8"); +const lines = inventoryContent.split("\n"); + +const inventoryCommands = new Set(); + +// Track whether we are inside the "## Commands" section +let inCommandsSection = false; +// Track whether we have passed the table header in the Commands section +let pastCommandsHeader = false; + +for (const line of lines) { + // Detect section headers + if (line.startsWith("##")) { + if (line.trim() === "## Commands") { + inCommandsSection = true; + pastCommandsHeader = false; + } else { + inCommandsSection = false; + } + continue; + } + + if (!inCommandsSection) continue; + + // Only process table rows (lines starting with |) + if (!line.startsWith("|")) continue; + + // The first table row in the Commands section is the header ("| Command | ...") + // The second is the separator ("| --- | ...") + // We skip both, then collect subsequent rows + if (!pastCommandsHeader) { + // Skip header row (contains "Command") and separator row (contains "---") + if (line.includes("Command") || line.includes("---")) { + // Once we see the separator row, the next rows are data + if (line.includes("---")) { + pastCommandsHeader = true; + } + continue; + } + } + + // Split row by | and take the second element (index 1, first data column) + const parts = line.split("|"); + if (parts.length < 2) continue; + + // Strip backticks and whitespace from the first column + const rawName = parts[1].trim().replace(/`/g, "").trim(); + if (!rawName) continue; + + inventoryCommands.add(rawName); +} + +// --------------------------------------------------------------------------- +// 4. Special case: cache-fs was moved to lib/cache-fs.ts (not in commands/) +// If it appears in inventory but not on disk, that is expected. +// --------------------------------------------------------------------------- + +const MOVED_COMMANDS = new Set(["cache-fs"]); + +// --------------------------------------------------------------------------- +// 5. Compute mismatches +// --------------------------------------------------------------------------- + +const missingFromInventory: string[] = []; +for (const cmd of diskCommands) { + if (!inventoryCommands.has(cmd)) { + missingFromInventory.push(cmd); + } +} + +const missingFromDisk: string[] = []; +for (const cmd of inventoryCommands) { + if (!diskCommands.has(cmd) && !MOVED_COMMANDS.has(cmd)) { + missingFromDisk.push(cmd); + } +} + +missingFromInventory.sort(); +missingFromDisk.sort(); + +// --------------------------------------------------------------------------- +// 6. Report results +// --------------------------------------------------------------------------- + +let hasErrors = false; + +if (missingFromInventory.length > 0) { + hasErrors = true; + console.error( + `✗ ${missingFromInventory.length} command file(s) found on disk but missing from COMMAND_INVENTORY.md:`, + ); + for (const cmd of missingFromInventory) { + console.error(` ${cmd}`); + } +} + +if (missingFromDisk.length > 0) { + hasErrors = true; + console.error( + `✗ ${missingFromDisk.length} command(s) in COMMAND_INVENTORY.md but not found on disk:`, + ); + for (const cmd of missingFromDisk) { + console.error(` ${cmd}`); + } +} + +if (hasErrors) { + console.error( + "\nFix: ensure every .ts file under src/cli/commands/ (excl. __tests__/) has a row in COMMAND_INVENTORY.md.", + ); + process.exit(1); +} + +// --------------------------------------------------------------------------- +// 7. Success +// --------------------------------------------------------------------------- + +// Count total: disk commands + moved commands that appear in inventory +const inventoryCount = diskCommands.size + MOVED_COMMANDS.size; +console.log( + `✓ COMMAND_INVENTORY.md is in sync with src/cli/commands/ (${inventoryCount} commands)`, +); +process.exit(0); diff --git a/assistant/src/cli/AGENTS.md b/assistant/src/cli/AGENTS.md index 9e44869c6a2..021dc47eff6 100644 --- a/assistant/src/cli/AGENTS.md +++ b/assistant/src/cli/AGENTS.md @@ -22,13 +22,39 @@ Examples: `config`, `contacts`, `memory`, `autonomy`, `conversations` belong her - Use `getCliLogger("cli")` for output (not raw `console.log`). - When adding/removing/renaming assistant CLI commands or subcommands, update the gateway bash risk registry coverage in `gateway/src/risk/command-registry/commands/assistant.ts` (supported command paths + risk overrides) so permission prompts stay correct. -## Service calls — no gateway proxying +## Service calls — transport-based dispatch -CLI commands must call the service/store layer directly — the same functions that the HTTP route handlers in `runtime/routes/` call. Do not proxy through the gateway HTTP API. +CLI commands use one of two transport patterns depending on whether they need a running daemon: -Both the gateway routes and the CLI are thin wrappers around the same shared business logic. For example, `runtime/routes/invite-routes.ts` delegates to `runtime/invite-service.ts`, and `runtime/routes/contact-routes.ts` delegates to `contacts/contact-store.ts`. CLI commands should import and call those same service modules directly. +- **`ipc`-tagged commands** call `cliIpcCall` from `../../ipc/cli-client.js`. They are thin wrappers that forward requests to the daemon over the IPC socket and never import daemon-internal modules. +- **`local`-tagged commands** read or write workspace files directly (config, autonomy, completions, etc.) using the same config/store helpers the daemon uses internally. They do not require the daemon to be running. -This avoids a dependency on the gateway process being running and removes an unnecessary network hop. +Both transport classes avoid proxying through the gateway HTTP API. `ipc` commands reach the daemon directly via the socket; `local` commands bypass the daemon entirely. The transport tag is declared via `registerCommand({ transport, ... })` — see the "Transport tagging" section below. + +## Transport tagging + +Every command file declares its transport class via `registerCommand({ transport, ... })` +from `../lib/register-command.ts`. The three transport classes are: + +| Class | Rule | When to use | +|---|---|---| +| `ipc` | Wraps exactly one IPC method per subcommand. No daemon-internal imports. | Commands that call the daemon | +| `local` | Touches only static workspace files / shell artifacts. | Commands that work without a running daemon | +| `bootstrap` | Runs before the daemon is up (e.g. `assistant config init`). | Pre-daemon setup | + +The ESLint rule `cli/no-daemon-internals` enforces import allowlists per class. +See `COMMAND_INVENTORY.md` for the full command inventory. + +## Canonical example + +`commands/pending.ts` is the reference implementation for a thin `ipc`-tagged command: +- Single `registerCommand({ transport: "ipc", ... })` call +- One `cliIpcCall` per subcommand action +- Table output with `--json` flag alternative +- Clean error handling via `exitFromIpcResult` +- No daemon-internal imports + +When migrating a legacy command, start by reading `pending.ts`. ## Help Text Standards diff --git a/assistant/src/cli/COMMAND_INVENTORY.md b/assistant/src/cli/COMMAND_INVENTORY.md new file mode 100644 index 00000000000..1cc1d18e16a --- /dev/null +++ b/assistant/src/cli/COMMAND_INVENTORY.md @@ -0,0 +1,76 @@ +# CLI Command Inventory + +Source of truth for all `assistant` CLI commands. Every `.ts` file under +`src/cli/commands/` (excluding `__tests__/`) must appear in this table. +Run `bun run lint:inventory` to validate. + +## Status enum +| Value | Meaning | +|---|---| +| `THIN` | Already a thin IPC wrapper — calls cliIpcCall, no daemon-internal imports | +| `LOCAL` | Touches only local files/config — daemon need not be running | +| `LEGACY` | Not yet migrated — contains daemon-internal imports (see Sections C–I) | + +## Commands + +| Command | Subcommands | Class | Operation IDs | Status | Notes | +|---|---|---|---|---|---| +| `attachment` | `register`, `lookup` | `ipc` | `attachment_register`, `attachment_lookup` | `THIN` | | +| `bash` | _(positional command arg)_ | `ipc` | `debug_bash` | `THIN` | Debug tool; requires VELLUM_DEBUG=1 | +| `browser` | `navigate`, `snapshot`, `click`, `type`, `screenshot`, `scroll`, `press-key`, `select`, `hover`, `drag`, `wait`, `close`, `status`, _(dynamic from BROWSER_OPERATION_META)_ | `ipc` | `browser_execute` | `THIN` | Subcommands generated from BROWSER_OPERATION_META | +| `cache` | `set`, `get`, `delete` | `ipc` | `cache_set`, `cache_get`, `cache_delete` | `THIN` | | +| `clients` | `list`, `disconnect` | `ipc` | `list_clients`, `disconnect_client` | `THIN` | | +| `conversations-defer` | _(positional: defer)_, `list`, `cancel` | `ipc` | `defer_create`, `defer_list`, `defer_cancel` | `THIN` | Registered as `defer` under `conversations` namespace | +| `gateway` | `logs tail` | `ipc` | `gateway_logs_tail` | `THIN` | | +| `mcp` | `list`, `reload`, `add`, `auth`, `remove` | `ipc` | `internal_mcp_auth_status`, `internal_mcp_reload`, `internal_mcp_auth_start` | `THIN` | `list`/`add`/`remove` also touch config directly | +| `memory-v2` | `migrate`, `reembed`, `reembed-skills`, `activation`, `explain`, `rebuild-corpus-stats`, `fit-anisotropy`, `validate` | `ipc` | `memory_v2_backfill`, `memory_v2_validate`, `memory_v2_reembed_skills`, `memory_v2_explain_similarity`, `memory_v2_rebuild_corpus_stats`, `memory_v2_fit_anisotropy` | `THIN` | Registered as `v2` subgroup under `memory` | +| `notifications` | `send`, `list` | `ipc` | `emit_notification_signal`, `list_notification_events` | `THIN` | | +| `pending` | `list` | `ipc` | `pending_interactions` | `THIN` | **Canonical example** | +| `status` | _(none)_ | `ipc` | `health` | `THIN` | Has daemon-down fallback (§3.7) | +| `task` | `save`, `list`, `run`, `delete`, `queue show`, `queue add`, `queue update`, `queue remove`, `queue run` | `ipc` | `task_save`, `task_list`, `task_run`, `task_delete`, `task_queue_show`, `task_queue_add`, `task_queue_update`, `task_queue_remove`, `task_queue_run` | `THIN` | | +| `trust` | `list` | `ipc` | `trust_rules_list` | `THIN` | | +| `watchers` | `list`, `create`, `update`, `delete`, `digest` | `ipc` | `watcher_list`, `watcher_create`, `watcher_update`, `watcher_delete`, `watcher_digest` | `THIN` | | +| `autonomy` | `get`, `set` | `local` | _(none — file I/O only)_ | `LOCAL` | JARVIS-745: no daemon consumer | +| `cache-fs` | _(utility)_ | `local` | _(none)_ | `LOCAL` | **Moved to lib/cache-fs.ts** — test-isolation re-export | +| `completions` | _(positional shell arg)_ | `local` | _(none)_ | `LOCAL` | Shell completion generation | +| `config` | `get`, `set`, `list`, `schema`, `validate-allowlist` | `local` | _(none)_ | `LOCAL` | Config file r/w | +| `credential-execution` | `grants list`, `grants revoke`, `audit list` | `local` | _(none)_ | `LOCAL` | Secure key storage; communicates via CES RPC | +| `default-action` | _(none — root program.action)_ | `local` | _(none)_ | `LOCAL` | Root default action; uses program.action(), not program.command() | +| `keys` | `list`, `set`, `delete` | `local` | _(none)_ | `LOCAL` | Secure key storage via daemon credential client | +| `audit` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `auth` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `avatar` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `backup` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `channel-verification-sessions` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `contacts` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `conversations` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `conversations-import` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `credentials` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `domain` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `email` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `image-generation` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `inference` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `inference-session` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `memory` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `routes` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `sequence` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `skills` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `stt` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `tts` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `ui` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `usage` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `webhooks` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `oauth/index` | _(namespace registrar)_ | `ipc` | _(pending migration)_ | `LEGACY` | Registers all oauth/* subcommands | +| `oauth/apps` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `oauth/connect` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `oauth/disconnect` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `oauth/mode` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `oauth/ping` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `oauth/providers` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `oauth/request` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `oauth/shared` | _(shared utilities, no direct subcommands)_ | `ipc` | _(none — utility module)_ | `LEGACY` | Utility helpers for oauth commands | +| `oauth/status` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `oauth/token` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `platform/index` | _(namespace registrar)_ | `ipc` | _(pending migration)_ | `LEGACY` | Registers platform/* subcommands | +| `platform/connect` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | +| `platform/disconnect` | _(pending migration)_ | `ipc` | _(pending migration)_ | `LEGACY` | | From bb15a8c0bc5cc8de03395961aa20906a249f539d Mon Sep 17 00:00:00 2001 From: credence-the-bot <195577519+credence-the-bot@users.noreply.github.com> Date: Sun, 10 May 2026 00:53:02 +0000 Subject: [PATCH 4/6] fix(cli): address Codex + Devin findings on Section B rollup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle 1 fixes for PR #30159 review feedback: 1. Allowlist gap (Codex P2 / Devin 🟡): Add `../logger` and `../output` to `local` and `bootstrap` allowlists in `cli-no-daemon-internals.js`. Without this, every newly tagged `local` command (autonomy, config, completions, keys, credential-execution) false-positives on its standard logger/output imports, killing the rule's signal-to-noise ratio. 2. Doc reference (Devin 🟡): `forbiddenImport` error message pointed to nonexistent `DESIGN.md §3.1`. Now points to `src/cli/AGENTS.md` which is the actual shipped reference. 3. Target glob too broad (Codex P2): The rule fired `missingTransport` on every file under `src/cli/commands/**/*.ts` including helper modules like `oauth/shared.ts` that aren't command registrars. Narrowed by making the rule skip files that don't call `registerCommand` directly (now a tri-state `findTransport`: `string` for tagged, `MISSING_TRANSPORT` for registrars without transport prop, `null` for non-registrars). Helper modules pass silently. Backdoor closed because helpers can't be imported via blessed paths anyway (sibling `./helper.js` doesn't match any allowlist prefix). 4. Type-only imports (extends Codex P2 #1): `import type {...}` is erased at compile time and doesn't constitute a runtime boundary violation. Rule now skips type-only imports. Drops 3 false-positives on `memory-v2.ts` and `mcp.ts` type imports from runtime/routes/. 5. Canonical-example claim (Devin 🟡): `AGENTS.md` claimed `pending.ts` uses `exitFromIpcResult` but it didn't. Migrated `pending.ts` to actually use the helper added in Section A. Now matches the doc. 6. mcp.ts misclassification (Devin 🚩): `mcp.ts` was marked `THIN` in COMMAND_INVENTORY but imports `mcp/client.js`, `mcp/mcp-oauth-provider.js`, `util/browser.js` — actual daemon internals. Reclassified to `LEGACY` with a note that Section C/D will thin it. Until then the advisory rule will warn on this file (and that's the right signal). Lint: 18 warnings remain (down from 21) on partially-migrated commands (config, keys, credential-execution, status, task, mcp). All are legitimate signals — those files have non-allowlisted imports that need migration before Section J flips the rule from advisory → error. Tests: rule test suite extended with helper-module + type-only + local-with-logger regression cases; 5/5 manual smoke tests pass; tsc clean; lint:inventory passes. --- .../__tests__/cli-no-daemon-internals.test.ts | 57 +++++++++++++++++-- .../eslint-rules/cli-no-daemon-internals.js | 33 ++++++++++- assistant/src/cli/COMMAND_INVENTORY.md | 2 +- assistant/src/cli/commands/pending.ts | 13 ++++- 4 files changed, 95 insertions(+), 10 deletions(-) diff --git a/assistant/eslint-rules/__tests__/cli-no-daemon-internals.test.ts b/assistant/eslint-rules/__tests__/cli-no-daemon-internals.test.ts index 8ca234071d8..1f7be94fec9 100644 --- a/assistant/eslint-rules/__tests__/cli-no-daemon-internals.test.ts +++ b/assistant/eslint-rules/__tests__/cli-no-daemon-internals.test.ts @@ -78,18 +78,67 @@ tester.run("cli/no-daemon-internals", rule, { export { utilHelper }; `, }, + // Helper module — has imports but does not call registerCommand directly. + // Helper modules under commands/ (e.g. oauth/shared.ts, lib/cache-fs.ts) + // are not command entries and the rule must not fire on them, even when + // they import from outside the registrar allowlists. + { + code: ` + import type { Command } from "commander"; + import { getProvider } from "../../../oauth/oauth-store.js"; + + export function buildAuthFlow(program) { + // helper module — no registerCommand call here; the actual + // command file imports this and calls registerCommand itself. + } + `, + }, + // local-tagged file importing ../logger and ../output — both must be on + // the local allowlist (regression test for the allowlist gap that + // false-positived autonomy/config/completions/keys/credential-execution). + { + code: ` + import type { Command } from "commander"; + import { log } from "../logger.js"; + import { writeOutput } from "../output.js"; + + registerCommand(program, { + name: "local-with-output", + transport: "local", + build: () => {}, + }); + `, + }, + // Type-only imports are erased at compile time and must not count as + // runtime boundary violations, even when the source path is outside the + // allowlist (e.g. `import type` from runtime/routes for response shapes). + { + code: ` + import type { Command } from "commander"; + import { cliIpcCall } from "../../ipc/cli-client.js"; + import type { MemoryV2Result } from "../../runtime/routes/memory-v2-routes.js"; + + registerCommand(program, { + name: "ipc-with-type-import", + transport: "ipc", + build: () => {}, + }); + `, + }, ], invalid: [ - // File with imports but no registerCommand call + // registerCommand called without a string transport prop — the actual + // missingTransport case (command-entry file forgot to declare its class). { code: ` import type { Command } from "commander"; import { cliIpcCall } from "../../ipc/cli-client.js"; - export function registerMyCommand(program) { - // forgot to call registerCommand - } + registerCommand(program, { + name: "no-transport", + build: () => {}, + }); `, errors: [ { diff --git a/assistant/eslint-rules/cli-no-daemon-internals.js b/assistant/eslint-rules/cli-no-daemon-internals.js index 60572ba9029..bb3a6e1c39c 100644 --- a/assistant/eslint-rules/cli-no-daemon-internals.js +++ b/assistant/eslint-rules/cli-no-daemon-internals.js @@ -15,6 +15,8 @@ const ALLOWED_PREFIXES = { "../../config/loader", "../../config/schema", "../../util/platform", + "../logger", + "../output", "../lib/", ], bootstrap: [ @@ -24,13 +26,26 @@ const ALLOWED_PREFIXES = { "../../config/loader", "../../config/schema", "../../util/platform", + "../logger", + "../output", "../lib/", ], }; +/** + * Walks the program AST looking for a `registerCommand({ transport: ... })` + * call. Returns: + * - the transport string when registerCommand is called with a string + * transport prop ("ipc" / "local" / "bootstrap") + * - "MISSING_TRANSPORT" when registerCommand is called but no string + * transport prop is present + * - null when no registerCommand call is found at all (helper module — + * not a command entry, no checks apply) + */ function findTransport(program) { const worklist = [...program.body]; const seen = new WeakSet(); + let registerCommandCalled = false; while (worklist.length > 0) { const node = worklist.pop(); @@ -45,6 +60,7 @@ function findTransport(program) { node.callee.type === "Identifier" && node.callee.name === "registerCommand" ) { + registerCommandCalled = true; for (const arg of node.arguments) { if (arg.type === "ObjectExpression") { for (const prop of arg.properties) { @@ -105,7 +121,7 @@ function findTransport(program) { } } - return null; + return registerCommandCalled ? "MISSING_TRANSPORT" : null; } const rule = { @@ -119,7 +135,7 @@ const rule = { missingTransport: "CLI command file must call registerCommand({ transport: ... }) to declare its transport class.", forbiddenImport: - "'{{transport}}'-tagged CLI command imports forbidden module '{{source}}'. See DESIGN.md §3.1 for allowed imports.", + "'{{transport}}'-tagged CLI command imports forbidden module '{{source}}'. See src/cli/AGENTS.md for allowed imports.", }, schema: [], }, @@ -139,7 +155,14 @@ const rule = { const transport = findTransport(program); + // Helper modules (no registerCommand call) are not command entries — + // skip them. Command files that call registerCommand without a string + // transport prop still trip missingTransport. if (transport === null) { + return; + } + + if (transport === "MISSING_TRANSPORT") { context.report({ node: program, messageId: "missingTransport", @@ -153,6 +176,12 @@ const rule = { } for (const importNode of importNodes) { + // `import type {...}` and `import { type X }` are erased at compile + // time — they don't ship in the bundle and don't constitute a + // runtime boundary violation. Skip them. + if (importNode.importKind === "type") { + continue; + } const source = importNode.source.value; const allowed = allowedPrefixes.some((prefix) => source.startsWith(prefix), diff --git a/assistant/src/cli/COMMAND_INVENTORY.md b/assistant/src/cli/COMMAND_INVENTORY.md index 1cc1d18e16a..a5ac4696d04 100644 --- a/assistant/src/cli/COMMAND_INVENTORY.md +++ b/assistant/src/cli/COMMAND_INVENTORY.md @@ -22,7 +22,7 @@ Run `bun run lint:inventory` to validate. | `clients` | `list`, `disconnect` | `ipc` | `list_clients`, `disconnect_client` | `THIN` | | | `conversations-defer` | _(positional: defer)_, `list`, `cancel` | `ipc` | `defer_create`, `defer_list`, `defer_cancel` | `THIN` | Registered as `defer` under `conversations` namespace | | `gateway` | `logs tail` | `ipc` | `gateway_logs_tail` | `THIN` | | -| `mcp` | `list`, `reload`, `add`, `auth`, `remove` | `ipc` | `internal_mcp_auth_status`, `internal_mcp_reload`, `internal_mcp_auth_start` | `THIN` | `list`/`add`/`remove` also touch config directly | +| `mcp` | `list`, `reload`, `add`, `auth`, `remove` | `ipc` | `internal_mcp_auth_status`, `internal_mcp_reload`, `internal_mcp_auth_start` | `LEGACY` | Tagged `ipc` but still imports `mcp/client.js`, `mcp/mcp-oauth-provider.js`, `util/browser.js`. Section C/D will thin (move daemon work behind IPC routes). Until then, `cli/no-daemon-internals` will warn on this file. | | `memory-v2` | `migrate`, `reembed`, `reembed-skills`, `activation`, `explain`, `rebuild-corpus-stats`, `fit-anisotropy`, `validate` | `ipc` | `memory_v2_backfill`, `memory_v2_validate`, `memory_v2_reembed_skills`, `memory_v2_explain_similarity`, `memory_v2_rebuild_corpus_stats`, `memory_v2_fit_anisotropy` | `THIN` | Registered as `v2` subgroup under `memory` | | `notifications` | `send`, `list` | `ipc` | `emit_notification_signal`, `list_notification_events` | `THIN` | | | `pending` | `list` | `ipc` | `pending_interactions` | `THIN` | **Canonical example** | diff --git a/assistant/src/cli/commands/pending.ts b/assistant/src/cli/commands/pending.ts index d41bbaf942c..b97994ccbd2 100644 --- a/assistant/src/cli/commands/pending.ts +++ b/assistant/src/cli/commands/pending.ts @@ -1,6 +1,6 @@ import type { Command } from "commander"; -import { cliIpcCall } from "../../ipc/cli-client.js"; +import { cliIpcCall, exitFromIpcResult } from "../../ipc/cli-client.js"; import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; @@ -44,9 +44,16 @@ export function registerPendingCommand(program: Command): void { "pending_interactions", ); - if (!response.ok || !response.result) { + if (!response.ok) { + return exitFromIpcResult({ + ok: false, + error: response.error, + statusCode: response.statusCode, + }); + } + if (!response.result) { log.error( - `Failed to list pending interactions: ${response.error ?? "unknown error"}`, + "pending_interactions returned ok with no result body", ); process.exitCode = 1; return; From d4c9d000ca591c784533548a8cfe7ab517d62903 Mon Sep 17 00:00:00 2001 From: credence-the-bot <226213748+credence-the-bot@users.noreply.github.com> Date: Sun, 10 May 2026 01:42:09 +0000 Subject: [PATCH 5/6] =?UTF-8?q?fix(eslint-rule):=20cycle-1=20review=20find?= =?UTF-8?q?ings=20=E2=80=94=20chained=20registerCommand=20+=20inline-type?= =?UTF-8?q?=20imports?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P2 (cli-no-daemon-internals.js:88) — traverse callees: The AST walk only enqueued CallExpression.arguments, never the callee. Chained patterns like `registerCommand({...}).command(...).description(...)` were treated as if no registerCommand call existed (findTransport returned null), silently skipping all checks for those command-entry files. Fix: enqueue node.callee on CallExpression and node.object on MemberExpression so the walk reaches through the receiver chain. Codex P2 (cli-no-daemon-internals.js:183) — inline type-only specifiers: The rule only checked importNode.importKind === "type", which catches `import type {...}` but not `import { type X }` — for the inline form the declaration's importKind stays "value" and each ImportSpecifier carries its own importKind. All-type-only inline imports are erased at compile time and must not flag forbidden-import. Fix: also skip when every specifier is an ImportSpecifier with importKind === "type". Side-effect-only imports (no specifiers) and mixed value+type imports still get linted. --- .../__tests__/cli-no-daemon-internals.test.ts | 33 +++++++++++++++++++ .../eslint-rules/cli-no-daemon-internals.js | 30 +++++++++++++++-- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/assistant/eslint-rules/__tests__/cli-no-daemon-internals.test.ts b/assistant/eslint-rules/__tests__/cli-no-daemon-internals.test.ts index 1f7be94fec9..186141db2f9 100644 --- a/assistant/eslint-rules/__tests__/cli-no-daemon-internals.test.ts +++ b/assistant/eslint-rules/__tests__/cli-no-daemon-internals.test.ts @@ -125,6 +125,39 @@ tester.run("cli/no-daemon-internals", rule, { }); `, }, + // Inline `import { type X }` form: importKind is set per specifier, + // not on the declaration. When every specifier is type-only the import + // is erased and must not flag a forbidden-runtime-import violation. + { + code: ` + import type { Command } from "commander"; + import { cliIpcCall } from "../../ipc/cli-client.js"; + import { type MemoryV2Result, type ScoreBreakdown } from "../../runtime/routes/memory-v2-routes.js"; + + registerCommand(program, { + name: "ipc-with-inline-type-import", + transport: "ipc", + build: () => {}, + }); + `, + }, + // Chained registerCommand pattern: `registerCommand(...).command(...)`. + // The outer call's callee is a MemberExpression whose object is the + // inner registerCommand call. The AST walker must follow callee + + // MemberExpression.object so findTransport() locates the registerCommand + // call and applies the correct transport allowlist. + { + code: ` + import type { Command } from "commander"; + import { cliIpcCall } from "../../ipc/cli-client.js"; + + registerCommand(program, { + name: "ipc-chained", + transport: "ipc", + build: () => {}, + }).command("subcmd").description("desc"); + `, + }, ], invalid: [ diff --git a/assistant/eslint-rules/cli-no-daemon-internals.js b/assistant/eslint-rules/cli-no-daemon-internals.js index bb3a6e1c39c..af1879b38ec 100644 --- a/assistant/eslint-rules/cli-no-daemon-internals.js +++ b/assistant/eslint-rules/cli-no-daemon-internals.js @@ -83,10 +83,21 @@ function findTransport(program) { worklist.push(node.expression); break; case "CallExpression": + // Enqueue both arguments AND the callee so chained patterns like + // `registerCommand(...).command(...).description(...)` are walked. + // The outer call's callee is a MemberExpression whose object is the + // inner CallExpression; without traversing the callee we'd never + // reach the inner `registerCommand` invocation and findTransport() + // would return null, silently skipping all checks. + worklist.push(node.callee); for (const arg of node.arguments) { worklist.push(arg); } break; + case "MemberExpression": + // Reach through `.foo` chains so we can walk into the receiver. + worklist.push(node.object); + break; case "FunctionDeclaration": case "FunctionExpression": case "ArrowFunctionExpression": @@ -176,12 +187,25 @@ const rule = { } for (const importNode of importNodes) { - // `import type {...}` and `import { type X }` are erased at compile - // time — they don't ship in the bundle and don't constitute a - // runtime boundary violation. Skip them. + // `import type {...}` is erased at compile time — top-level type + // import kind is set to "type" on the declaration. Skip. if (importNode.importKind === "type") { continue; } + // Inline-type form `import { type X, type Y } from "..."` keeps + // the declaration `importKind === "value"` while marking each + // ImportSpecifier with `importKind === "type"`. When *every* + // specifier is type-only the entire import is erased. Skip those + // too. Side-effect-only imports (`import "x"`) have an empty + // specifiers list and run at module load — must NOT skip. + if ( + importNode.specifiers.length > 0 && + importNode.specifiers.every( + (s) => s.type === "ImportSpecifier" && s.importKind === "type", + ) + ) { + continue; + } const source = importNode.source.value; const allowed = allowedPrefixes.some((prefix) => source.startsWith(prefix), From a275ce18e386c03d8d7e4fe056da392d49cf379a Mon Sep 17 00:00:00 2001 From: "credence-the-bot[bot]" Date: Sun, 10 May 2026 02:01:42 +0000 Subject: [PATCH 6/6] =?UTF-8?q?fix(cli):=20status=20=E2=80=94=20narrow=20z?= =?UTF-8?q?ero-exit=20fallback=20to=20daemon-unreachable=20cases=20(cycle-?= =?UTF-8?q?3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/cli/commands/__tests__/status.test.ts | 82 ++++++++++++++----- assistant/src/cli/commands/status.ts | 27 ++++-- 2 files changed, 80 insertions(+), 29 deletions(-) diff --git a/assistant/src/cli/commands/__tests__/status.test.ts b/assistant/src/cli/commands/__tests__/status.test.ts index 11c4c429267..a8f496dc9b5 100644 --- a/assistant/src/cli/commands/__tests__/status.test.ts +++ b/assistant/src/cli/commands/__tests__/status.test.ts @@ -3,8 +3,8 @@ * * Validates: * - Successful IPC response shows version, workspace, and runtime health - * - When IPC fails (daemon down), prints "Daemon: down" or "Daemon: running" - * and exits with code 0 (not 1) + * - Daemon-unreachable (ENOENT/ECONNREFUSED) exits 0 with "Assistant: down/running" + * - Non-connection IPC failures (daemon-side error, framing) exit non-zero */ import { beforeEach, describe, expect, mock, test } from "bun:test"; @@ -15,10 +15,15 @@ import { Command } from "commander"; // Mock state // --------------------------------------------------------------------------- +// The exact prefix that cliIpcCall uses for ENOENT/ECONNREFUSED/connect-timeout. +const DAEMON_UNREACHABLE_ERROR = + "Could not connect to the assistant at /tmp/test-assistant.sock.\n" + + "Run `assistant status` to check, or `assistant gateway start` to start it."; + let ipcCalls: Array<{ method: string }> = []; -let mockResponse: { ok: boolean; result?: unknown; error?: string } = { +let mockResponse: { ok: boolean; result?: unknown; error?: string; statusCode?: number } = { ok: false, - error: "ENOENT", + error: DAEMON_UNREACHABLE_ERROR, }; let socketExists = false; @@ -141,6 +146,7 @@ async function runStatusCommand(): Promise<{ beforeEach(() => { ipcCalls = []; socketExists = false; + mockResponse = { ok: false, error: DAEMON_UNREACHABLE_ERROR }; process.exitCode = 0; }); @@ -148,9 +154,9 @@ beforeEach(() => { // Tests // --------------------------------------------------------------------------- -describe("status command — daemon down", () => { - test("exits with code 0 (not 1) when IPC call fails", async () => { - mockResponse = { ok: false, error: "ENOENT" }; +describe("status command — daemon unreachable (ENOENT/ECONNREFUSED)", () => { + test("exits 0 when daemon is unreachable", async () => { + mockResponse = { ok: false, error: DAEMON_UNREACHABLE_ERROR }; socketExists = false; const { exitCode } = await runStatusCommand(); @@ -158,35 +164,67 @@ describe("status command — daemon down", () => { expect(exitCode).toBe(0); }); - test('prints "Daemon: down" when socket file does not exist', async () => { - mockResponse = { ok: false, error: "ENOENT" }; + test('prints "Assistant: down" when socket file does not exist', async () => { + mockResponse = { ok: false, error: DAEMON_UNREACHABLE_ERROR }; socketExists = false; - const { stdout, stderr } = await runStatusCommand(); - const combined = stdout + stderr; + const { stdout } = await runStatusCommand(); - expect(combined).toContain("Assistant: down"); + expect(stdout).toContain("Assistant: down"); }); - test('prints "Daemon: running" when socket file exists but IPC fails', async () => { - mockResponse = { ok: false, error: "Connection closed before response" }; + test('prints "Assistant: running" when socket file exists but connection refused', async () => { + mockResponse = { ok: false, error: DAEMON_UNREACHABLE_ERROR }; socketExists = true; - const { stdout, stderr } = await runStatusCommand(); - const combined = stdout + stderr; + const { stdout } = await runStatusCommand(); - expect(combined).toContain("Assistant: running"); + expect(stdout).toContain("Assistant: running"); }); - test("does not print version or memory when daemon is down", async () => { - mockResponse = { ok: false, error: "ENOENT" }; + test("does not print version or memory when daemon is unreachable", async () => { + mockResponse = { ok: false, error: DAEMON_UNREACHABLE_ERROR }; socketExists = false; const { stdout, stderr } = await runStatusCommand(); - const combined = stdout + stderr; - expect(combined).not.toContain("Version"); - expect(combined).not.toContain("Memory"); + expect(stdout + stderr).not.toContain("Version"); + expect(stdout + stderr).not.toContain("Memory"); + }); +}); + +describe("status command — non-connection IPC failures", () => { + test("exits non-zero when daemon returns an internal error", async () => { + mockResponse = { ok: false, error: "Internal server error", statusCode: 500 }; + + const { exitCode } = await runStatusCommand(); + + expect(exitCode).toBe(1); + }); + + test("exits non-zero when connection closes before response", async () => { + mockResponse = { ok: false, error: "Connection closed before response" }; + + const { exitCode } = await runStatusCommand(); + + expect(exitCode).toBe(1); + }); + + test("prints the error message to stderr for non-connection failures", async () => { + mockResponse = { ok: false, error: "Internal server error", statusCode: 500 }; + + const { stderr } = await runStatusCommand(); + + expect(stderr).toContain("Internal server error"); + }); + + test("exits non-zero and prints message for empty health response", async () => { + mockResponse = { ok: true, result: undefined }; + + const { exitCode, stderr } = await runStatusCommand(); + + expect(exitCode).toBe(1); + expect(stderr).toContain("empty response"); }); }); diff --git a/assistant/src/cli/commands/status.ts b/assistant/src/cli/commands/status.ts index 53ac9e551fa..2ba43253aed 100644 --- a/assistant/src/cli/commands/status.ts +++ b/assistant/src/cli/commands/status.ts @@ -27,13 +27,26 @@ export function registerStatusCommand(program: Command): void { cmd.action(async () => { const result = await cliIpcCall("health"); - if (!result.ok || !result.result) { - const socketPath = getAssistantSocketPath(); - const socketExists = existsSync(socketPath); - const workspace = getWorkspaceDirDisplay(); - process.stdout.write((socketExists ? "Assistant: running" : "Assistant: down") + "\n"); - process.stdout.write(`Workspace: ${workspace}\n`); - process.exit(0); + if (!result.ok) { + // Only ENOENT/ECONNREFUSED/connect-timeout produce this prefix; other + // failures (daemon-side error, framing error, abort) are real failures. + if (result.error?.startsWith("Could not connect to the assistant at ")) { + const socketPath = getAssistantSocketPath(); + const socketExists = existsSync(socketPath); + const workspace = getWorkspaceDirDisplay(); + process.stdout.write( + (socketExists ? "Assistant: running" : "Assistant: down") + "\n", + ); + process.stdout.write(`Workspace: ${workspace}\n`); + process.exit(0); + } + process.stderr.write((result.error ?? "health check failed") + "\n"); + process.exit(1); + } + + if (!result.result) { + process.stderr.write("health check returned empty response\n"); + process.exit(1); } const h = result.result;