diff --git a/assistant/openapi.yaml b/assistant/openapi.yaml index 82ec8f937ab..e6c5a7d113e 100644 --- a/assistant/openapi.yaml +++ b/assistant/openapi.yaml @@ -3135,6 +3135,18 @@ paths: responses: "200": description: Successful response + /v1/config/allowlist/validate: + get: + operationId: config_allowlist_validate_get + summary: Validate secret-allowlist.json regex patterns + description: + "Compile each regex pattern in secret-allowlist.json and return any syntax errors. Returns { exists: false + } if no file is present." + tags: + - config + responses: + "200": + description: Successful response /v1/config/embeddings: get: operationId: config_embeddings_get @@ -3240,6 +3252,37 @@ paths: required: - baseUrl additionalProperties: false + /v1/config/schema: + get: + operationId: config_schema_get + summary: Get config JSON Schema + description: + Return the JSON Schema for the assistant config, optionally scoped to a dotted-path sub-schema (e.g. + ?path=calls). + tags: + - config + responses: + "200": + description: Successful response + parameters: + - name: path + in: query + required: false + schema: + type: string + description: Optional dotted path to a config sub-key + /v1/config/set: + post: + operationId: config_set_post + summary: Set a single config path + description: + Assign a value at a dotted config path with direct-replacement semantics (preserves explicit null, replaces + object subtrees instead of merging). Used by the `assistant config set ` CLI command. + tags: + - config + responses: + "200": + description: Successful response /v1/confirm: post: operationId: confirm_post diff --git a/assistant/src/__tests__/config-schema-cmd.test.ts b/assistant/src/__tests__/config-schema-cmd.test.ts index c2d9271499d..61d482752a6 100644 --- a/assistant/src/__tests__/config-schema-cmd.test.ts +++ b/assistant/src/__tests__/config-schema-cmd.test.ts @@ -3,7 +3,7 @@ import { join } from "node:path"; import { describe, expect, mock, test } from "bun:test"; // --------------------------------------------------------------------------- -// Mocks — declared before imports that depend on platform/logger +// Mocks - declared before imports that depend on platform/logger/ipc // --------------------------------------------------------------------------- const WORKSPACE_DIR = process.env.VELLUM_WORKSPACE_DIR!; @@ -45,28 +45,38 @@ mock.module("../util/logger.js", () => ({ getCliLogger: () => makeLoggerStub(), })); -mock.module("../config/loader.js", () => ({ - getConfig: () => ({ - services: { - inference: { - mode: "your-own", - provider: "anthropic", - model: "claude-opus-4-6", - }, - "image-generation": { - mode: "your-own", - provider: "gemini", - model: "gemini-3.1-flash-image-preview", - }, - "web-search": { mode: "your-own", provider: "inference-provider-native" }, - }, - }), - loadConfig: () => ({}), - loadRawConfig: () => ({}), - saveRawConfig: () => {}, - invalidateConfigCache: () => {}, - getNestedValue: () => undefined, - setNestedValue: () => {}, +// --------------------------------------------------------------------------- +// Mocks - ipc/cli-client +// +// The `config` CLI is IPC-tagged, so all schema lookups go through the +// daemon. Mock cliIpcCall so we can drive the response in each test and +// assert on exit behavior without spinning up a daemon socket. +// --------------------------------------------------------------------------- + +let mockIpcResult: { + ok: boolean; + result?: unknown; + error?: string; + statusCode?: number; +} = { ok: true, result: { schema: {} } }; + +mock.module("../ipc/cli-client.js", () => ({ + cliIpcCall: async () => mockIpcResult, + exitFromIpcResult: (r: { + error?: string; + statusCode?: number; + }) => { + process.stderr.write((r.error ?? "Unknown error") + "\n"); + if (r.statusCode === undefined) { + process.exit(10); + } else if (r.statusCode >= 500) { + process.exit(3); + } else if (r.statusCode >= 400) { + process.exit(2); + } else { + process.exit(1); + } + }, })); import { Command } from "commander"; @@ -259,30 +269,54 @@ describe("z.toJSONSchema integration", () => { // --------------------------------------------------------------------------- // Tests: CLI schema command error path +// +// The CLI now routes `config schema ` through the daemon. When the +// daemon throws a BadRequestError for an unknown path, the IPC layer +// returns statusCode=400, and exitFromIpcResult maps that to process exit +// code 2 (per the matrix in cli-client.ts:exitFromIpcResult). // --------------------------------------------------------------------------- describe("CLI schema command", () => { - test("nonexistent path prints error and exits with code 1", () => { + test("daemon error for nonexistent path surfaces via exitFromIpcResult", async () => { + // Drive the IPC mock to return a BadRequest as the daemon would + mockIpcResult = { + ok: false, + error: "No schema found at path: nonexistent", + statusCode: 400, + }; + const program = new Command(); - program.exitOverride(); // throw instead of calling process.exit + program.exitOverride(); registerConfigCommand(program); const origExit = process.exit; - // Replace process.exit to capture the exit code without killing the test + const origStderrWrite = process.stderr.write.bind(process.stderr); let exitCode: number | undefined; process.exit = ((code?: number) => { exitCode = code; throw new Error(`process.exit(${code})`); }) as never; + process.stderr.write = (() => true) as typeof process.stderr.write; try { - program.parse(["node", "test", "config", "schema", "nonexistent"]); + await program.parseAsync([ + "node", + "test", + "config", + "schema", + "nonexistent", + ]); } catch { - // Expected: either Commander's exitOverride or our process.exit stub throws + // Expected: process.exit stub throws } finally { process.exit = origExit; + process.stderr.write = origStderrWrite; } - expect(exitCode).toBe(1); + // 400 → exit 2 (per exitFromIpcResult matrix) + expect(exitCode).toBe(2); + + // Restore default + mockIpcResult = { ok: true, result: { schema: {} } }; }); }); diff --git a/assistant/src/__tests__/config-set-platform-guard.test.ts b/assistant/src/__tests__/config-set-platform-guard.test.ts index b323e45dec3..364df39aa7e 100644 --- a/assistant/src/__tests__/config-set-platform-guard.test.ts +++ b/assistant/src/__tests__/config-set-platform-guard.test.ts @@ -4,6 +4,11 @@ import { Command } from "commander"; // --------------------------------------------------------------------------- // Mock state +// +// The `config` CLI is now an IPC-tagged command (routed via cliIpcCall) so +// these tests assert on the IPC calls the CLI emits, not on direct loader +// writes. The only non-IPC interaction is `requirePlatformConnection`, which +// reads the platform client - mocked below. // --------------------------------------------------------------------------- let mockPlatformClientCreate: () => Promise Promise | null> = async () => null; -let mockLoadRawConfig: () => Record = () => ({}); -const mockSaveRawConfigCalls: Array> = []; -const mockSetNestedValueCalls: Array<{ - obj: Record; - key: string; - value: unknown; +const mockIpcCalls: Array<{ + method: string; + params?: Record; }> = []; -let mockGetNestedValue: ( - obj: Record, - key: string, -) => unknown = () => undefined; + +let mockIpcResult: { + ok: boolean; + result?: unknown; + error?: string; + statusCode?: number; +} = { ok: true, result: { ok: true } }; // --------------------------------------------------------------------------- -// Mocks — platform/client (controls requirePlatformConnection) +// Mocks - platform/client (controls requirePlatformConnection) // --------------------------------------------------------------------------- mock.module("../platform/client.js", () => ({ @@ -34,57 +39,37 @@ mock.module("../platform/client.js", () => ({ })); // --------------------------------------------------------------------------- -// Mocks — config/loader +// Mocks - ipc/cli-client (the CLI's only write path) // --------------------------------------------------------------------------- -mock.module("../config/loader.js", () => ({ - getConfig: () => ({ services: {} }), - loadConfig: () => ({ services: {} }), - invalidateConfigCache: () => {}, - loadRawConfig: () => mockLoadRawConfig(), - saveRawConfig: (raw: Record) => { - mockSaveRawConfigCalls.push(raw); - }, - applyNestedDefaults: (c: unknown) => c, - deepMergeOverwrite: (a: unknown) => a, - mergeDefaultWorkspaceConfig: () => {}, - getNestedValue: (obj: Record, key: string) => - mockGetNestedValue(obj, key), - setNestedValue: ( - obj: Record, - key: string, - value: unknown, +mock.module("../ipc/cli-client.js", () => ({ + cliIpcCall: async ( + method: string, + params?: Record, ) => { - mockSetNestedValueCalls.push({ obj, key, value }); - const keys = key.split("."); - let current = obj; - for (let i = 0; i < keys.length - 1; i++) { - const segment = keys[i]!; - if ( - current[segment] == null || - typeof current[segment] !== "object" || - Array.isArray(current[segment]) - ) { - current[segment] = {}; - } - current = current[segment] as Record; + mockIpcCalls.push({ method, params }); + return mockIpcResult; + }, + exitFromIpcResult: (r: { + error?: string; + statusCode?: number; + }) => { + process.stderr.write((r.error ?? "Unknown error") + "\n"); + if (r.statusCode === undefined) { + process.exitCode = 10; + } else if (r.statusCode >= 500) { + process.exitCode = 3; + } else if (r.statusCode >= 400) { + process.exitCode = 2; + } else { + process.exitCode = 1; } - current[keys[keys.length - 1]!] = value; + throw new Error(`exitFromIpcResult(${r.statusCode ?? "no-status"})`); }, - API_KEY_PROVIDERS: [ - "anthropic", - "openai", - "gemini", - "ollama", - "fireworks", - "openrouter", - "brave", - "perplexity", - ], })); // --------------------------------------------------------------------------- -// Mocks — util/logger (suppress log output) +// Mocks - util/logger (suppress log output) // --------------------------------------------------------------------------- mock.module("../util/logger.js", () => ({ @@ -103,7 +88,7 @@ mock.module("../util/logger.js", () => ({ })); // --------------------------------------------------------------------------- -// Mocks — oauth/oauth-store (transitive dep of oauth/shared.ts) +// Mocks - oauth/oauth-store (transitive dep guard for oauth/shared.ts) // --------------------------------------------------------------------------- mock.module("../oauth/oauth-store.js", () => ({ @@ -187,17 +172,15 @@ async function runCli( // Tests // --------------------------------------------------------------------------- -describe("config set — platform connection guard for service mode paths", () => { +describe("config set - platform connection guard for service mode paths", () => { beforeEach(() => { // Default: not connected to platform mockPlatformClientCreate = async () => null; - mockLoadRawConfig = () => ({}); - mockSaveRawConfigCalls.length = 0; - mockSetNestedValueCalls.length = 0; - mockGetNestedValue = () => undefined; + mockIpcCalls.length = 0; + mockIpcResult = { ok: true, result: { ok: true } }; }); - test("config set services.image-generation.mode your-own — succeeds without platform connection", async () => { + test("config set services.image-generation.mode your-own - succeeds without platform connection", async () => { const { exitCode } = await runCli([ "node", "assistant", @@ -209,16 +192,18 @@ describe("config set — platform connection guard for service mode paths", () = ]); expect(exitCode).toBe(0); - // Config should have been written — setting to "your-own" doesn't need platform - expect(mockSetNestedValueCalls).toHaveLength(1); - expect(mockSetNestedValueCalls[0]!.key).toBe( - "services.image-generation.mode", - ); - expect(mockSetNestedValueCalls[0]!.value).toBe("your-own"); - expect(mockSaveRawConfigCalls).toHaveLength(1); + // The CLI should have emitted exactly one config_set IPC call. + const setCalls = mockIpcCalls.filter((c) => c.method === "config_set"); + expect(setCalls).toHaveLength(1); + expect(setCalls[0]!.params).toEqual({ + body: { + path: "services.image-generation.mode", + value: "your-own", + }, + }); }); - test("config set calls.enabled true — succeeds without platform connection", async () => { + test("config set calls.enabled true - succeeds without platform connection and parses to boolean", async () => { const { exitCode } = await runCli([ "node", "assistant", @@ -229,21 +214,17 @@ describe("config set — platform connection guard for service mode paths", () = ]); expect(exitCode).toBe(0); - // Config should have been written - expect(mockSetNestedValueCalls).toHaveLength(1); - expect(mockSetNestedValueCalls[0]!.key).toBe("calls.enabled"); - expect(mockSetNestedValueCalls[0]!.value).toBe(true); - expect(mockSaveRawConfigCalls).toHaveLength(1); - }); - - test("config set ingress.publicBaseUrl overwrites existing value", async () => { - mockLoadRawConfig = () => ({ - ingress: { - publicBaseUrl: "https://stale-velay.example.test", - publicBaseUrlManagedBy: "velay", - }, + const setCalls = mockIpcCalls.filter((c) => c.method === "config_set"); + expect(setCalls).toHaveLength(1); + expect(setCalls[0]!.params).toEqual({ + body: { path: "calls.enabled", value: true }, }); + }); + test("config set ingress.publicBaseUrl - sends the new value to the daemon", async () => { + // Daemon-side semantics (sibling preservation, deep-merge avoidance) are + // covered by daemon route tests. Here we only verify the CLI sends the + // correct dotted-path + value pair via IPC. const { exitCode } = await runCli([ "node", "assistant", @@ -254,16 +235,17 @@ describe("config set — platform connection guard for service mode paths", () = ]); expect(exitCode).toBe(0); - expect(mockSaveRawConfigCalls).toHaveLength(1); - expect(mockSaveRawConfigCalls[0]).toEqual({ - ingress: { - publicBaseUrl: "https://manual.example.test", - publicBaseUrlManagedBy: "velay", + const setCalls = mockIpcCalls.filter((c) => c.method === "config_set"); + expect(setCalls).toHaveLength(1); + expect(setCalls[0]!.params).toEqual({ + body: { + path: "ingress.publicBaseUrl", + value: "https://manual.example.test", }, }); }); - test("config set services.web-search.mode managed — fails when not connected", async () => { + test("config set services.web-search.mode managed - fails when not connected, no IPC write emitted", async () => { const { exitCode, stdout } = await runCli([ "node", "assistant", @@ -278,7 +260,10 @@ describe("config set — platform connection guard for service mode paths", () = const parsed = JSON.parse(stdout); expect(parsed.ok).toBe(false); expect(parsed.error).toContain("vellum platform connect"); - expect(mockSaveRawConfigCalls).toHaveLength(0); + // The guard runs *before* the IPC call - no config_set should have been + // emitted. + expect( + mockIpcCalls.filter((c) => c.method === "config_set"), + ).toHaveLength(0); }); - }); diff --git a/assistant/src/__tests__/config-set-route.test.ts b/assistant/src/__tests__/config-set-route.test.ts new file mode 100644 index 00000000000..94b1c73fcc6 --- /dev/null +++ b/assistant/src/__tests__/config-set-route.test.ts @@ -0,0 +1,198 @@ +/** + * Daemon-side tests for the `config_set` and `config_allowlist_validate` + * IPC routes. Exercises input validation + error handling on the route + * handlers directly (Codex review feedback on PR #30262). + */ + +import { describe, expect, mock, test } from "bun:test"; + +import { makeMockLogger } from "./helpers/mock-logger.js"; + +mock.module("../util/logger.js", () => ({ + getLogger: () => makeMockLogger(), +})); + +// --------------------------------------------------------------------------- +// Mocks for handleSetConfig's transitive deps +// --------------------------------------------------------------------------- + +let savedRaw: Record | null = null; +let rawConfig: Record = {}; + +mock.module("../config/loader.js", () => ({ + loadRawConfig: () => structuredClone(rawConfig), + saveRawConfig: (raw: Record) => { + savedRaw = raw; + }, + deepMergeOverwrite: () => {}, + getConfig: () => rawConfig, + invalidateConfigCache: () => {}, + // setNestedValue is also exported by loader; handleSetConfig imports the + // real one from this module, so we re-export a thin implementation that + // mutates in place (matches loader's behavior). + setNestedValue: ( + obj: Record, + path: string, + value: unknown, + ) => { + const keys = path.split("."); + let current: Record = obj; + for (let i = 0; i < keys.length - 1; i++) { + const key = keys[i]!; + if (current[key] == null || typeof current[key] !== "object") { + current[key] = {}; + } + current = current[key] as Record; + } + current[keys[keys.length - 1]!] = value; + }, +})); + +mock.module("../providers/registry.js", () => ({ + initializeProviders: async () => {}, +})); + +mock.module("../memory/embedding-backend.js", () => ({ + clearEmbeddingBackendCache: () => {}, +})); + +// --------------------------------------------------------------------------- +// Mock the allowlist module so we can drive throw / return paths +// --------------------------------------------------------------------------- + +let mockAllowlistResult: + | { kind: "ok"; value: Array<{ index: number; pattern: string; message: string }> | null } + | { kind: "throw"; error: Error } = { kind: "ok", value: null }; + +mock.module("../security/secret-allowlist.js", () => ({ + validateAllowlistFile: () => { + if (mockAllowlistResult.kind === "throw") { + throw mockAllowlistResult.error; + } + return mockAllowlistResult.value; + }, +})); + +import { ROUTES } from "../runtime/routes/conversation-query-routes.js"; +import { BadRequestError } from "../runtime/routes/errors.js"; + +const configSetRoute = ROUTES.find((r) => r.operationId === "config_set")!; +const allowlistRoute = ROUTES.find( + (r) => r.operationId === "config_allowlist_validate", +)!; + +// --------------------------------------------------------------------------- +// config_set request validation +// --------------------------------------------------------------------------- + +describe("config_set route - request validation", () => { + test("rejects body with missing value field (P2 - Codex)", async () => { + // Body has a path but no value key — would otherwise pass `undefined` + // through setNestedValue and silently drop the key on save. + await expect( + configSetRoute.handler({ body: { path: "foo.bar" } }), + ).rejects.toThrow(BadRequestError); + await expect( + configSetRoute.handler({ body: { path: "foo.bar" } }), + ).rejects.toThrow("`value` is required"); + }); + + test("accepts body with explicit null value", async () => { + rawConfig = { heartbeat: { activeHoursStart: "09:00" } }; + savedRaw = null; + const result = await configSetRoute.handler({ + body: { path: "heartbeat.activeHoursStart", value: null }, + }); + expect(result).toEqual({ ok: true }); + expect(savedRaw).not.toBeNull(); + const heartbeat = (savedRaw as unknown as Record) + .heartbeat as Record; + expect(heartbeat.activeHoursStart).toBeNull(); + }); + + test("rejects body missing path field", async () => { + await expect( + configSetRoute.handler({ body: { value: "foo" } }), + ).rejects.toThrow(BadRequestError); + }); + + test("rejects body that is not a plain object", async () => { + // Pass null/array via the unknown cast because RouteHandlerArgs.body + // narrows to Record | undefined - we explicitly want + // to test the runtime guard against malformed inputs. + await expect( + configSetRoute.handler({ + body: null as unknown as Record, + }), + ).rejects.toThrow(BadRequestError); + await expect( + configSetRoute.handler({ + body: [] as unknown as Record, + }), + ).rejects.toThrow(BadRequestError); + }); + + test("accepts a normal scalar set and writes to raw config", async () => { + rawConfig = {}; + savedRaw = null; + const result = await configSetRoute.handler({ + body: { path: "calls.enabled", value: true }, + }); + expect(result).toEqual({ ok: true }); + const calls = (savedRaw as unknown as Record) + .calls as Record; + expect(calls.enabled).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// config_allowlist_validate error handling +// --------------------------------------------------------------------------- + +describe("config_allowlist_validate route - error handling", () => { + test("returns parseError when validateAllowlistFile throws (P3 - Codex)", () => { + mockAllowlistResult = { + kind: "throw", + error: new SyntaxError("Unexpected token } in JSON at position 42"), + }; + + const result = allowlistRoute.handler({}) as { + exists: boolean; + parseError?: string; + errors: unknown[]; + }; + expect(result.exists).toBe(true); + expect(result.parseError).toContain("Unexpected token"); + expect(result.errors).toEqual([]); + }); + + test("returns { exists: false } when file is absent", () => { + mockAllowlistResult = { kind: "ok", value: null }; + const result = allowlistRoute.handler({}) as { exists: boolean }; + expect(result.exists).toBe(false); + }); + + test("returns { exists: true, errors } when file is valid", () => { + mockAllowlistResult = { kind: "ok", value: [] }; + const result = allowlistRoute.handler({}) as { + exists: boolean; + errors: unknown[]; + }; + expect(result.exists).toBe(true); + expect(result.errors).toEqual([]); + }); + + test("returns { exists: true, errors } with details when patterns are invalid", () => { + mockAllowlistResult = { + kind: "ok", + value: [{ index: 0, pattern: "(", message: "Unterminated group" }], + }; + const result = allowlistRoute.handler({}) as { + exists: boolean; + errors: Array<{ index: number; pattern: string; message: string }>; + }; + expect(result.exists).toBe(true); + expect(result.errors).toHaveLength(1); + expect(result.errors[0]?.message).toBe("Unterminated group"); + }); +}); diff --git a/assistant/src/cli/commands/config.ts b/assistant/src/cli/commands/config.ts index 3aa3e9d33b4..e91e2eee47a 100644 --- a/assistant/src/cli/commands/config.ts +++ b/assistant/src/cli/commands/config.ts @@ -1,14 +1,7 @@ import type { Command } from "commander"; -import { z } from "zod"; -import { - getNestedValue, - loadRawConfig, - saveRawConfig, - setNestedValue, -} from "../../config/loader.js"; -import { AssistantConfigSchema } from "../../config/schema.js"; -import { getSchemaAtPath } from "../../config/schema-utils.js"; +import { cliIpcCall, exitFromIpcResult } from "../../ipc/cli-client.js"; +import { getNestedValue } from "../lib/nested-value.js"; import { registerCommand } from "../lib/register-command.js"; import { log } from "../logger.js"; import { requirePlatformConnection } from "./oauth/shared.js"; @@ -39,20 +32,38 @@ function flattenConfig( /** Matches config paths like `services.image-generation.mode`, `services.web-search.mode`, etc. */ const SERVICE_MODE_PATH_RE = /^services\.[^.]+\.mode$/; +/** + * Fetch the full raw config from the assistant via IPC. + * On transport / connection error, prints a helpful message and exits. + */ +async function fetchRawConfig( + cmd: Command, +): Promise | undefined> { + const ipcResult = await cliIpcCall>("config_get"); + if (!ipcResult.ok) { + exitFromIpcResult(ipcResult, cmd); + return undefined; + } + return ipcResult.result ?? {}; +} + export function registerConfigCommand(program: Command): void { registerCommand(program, { name: "config", - transport: "local", + transport: "ipc", description: "Manage configuration", build: (config) => { config.addHelpText( "after", ` -Configuration is stored in config.json in the assistant workspace directory. -Keys support dotted paths for nested values (e.g. calls.enabled, twilio.accountSid). -Values are auto-parsed as JSON (booleans, numbers, objects) with fallback to -plain string if parsing fails. +Configuration is managed by the assistant. The CLI sends every read/write +through the assistant so the in-memory cache, provider registry, and +file-watcher stay coherent with config.json. + +Keys support dotted paths for nested values (e.g. calls.enabled, +twilio.accountSid). Values are auto-parsed as JSON (booleans, numbers, +objects) with fallback to plain string if parsing fails. API keys are managed separately via secure storage. Use "assistant keys list" and "assistant keys set " to view and manage API keys. @@ -81,7 +92,10 @@ Arguments: true, "42" becomes number 42). Falls back to plain string if JSON parsing fails. -After writing the value to config.json, the change takes effect immediately. +The CLI sends the change to the assistant, which assigns the value at the +given path, invalidates caches, and reinitializes providers so the new +value takes effect immediately. Object subtrees replace (not merge), and +explicit null is preserved. To manage API keys, use "assistant keys set " instead. @@ -105,9 +119,16 @@ Examples: if (!connected) return; } - const raw = loadRawConfig(); - setNestedValue(raw, key, parsed); - saveRawConfig(raw); + // Direct-replacement set semantics (preserves null, replaces objects). + // See conversation-query-routes.ts:handleSetConfig for why this is a + // separate route from config_patch. + const result = await cliIpcCall("config_set", { + body: { path: key, value: parsed }, + }); + if (!result.ok) { + exitFromIpcResult(result, cmd); + return; + } log.info(`Set ${key} = ${JSON.stringify(parsed)}`); }, ); @@ -122,8 +143,9 @@ Arguments: key Dotted path to the config key (e.g. llm.default.provider, calls.enabled) -Prints the value at the given key path. If the key is not set, prints -"(not set)". Object values are pretty-printed as indented JSON. +Fetches the full config from the assistant and prints the value at the +given key path. If the key is not set, prints "(not set)". Object +values are pretty-printed as indented JSON. To view API keys, use "assistant keys list" instead. @@ -131,8 +153,9 @@ Examples: $ assistant config get llm.default.provider $ assistant config get calls.enabled`, ) - .action((key: string) => { - const raw = loadRawConfig(); + .action(async (key: string, _opts: unknown, cmd: Command) => { + const raw = await fetchRawConfig(cmd); + if (!raw) return; const value = getNestedValue(raw, key); if (value === undefined) { log.info(`(not set)`); @@ -154,36 +177,25 @@ Examples: Arguments: path Optional dotted path to a config key (e.g. calls, memory.segmentation) -Prints the JSON Schema for the entire config object, or the sub-schema at the -given path. Useful for understanding available fields, their types, defaults, -and constraints. +Asks the assistant for the JSON Schema of the entire config object, or +the sub-schema at the given path. Useful for understanding available +fields, their types, defaults, and constraints. Examples: $ assistant config schema $ assistant config schema calls $ assistant config schema memory.segmentation`, ) - .action((path?: string) => { - if (!path) { - const jsonSchema = z.toJSONSchema(AssistantConfigSchema, { - unrepresentable: "any", - io: "input", - }); - log.info(JSON.stringify(jsonSchema, null, 2)); + .action(async (path: string | undefined, _opts: unknown, cmd: Command) => { + const result = await cliIpcCall<{ schema: unknown }>( + "config_schema_get", + path ? { queryParams: { path } } : undefined, + ); + if (!result.ok) { + exitFromIpcResult(result, cmd); return; } - - const subSchema = getSchemaAtPath(AssistantConfigSchema, path); - if (!subSchema) { - log.error(`No schema found at path: ${path}`); - process.exit(1); - } - - const jsonSchema = z.toJSONSchema(subSchema, { - unrepresentable: "any", - io: "input", - }); - log.info(JSON.stringify(jsonSchema, null, 2)); + log.info(JSON.stringify(result.result?.schema ?? {}, null, 2)); }); config @@ -196,8 +208,9 @@ Examples: .addHelpText( "after", ` -Dumps the full raw configuration from config.json as pretty-printed JSON. -If no configuration has been set, prints "No configuration set". +Fetches the full raw configuration from the assistant and prints it as +pretty-printed JSON. If no configuration has been set, prints +"No configuration set". The --search flag filters results by case-insensitive substring match against flattened dotted key paths. For example, --search calls matches calls.enabled, @@ -208,8 +221,9 @@ Examples: $ assistant config list --search api $ assistant config list --search calls`, ) - .action((opts: { search?: string }) => { - const raw = loadRawConfig(); + .action(async (opts: { search?: string }, cmd: Command) => { + const raw = await fetchRawConfig(cmd); + if (!raw) return; if (Object.keys(raw).length === 0) { log.info("No configuration set"); return; @@ -254,33 +268,42 @@ reports that and exits normally. Examples: $ assistant config validate-allowlist`, ) - .action(() => { - const { validateAllowlistFile } = - // eslint-disable-next-line @typescript-eslint/no-require-imports - require("../../security/secret-allowlist.js") as typeof import("../../security/secret-allowlist.js"); - try { - const errors = validateAllowlistFile(); - if (errors == null) { - log.info("No secret-allowlist.json file found"); - return; - } - if (errors.length === 0) { - log.info("All patterns in secret-allowlist.json are valid"); - return; - } - log.error( - `Found ${errors.length} invalid pattern(s) in secret-allowlist.json:`, - ); - for (const e of errors) { - log.error(` [${e.index}] "${e.pattern}": ${e.message}`); - } - process.exit(1); - } catch (err) { + .action(async (_opts: unknown, cmd: Command) => { + const result = await cliIpcCall<{ + exists: boolean; + parseError?: string; + errors?: Array<{ index: number; pattern: string; message: string }>; + }>("config_allowlist_validate"); + if (!result.ok) { + exitFromIpcResult(result, cmd); + return; + } + const payload = result.result; + if (!payload || !payload.exists) { + log.info("No secret-allowlist.json file found"); + return; + } + // The daemon surfaces a malformed-JSON failure as `parseError` so + // the CLI can print a single user-readable message and exit 1, + // matching the pre-IPC behavior. + if (payload.parseError) { log.error( - `Failed to read secret-allowlist.json: ${(err as Error).message}`, + `Failed to read secret-allowlist.json: ${payload.parseError}`, ); process.exit(1); } + const errors = payload.errors ?? []; + if (errors.length === 0) { + log.info("All patterns in secret-allowlist.json are valid"); + return; + } + log.error( + `Found ${errors.length} invalid pattern(s) in secret-allowlist.json:`, + ); + for (const e of errors) { + log.error(` [${e.index}] "${e.pattern}": ${e.message}`); + } + process.exit(1); }); }, }); diff --git a/assistant/src/cli/lib/nested-value.ts b/assistant/src/cli/lib/nested-value.ts new file mode 100644 index 00000000000..e177f6bcc86 --- /dev/null +++ b/assistant/src/cli/lib/nested-value.ts @@ -0,0 +1,44 @@ +/** + * Dotted-path get/set helpers for the CLI. + * + * These two functions are byte-for-byte equivalent to the implementations in + * `config/loader.ts` (see `getNestedValue` / `setNestedValue` there). They + * are inlined here so the IPC-tagged `config` command can walk dotted paths + * without importing `config/loader.js`, which has module-level side effects + * (config cache, file watcher, etc.) inappropriate for the CLI process. + * + * Loader and CLI both call `setNestedValue("a.b.c", v)`-style helpers - if + * the behavior needs to change in one place, change it in both. + */ + +export function getNestedValue( + obj: Record, + path: string, +): unknown { + const keys = path.split("."); + let current: unknown = obj; + for (const key of keys) { + if (current == null || typeof current !== "object") { + return undefined; + } + current = (current as Record)[key]; + } + return current; +} + +export function setNestedValue( + obj: Record, + path: string, + value: unknown, +): void { + const keys = path.split("."); + let current = obj; + for (let i = 0; i < keys.length - 1; i++) { + const key = keys[i]!; + if (current[key] == null || typeof current[key] !== "object") { + current[key] = {}; + } + current = current[key] as Record; + } + current[keys[keys.length - 1]!] = value; +} diff --git a/assistant/src/runtime/auth/route-policy.ts b/assistant/src/runtime/auth/route-policy.ts index c1ee6d638d2..95d9c432236 100644 --- a/assistant/src/runtime/auth/route-policy.ts +++ b/assistant/src/runtime/auth/route-policy.ts @@ -397,7 +397,14 @@ const ACTOR_ENDPOINTS: Array<{ endpoint: string; scopes: Scope[] }> = [ // Generic config read/patch { endpoint: "config:GET", scopes: ["settings.read"] }, + + // Config JSON Schema (full or scoped sub-schema) + { endpoint: "config/schema:GET", scopes: ["settings.read"] }, { endpoint: "config:PATCH", scopes: ["settings.write"] }, + // Direct single-path set (preserves null, replaces objects) + { endpoint: "config/set:POST", scopes: ["settings.write"] }, + // Secret-allowlist regex validation (read-only) + { endpoint: "config/allowlist/validate:GET", scopes: ["settings.read"] }, // LLM call site catalog { endpoint: "config/llm/call-sites:GET", scopes: ["settings.read"] }, diff --git a/assistant/src/runtime/routes/conversation-query-routes.ts b/assistant/src/runtime/routes/conversation-query-routes.ts index 9acc16e9a6e..484bf1a8659 100644 --- a/assistant/src/runtime/routes/conversation-query-routes.ts +++ b/assistant/src/runtime/routes/conversation-query-routes.ts @@ -28,7 +28,10 @@ import { invalidateConfigCache, loadRawConfig, saveRawConfig, + setNestedValue, } from "../../config/loader.js"; +import { AssistantConfigSchema } from "../../config/schema.js"; +import { getSchemaAtPath } from "../../config/schema-utils.js"; import { ProfileEntry } from "../../config/schemas/llm.js"; import { VALID_MEMORY_EMBEDDING_PROVIDERS } from "../../config/schemas/memory-storage.js"; import { VALID_INFERENCE_PROVIDERS } from "../../config/schemas/services.js"; @@ -66,6 +69,7 @@ import { getMemoryRecallLogByMessageIds } from "../../memory/memory-recall-log-s import { getMemoryV2ActivationLogByMessageIds } from "../../memory/memory-v2-activation-log-store.js"; import { MEMORY_V2_CONSOLIDATION_SOURCE } from "../../memory/v2/constants.js"; import { initializeProviders } from "../../providers/registry.js"; +import { validateAllowlistFile } from "../../security/secret-allowlist.js"; import { resolvePricingForUsage } from "../../util/pricing.js"; import { BadRequestError, InternalError, NotFoundError } from "./errors.js"; import { @@ -413,6 +417,38 @@ function handleGetConfig() { } } +/** + * Return the JSON Schema for the assistant config (full or scoped). + * + * The schema is derived from `AssistantConfigSchema` at runtime via + * `z.toJSONSchema()`. Pure read; no daemon state involved. + */ +function handleGetConfigSchema({ queryParams = {} }: RouteHandlerArgs) { + const rawPath = queryParams.path; + const path = typeof rawPath === "string" ? rawPath.trim() : ""; + + if (!path) { + return { + schema: z.toJSONSchema(AssistantConfigSchema, { + unrepresentable: "any", + io: "input", + }), + }; + } + + const subSchema = getSchemaAtPath(AssistantConfigSchema, path); + if (!subSchema) { + throw new BadRequestError(`No schema found at path: ${path}`); + } + + return { + schema: z.toJSONSchema(subSchema, { + unrepresentable: "any", + io: "input", + }), + }; +} + function rejectManagedProfileDeletion(body: Record): void { const llm = asMutablePlainObject(body.llm); if (!llm) return; @@ -430,27 +466,23 @@ function rejectManagedProfileDeletion(body: Record): void { } } -async function handlePatchConfig({ body }: RouteHandlerArgs) { - if ( - !body || - typeof body !== "object" || - Array.isArray(body) || - Object.keys(body).length === 0 - ) { - throw new BadRequestError("Body must be a non-empty JSON object"); - } - rejectManagedProfileDeletion(body as Record); - - const raw = loadRawConfig(); - const patch = body as Record; - deepMergeOverwrite(raw, patch); - +/** + * Persist a mutated raw config object to disk and synchronize the running + * daemon (file-watcher, embedding cache, provider registry). + * + * Shared by `handlePatchConfig` and `handleSetConfig` so both write paths get + * identical post-write side effects. + */ +async function commitConfigWrite( + raw: Record, + opLabel: string, +): Promise { // Suppress the file-watcher callback for the duration of the debounce // window. Without this, the ConfigWatcher detects the config.json write // ~200ms later, sees a stale fingerprint, and calls initializeProviders a - // second time — starting with providers.clear() which races with the + // second time - starting with providers.clear() which races with the // explicit reinit below. The watcher also fires onConversationEvict(), - // which would evict all cached conversations on every PATCH. Mirror the + // which would evict all cached conversations on every write. Mirror the // suppress/reset pattern used in setModel (config-model.ts). const configWatcher = getConfigWatcher(); const wasSuppressed = configWatcher.suppressConfigReload; @@ -460,7 +492,7 @@ async function handlePatchConfig({ body }: RouteHandlerArgs) { } catch (err) { configWatcher.suppressConfigReload = wasSuppressed; const message = err instanceof Error ? err.message : String(err); - throw new InternalError(`Failed to patch config: ${message}`); + throw new InternalError(`Failed to ${opLabel} config: ${message}`); } configWatcher.timers.schedule( "__suppress_reset__", @@ -475,7 +507,7 @@ async function handlePatchConfig({ body }: RouteHandlerArgs) { // Reinitialize providers so the live registry reflects the new config // (e.g. a mode flip between managed and your-own). Isolated try/catch so // a provider reinit failure doesn't mask the successful config save. - // Only advance the config fingerprint on success — if reinit failed, leave + // Only advance the config fingerprint on success - if reinit failed, leave // it stale so the watcher can detect the saved config on the next event // and retry provider initialization. try { @@ -483,11 +515,101 @@ async function handlePatchConfig({ body }: RouteHandlerArgs) { configWatcher.updateFingerprint(); } catch (err) { const message = err instanceof Error ? err.message : String(err); - log.error({ err }, `handlePatchConfig: provider reinit failed: ${message}`); + log.error({ err }, `${opLabel} config: provider reinit failed: ${message}`); + } +} + +async function handlePatchConfig({ body }: RouteHandlerArgs) { + if ( + !body || + typeof body !== "object" || + Array.isArray(body) || + Object.keys(body).length === 0 + ) { + throw new BadRequestError("Body must be a non-empty JSON object"); } + rejectManagedProfileDeletion(body as Record); + + const raw = loadRawConfig(); + const patch = body as Record; + deepMergeOverwrite(raw, patch); + + await commitConfigWrite(raw, "patch"); + return { ok: true }; +} + +/** + * Direct path assignment - replaces `config_patch` for the `assistant + * config set ` CLI path. + * + * `config_patch` uses `deepMergeOverwrite` semantics, which strips `null` + * leaves when the target subtree doesn't exist and merges (rather than + * replaces) object subtrees. That's correct for partial updates (embedding + * config, profile patches) but breaks single-key `set` semantics, where the + * user expects: + * - `set heartbeat.activeHoursStart null` to persist explicit `null` + * - `set llm {}` to replace `llm`, not merge into it + * + * `config_set` performs `setNestedValue` directly on the loaded raw config + * (no merge), then runs the same post-write side effects as patch. + */ +async function handleSetConfig({ body }: RouteHandlerArgs) { + if (!body || typeof body !== "object" || Array.isArray(body)) { + throw new BadRequestError( + "Body must be a JSON object with `path` and `value`", + ); + } + const bodyRecord = body as Record; + const { path, value } = bodyRecord as { path?: unknown; value?: unknown }; + if (typeof path !== "string" || path.length === 0) { + throw new BadRequestError("`path` must be a non-empty string"); + } + // `value` must be present (use explicit `null` to clear a key). Without + // this check, `undefined` flows into `setNestedValue` and gets dropped by + // `JSON.stringify` at save time, silently removing the key - which is + // distinct from the documented "set to null" semantics. + if (!("value" in bodyRecord)) { + throw new BadRequestError( + "`value` is required (use `null` to clear a key)", + ); + } + // Build the equivalent patch shape so the managed-profile guard can + // inspect the touched subtree. + const patchShape: Record = {}; + setNestedValue(patchShape, path, value); + rejectManagedProfileDeletion(patchShape); + + const raw = loadRawConfig(); + setNestedValue(raw, path, value); + + await commitConfigWrite(raw, "set"); return { ok: true }; } +/** + * Validate the regex patterns inside the workspace's + * `secret-allowlist.json` file. + * + * Pure read: opens the file, attempts to compile each pattern, returns + * structured errors. The handler returns `{ exists: false }` if the file is + * absent, or `{ exists: true, errors: [...] }` otherwise. + */ +function handleValidateAllowlist() { + try { + const errors = validateAllowlistFile(); + if (errors == null) return { exists: false } as const; + return { exists: true, errors } as const; + } catch (err) { + // `validateAllowlistFile` does a raw `JSON.parse` on + // `secret-allowlist.json` and can throw on malformed JSON. Surface + // that as a structured `parseError` in the response payload instead + // of letting it propagate as a 500. Preserves the pre-IPC CLI + // behavior, which printed a user-readable failure and exited 1. + const message = err instanceof Error ? err.message : String(err); + return { exists: true, parseError: message, errors: [] } as const; + } +} + function handleReplaceInferenceProfile({ pathParams = {}, body, @@ -774,6 +896,49 @@ export const ROUTES: RouteDefinition[] = [ tags: ["config"], handler: handlePatchConfig, }, + { + operationId: "config_set", + endpoint: "config/set", + method: "POST", + policyKey: "config/set", + summary: "Set a single config path", + description: + "Assign a value at a dotted config path with direct-replacement semantics " + + "(preserves explicit null, replaces object subtrees instead of merging). " + + "Used by the `assistant config set ` CLI command.", + tags: ["config"], + handler: handleSetConfig, + }, + { + operationId: "config_allowlist_validate", + endpoint: "config/allowlist/validate", + method: "GET", + policyKey: "config/allowlist/validate", + summary: "Validate secret-allowlist.json regex patterns", + description: + "Compile each regex pattern in secret-allowlist.json and return any " + + "syntax errors. Returns { exists: false } if no file is present.", + tags: ["config"], + handler: handleValidateAllowlist, + }, + { + operationId: "config_schema_get", + endpoint: "config/schema", + method: "GET", + policyKey: "config/schema", + summary: "Get config JSON Schema", + description: + "Return the JSON Schema for the assistant config, optionally scoped to a dotted-path sub-schema (e.g. ?path=calls).", + tags: ["config"], + queryParams: [ + { + name: "path", + schema: { type: "string" }, + description: "Optional dotted path to a config sub-key", + }, + ], + handler: handleGetConfigSchema, + }, { operationId: "config_llm_profiles_replace", endpoint: "config/llm/profiles/:name",