From 763a3f757bd945298d408b7a593becbde3522ad5 Mon Sep 17 00:00:00 2001 From: Omri Maya Date: Wed, 8 Jul 2026 12:39:10 +0300 Subject: [PATCH 1/3] feat(cli): verb-level args, deep help, server-rendered human view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every ncl verb now declares its args: strict validation with fix-carrying error messages, generated per-verb --help, and multi-word custom-op key resolution (spaces vs dashes). Longest-prefix command fallback keeps dashed positional ids intact. Responses gain a server-rendered 'human' frame field (formatHuman hook): the host renders tables once and every client prints them verbatim — container agents cannot import host formatters, so this is how they get aligned output instead of a raw column dump. --help responses carry 'human' for clean multi-line rendering. Robustness fixes riding along: - generic list returns newest rows first, so the LIMIT no longer hides recent rows - stdout flushed before exit — >64KB responses were silently truncated Additive only: new response-frame fields, no schema changes. Co-Authored-By: Claude Fable 5 --- container/agent-runner/src/cli/ncl.ts | 7 +- src/cli/client.ts | 10 +- src/cli/commands/help.ts | 46 +++-- src/cli/crud-validate.test.ts | 173 +++++++++++++++++ src/cli/crud.ts | 115 ++++++++++- src/cli/dispatch.test.ts | 267 +++++++++++++++++++++++++- src/cli/dispatch.ts | 132 +++++++++++-- src/cli/frame.ts | 9 +- src/cli/help-render.test.ts | 95 +++++++++ src/cli/help-render.ts | 115 +++++++++++ src/cli/registry.ts | 17 +- 11 files changed, 941 insertions(+), 45 deletions(-) create mode 100644 src/cli/crud-validate.test.ts create mode 100644 src/cli/help-render.test.ts create mode 100644 src/cli/help-render.ts diff --git a/container/agent-runner/src/cli/ncl.ts b/container/agent-runner/src/cli/ncl.ts index c8353688c..f8e3290a0 100644 --- a/container/agent-runner/src/cli/ncl.ts +++ b/container/agent-runner/src/cli/ncl.ts @@ -22,7 +22,9 @@ type RequestFrame = { }; type ResponseFrame = - | { id: string; ok: true; data: unknown } + // `human` mirrors src/cli/frame.ts: an optional server-rendered string we + // print verbatim instead of running our own (drift-prone) formatter. + | { id: string; ok: true; data: unknown; human?: string } | { id: string; ok: false; error: { code: string; message: string } }; // --------------------------------------------------------------------------- @@ -244,6 +246,9 @@ if (!resp) { if (json) { process.stdout.write(JSON.stringify(resp, null, 2) + '\n'); +} else if (resp.ok && resp.human !== undefined) { + // Server-rendered view — print verbatim. + process.stdout.write(resp.human + '\n'); } else { const output = formatHuman(resp); if (!resp.ok) { diff --git a/src/cli/client.ts b/src/cli/client.ts index 0f155ca6c..612cc2910 100644 --- a/src/cli/client.ts +++ b/src/cli/client.ts @@ -43,8 +43,14 @@ async function main(): Promise { process.exit(2); } - process.stdout.write(formatResponse(res, json ? 'json' : 'human')); - process.exit(res.ok ? 0 : 1); + const output = + !json && res.ok && res.human !== undefined + ? res.human + '\n' // server-rendered view — print verbatim + : formatResponse(res, json ? 'json' : 'human'); + // Exit only after stdout drains: process.exit() discards buffered pipe + // writes, silently truncating any response past the 64KB pipe buffer + // (bit `ncl sessions list --json` at scale). + process.stdout.write(output, () => process.exit(res.ok ? 0 : 1)); } function pickTransport(): Transport { diff --git a/src/cli/commands/help.ts b/src/cli/commands/help.ts index 6ee538072..d3fad107a 100644 --- a/src/cli/commands/help.ts +++ b/src/cli/commands/help.ts @@ -5,11 +5,10 @@ * ncl groups help — show group resource details (verbs, columns, enums) */ import { getContainerConfig } from '../../db/container-configs.js'; -import { getResource, getResources } from '../crud.js'; +import { getResources } from '../crud.js'; +import { renderVerbHelp, summaryLine } from '../help-render.js'; import type { CallerContext } from '../frame.js'; -import { listCommands, register } from '../registry.js'; - -const GROUP_SCOPE_RESOURCES = new Set(['groups', 'sessions', 'destinations', 'members']); +import { GROUP_SCOPE_RESOURCES, listCommands, register } from '../registry.js'; function getCliScope(ctx: CallerContext): string | undefined { if (ctx.caller !== 'agent') return undefined; @@ -76,13 +75,28 @@ export function registerResourceHelpCommands(): void { try { register({ name: `${res.plural}-help`, - description: `Show ${res.name} resource details.`, + description: `Show ${res.name} resource details; \`${res.plural} help \` for one verb in depth.`, access: 'open', resource: res.plural, - parseArgs: () => ({}), - handler: async (_args, ctx) => { + parseArgs: (raw) => raw, + handler: async (args, ctx) => { const cliScope = getCliScope(ctx); const lines: string[] = []; + + // `ncl help ` arrives via the dispatcher's + // longest-prefix fallback (`groups-help-create` → `groups-help` + + // id=`create`) and renders that verb's deep help — full description, + // flags, examples. No new routing. Group-scope auto-fill also puts + // the caller's agent group ID into `id` on groups/destinations — + // that's not a verb request, so ignore it. + const autoFilled = ctx.caller === 'agent' && args.id === ctx.agentGroupId; + const verbArg = !autoFilled && typeof args.id === 'string' ? args.id : null; + if (verbArg) { + const deep = renderVerbHelp(res, verbArg); + if (!deep) throw new Error(`no verb "${verbArg}" on ${res.plural} — run \`ncl ${res.plural} help\``); + return deep; + } + lines.push(`${res.plural}: ${res.description}`); if (cliScope === 'group' && GROUP_SCOPE_RESOURCES.has(res.plural)) { @@ -92,23 +106,27 @@ export function registerResourceHelpCommands(): void { lines.push(''); - // Verbs + // Verbs — one summary line each; deep help is a verb away. Only the + // exceptional access levels are tagged: `open` is the unmarked default. const idAutoFilled = cliScope === 'group' && (res.plural === 'groups' || res.plural === 'destinations'); const idHint = idAutoFilled ? '' : ' '; + const tag = (access: string | undefined) => (!access || access === 'open' ? '' : ` [${access}]`); const verbs: string[] = []; - if (res.operations.list) verbs.push(`list [open]`); - if (res.operations.get) verbs.push(`get${idHint} [open]`); - if (res.operations.create) verbs.push(`create [approval]`); - if (res.operations.update) verbs.push(`update${idHint} [approval]`); - if (res.operations.delete) verbs.push(`delete${idHint} [approval]`); + if (res.operations.list) verbs.push(`list${tag(res.operations.list)}`); + if (res.operations.get) verbs.push(`get${idHint}${tag(res.operations.get)}`); + if (res.operations.create) verbs.push(`create${tag(res.operations.create)}`); + if (res.operations.update) verbs.push(`update${idHint}${tag(res.operations.update)}`); + if (res.operations.delete) verbs.push(`delete${idHint}${tag(res.operations.delete)}`); if (res.customOperations) { for (const [verb, op] of Object.entries(res.customOperations)) { - verbs.push(`${verb} [${op.access}] — ${op.description}`); + verbs.push(`${verb}${tag(op.access)} — ${summaryLine(op.description)}`); } } lines.push('Verbs:'); for (const v of verbs) lines.push(` ${v}`); lines.push(''); + lines.push(`Run \`ncl ${res.plural} help \` (or add --help to any command) for flags and examples.`); + lines.push(''); // Columns const autoFilledFields = diff --git a/src/cli/crud-validate.test.ts b/src/cli/crud-validate.test.ts new file mode 100644 index 000000000..56a0ae8d1 --- /dev/null +++ b/src/cli/crud-validate.test.ts @@ -0,0 +1,173 @@ +import { describe, it, expect, vi } from 'vitest'; + +vi.mock('../db/connection.js', () => ({ getDb: vi.fn() })); +vi.mock('../db/container-configs.js', () => ({ + getContainerConfig: vi.fn(() => ({ cli_scope: 'group' })), +})); + +import { registerResource, validateArgs } from './crud.js'; +import { registerResourceHelpCommands } from './commands/help.js'; +import { lookup } from './registry.js'; +import type { CallerContext } from './frame.js'; + +// --- validateArgs unit --- + +describe('validateArgs', () => { + const defs = [ + { name: 'target', type: 'string' as const, description: 'Target.', required: true }, + { name: 'count', type: 'number' as const, description: 'Count.', default: 1 }, + { name: 'force', type: 'boolean' as const, description: 'Force.' }, + { name: 'meta', type: 'json' as const, description: 'Meta.' }, + { name: 'size', type: 'string' as const, description: 'Size.', enum: ['s', 'm', 'l'] }, + ]; + + it('coerces types per declaration', () => { + const out = validateArgs(defs, { target: 'x', count: '5', force: 'true', meta: '{"a":1}' }); + expect(out).toMatchObject({ target: 'x', count: 5, force: true, meta: { a: 1 } }); + }); + + it('applies defaults for absent optional flags', () => { + expect(validateArgs(defs, { target: 'x' }).count).toBe(1); + }); + + it('rejects a missing required flag', () => { + expect(() => validateArgs(defs, {})).toThrow('--target is required'); + }); + + it('rejects unknown flags', () => { + expect(() => validateArgs(defs, { target: 'x', bogus: '1' })).toThrow('unknown flag --bogus'); + }); + + it('tolerates dispatch-injected keys without declaration', () => { + const out = validateArgs(defs, { target: 'x', id: 'ag-1', agent_group_id: 'ag-1', group: 'ag-1' }); + expect(out.id).toBe('ag-1'); + }); + + it('rejects enum violations', () => { + expect(() => validateArgs(defs, { target: 'x', size: 'xl' })).toThrow('--size must be one of: s, m, l'); + }); + + it('rejects non-numeric values for number flags', () => { + expect(() => validateArgs(defs, { target: 'x', count: 'many' })).toThrow('--count must be a number'); + }); + + it('rejects a value-less flag on a non-boolean (client sends true)', () => { + expect(() => validateArgs(defs, { target: true })).toThrow('--target requires a value'); + }); + + it('accepts a value-less boolean flag', () => { + expect(validateArgs(defs, { target: 'x', force: true }).force).toBe(true); + }); + + it('rejects invalid JSON', () => { + expect(() => validateArgs(defs, { target: 'x', meta: '{nope' })).toThrow('--meta must be valid JSON'); + }); +}); + +// --- registerResource wiring: strict where declared, lenient otherwise --- + +registerResource({ + name: 'widget', + plural: 'widgets', + table: 'widgets', + description: 'Test widgets.', + idColumn: 'id', + columns: [ + { name: 'id', type: 'string', description: 'UUID.', generated: true }, + { name: 'name', type: 'string', description: 'Display name.', required: true, updatable: true }, + ], + operations: {}, + customOperations: { + ping: { + access: 'open', + description: 'Ping a widget.', + args: [{ name: 'target', type: 'string', description: 'Where to ping.', required: true }], + examples: ['ncl widgets ping --target prod'], + handler: async (args) => ({ echo: args }), + }, + legacy: { + access: 'open', + description: 'Legacy op without declared args.', + handler: async (args) => ({ echo: args }), + }, + }, +}); +registerResourceHelpCommands(); + +describe('strict validation wiring (declared args)', () => { + const parse = lookup('widgets-ping')!.parseArgs; + + it('passes and coerces valid args', () => { + expect(parse({ target: 'prod' })).toMatchObject({ target: 'prod' }); + }); + + it('failure carries the verb usage block (error + fix in one round-trip)', () => { + let message = ''; + try { + parse({}); + } catch (e) { + message = (e as Error).message; + } + expect(message).toContain('--target is required'); + expect(message).toContain('ncl widgets ping'); // usage line + expect(message).toContain('Flags:'); + expect(message).toContain('Examples:'); + }); + + it('rejects unknown flags with the usage block', () => { + expect(() => parse({ target: 'prod', bogus: '1' })).toThrow(/unknown flag --bogus[\s\S]*Flags:/); + }); + + it('normalizes dashed flags before validating', () => { + // --target arrives as raw key "target"; a dashed alias like "tar-get" would + // normalize to underscores — prove normalize runs before validate. + expect(() => parse({ 'bogus-flag': '1', target: 'x' })).toThrow('unknown flag --bogus-flag'); + }); +}); + +describe('lenient ops (no declared args) keep legacy behavior', () => { + it('passes stray flags through untouched', () => { + const parse = lookup('widgets-legacy')!.parseArgs; + expect(parse({ anything: 'goes', 'dash-key': '1' })).toMatchObject({ anything: 'goes', dash_key: '1' }); + }); +}); + +// --- resource help: deep verb view + group-scope auto-fill guard --- + +describe('resource help command', () => { + const helpCmd = lookup('widgets-help')!; + const host: CallerContext = { caller: 'host' }; + const agent: CallerContext = { + caller: 'agent', + sessionId: 'sess-1', + agentGroupId: 'ag-1', + messagingGroupId: 'mg-1', + }; + + it('renders the resource overview with verb summaries', async () => { + const out = (await helpCmd.handler(helpCmd.parseArgs({}), host)) as string; + expect(out).toContain('widgets: Test widgets.'); + expect(out).toContain('ping — Ping a widget.'); + expect(out).toContain('help '); + }); + + it('renders deep help for `help ` (id from prefix fallback)', async () => { + const out = (await helpCmd.handler(helpCmd.parseArgs({ id: 'ping' }), host)) as string; + expect(out).toContain('ncl widgets ping'); + expect(out).toContain('--target'); + expect(out).toContain('Examples:'); + }); + + it('errors on an unknown verb', async () => { + await expect(helpCmd.handler(helpCmd.parseArgs({ id: 'bogus' }), host)).rejects.toThrow( + 'no verb "bogus" on widgets', + ); + }); + + it('treats an auto-filled agent group id as no verb (scoped agent, plain help)', async () => { + // dispatch auto-fills id=ctx.agentGroupId on groups/destinations; the + // handler must show the overview, not "no verb ". + const out = (await helpCmd.handler(helpCmd.parseArgs({ id: 'ag-1' }), agent)) as string; + expect(out).toContain('widgets: Test widgets.'); + }); +}); diff --git a/src/cli/crud.ts b/src/cli/crud.ts index a87d71ef8..45d64b879 100644 --- a/src/cli/crud.ts +++ b/src/cli/crud.ts @@ -9,6 +9,7 @@ import { randomUUID } from 'crypto'; import { getDb } from '../db/connection.js'; +import { renderVerbHelp } from './help-render.js'; import { register } from './registry.js'; import type { Access } from './registry.js'; import type { CallerContext } from './frame.js'; @@ -37,9 +38,21 @@ export interface ColumnDef { export interface CustomOperation { access: Access; + /** First line = one-line summary (resource help). Full text renders in the + * per-verb deep help (`ncl help ` / `--help`). */ description: string; + /** + * Declaring args opts this verb into strict validation: required/enum/type + * checks plus unknown-flag rejection, with the verb's generated usage block + * appended to every failure. Omit to keep the legacy lenient behavior + * (handler validates by hand, stray flags ignored). + */ args?: ColumnDef[]; + /** Ready-to-paste invocations, rendered under EXAMPLES in deep help. */ + examples?: string[]; handler: (args: Record, ctx: CallerContext) => Promise; + /** Presentational renderer for human mode — see CommandDef.formatHuman. */ + formatHuman?: (data: unknown) => string; } export interface ResourceDef { @@ -110,8 +123,11 @@ function genericList(def: ResourceDef) { } const where = filters.length > 0 ? ` WHERE ${filters.join(' AND ')}` : ''; params.push(limit); + // Newest first: without an ORDER BY the LIMIT silently hides the most + // recently inserted rows once a table outgrows it (bit `sessions list` + // past 200 sessions — a just-created session was invisible). return getDb() - .prepare(`SELECT ${cols} FROM ${def.table}${where} LIMIT ?`) + .prepare(`SELECT ${cols} FROM ${def.table}${where} ORDER BY rowid DESC LIMIT ?`) .all(...params); }; } @@ -222,6 +238,85 @@ function normalizeArgs(raw: Record): Record { return out; } +// --------------------------------------------------------------------------- +// Strict arg validation (opt-in via CustomOperation.args) +// --------------------------------------------------------------------------- + +/** + * Keys the dispatcher may inject into `req.args` before parseArgs runs + * (group-scope auto-fill). Strict validation must tolerate them even when a + * verb doesn't declare them, or every scoped agent call breaks. + */ +const DISPATCH_INJECTED_KEYS = ['id', 'agent_group_id', 'group'] as const; + +/** + * Validate `args` (already underscore-normalized) against a ColumnDef list: + * unknown-flag rejection, required, enum, and type coercion per the declared + * type. Returns a coerced copy; throws with a focused message on the first + * problem. Works from any ColumnDef list so generic CRUD resources can opt + * into the same strictness later without a second validator. + */ +export function validateArgs( + defs: ColumnDef[], + args: Record, + opts: { allowExtra?: readonly string[] } = {}, +): Record { + const declared = new Map(defs.map((d) => [d.name, d])); + const allowed = new Set([...declared.keys(), ...(opts.allowExtra ?? DISPATCH_INJECTED_KEYS)]); + + for (const key of Object.keys(args)) { + if (!allowed.has(key)) { + throw new Error(`unknown flag --${key.replace(/_/g, '-')}`); + } + } + + const out: Record = { ...args }; + for (const def of defs) { + const flag = `--${def.name.replace(/_/g, '-')}`; + const v = args[def.name]; + if (v === undefined) { + if (def.required) throw new Error(`${flag} is required`); + if (def.default !== undefined) out[def.name] = def.default; + continue; + } + // The client parses a value-less `--flag` as boolean true. + if (v === true && def.type !== 'boolean') { + throw new Error(`${flag} requires a value`); + } + switch (def.type) { + case 'number': { + const n = Number(v); + if (Number.isNaN(n)) throw new Error(`${flag} must be a number, got "${v}"`); + out[def.name] = n; + break; + } + case 'boolean': { + if (v === true || v === 'true' || v === '1') out[def.name] = true; + else if (v === false || v === 'false' || v === '0') out[def.name] = false; + else throw new Error(`${flag} must be true or false, got "${v}"`); + break; + } + case 'json': { + if (typeof v === 'string') { + try { + out[def.name] = JSON.parse(v); + } catch { + throw new Error(`${flag} must be valid JSON`); + } + } + break; + } + case 'string': + out[def.name] = String(v); + break; + } + if (def.enum && !def.enum.includes(String(out[def.name]))) { + throw new Error(`${flag} must be one of: ${def.enum.join(', ')}`); + } + } + return out; +} + // --------------------------------------------------------------------------- // registerResource // --------------------------------------------------------------------------- @@ -286,16 +381,30 @@ export function registerResource(def: ResourceDef): void { }); } - // Custom operations + // Custom operations. Declaring `args` opts the verb into strict validation; + // every failure carries the verb's usage block so a caller (human or agent) + // can fix the invocation without a second help round-trip. if (def.customOperations) { for (const [verb, op] of Object.entries(def.customOperations)) { + const declared = op.args; register({ name: `${def.plural}-${verb.replace(/ /g, '-')}`, description: op.description, access: op.access, resource: def.plural, - parseArgs: (raw) => normalizeArgs(raw), + parseArgs: declared + ? (raw) => { + try { + return validateArgs(declared, normalizeArgs(raw)); + } catch (e) { + const usage = renderVerbHelp(def, verb); + const msg = e instanceof Error ? e.message : String(e); + throw new Error(usage ? `${msg}\n\n${usage}` : msg); + } + } + : (raw) => normalizeArgs(raw), handler: async (args, ctx) => op.handler(args as Record, ctx), + formatHuman: op.formatHuman, }); } } diff --git a/src/cli/dispatch.test.ts b/src/cli/dispatch.test.ts index 7afd5320f..aacb812aa 100644 --- a/src/cli/dispatch.test.ts +++ b/src/cli/dispatch.test.ts @@ -174,6 +174,17 @@ register({ handler: async () => ({ agent_group_id: 'g1', model: 'opus' }), }); +// A dash-joined command whose custom-operation key contains spaces +// ('config update') — used by the --help space/dash bridging test. +register({ + name: 'groups-config-update', + description: 'bare registry description (should not be the help answer)', + resource: 'groups', + access: 'open', + parseArgs: (raw) => raw, + handler: async (args) => ({ echo: args }), +}); + // The real `sessions-get` name — triggers the pre-handler ownership check. register({ name: 'sessions-get', @@ -595,18 +606,260 @@ describe('CLI scope enforcement', () => { }); }); -// --- Dash-joined positional id resolution (generated ids contain dashes) --- +// Multi-segment command, to prove the longest-prefix match (verb itself has dashes). +register({ + name: 'groups-cfg-get', + description: 'test multi-segment command', + resource: 'groups', + access: 'open', + parseArgs: (raw) => raw, + handler: async (args) => ({ echo: args }), +}); -describe('dash-joined positional id resolution', () => { - it('resolves `groups-get-` to (groups get, id=)', async () => { - const uuid = '550e8400-e29b-41d4-a716-446655440000'; +describe('positional dashed-id resolution', () => { + const host = { caller: 'host' as const }; - const resp = await dispatch({ id: '1', command: `groups-get-${uuid}`, args: {} }, { caller: 'host' }); + it('resolves a long dashed id to command + intact id (no shredding)', async () => { + const id = 'task-374f0630-d3e0-4965-81da-fe4bf7a6a442'; + const resp = await dispatch({ id: '1', command: `groups-test-${id}`, args: {} }, host); + expect(resp.ok).toBe(true); + if (resp.ok) expect(resp.data).toEqual({ echo: { id } }); + }); + + it('matches the LONGEST command prefix when the verb itself has dashes', async () => { + const resp = await dispatch({ id: '2', command: 'groups-cfg-get-abc-123', args: {} }, host); + expect(resp.ok).toBe(true); + if (resp.ok) expect(resp.data).toEqual({ echo: { id: 'abc-123' } }); + }); + + it('leaves a registered no-id command alone', async () => { + const resp = await dispatch({ id: '3', command: 'groups-test', args: {} }, host); + expect(resp.ok).toBe(true); + if (resp.ok) expect(resp.data).toEqual({ echo: {} }); + }); + + it('does not override an explicit --id', async () => { + const resp = await dispatch({ id: '4', command: 'groups-test-tail', args: { id: 'explicit' } }, host); + expect(resp.ok).toBe(true); + if (resp.ok) expect(resp.data).toEqual({ echo: { id: 'explicit' } }); + }); +}); + +// --- `--help` interception: answer with generated help, execute nothing --- + +describe('--help interception', () => { + it('returns command help instead of executing (open command)', async () => { + const resp = await dispatch({ id: '1', command: 'test-cmd', args: { help: true } }, { caller: 'host' }); expect(resp.ok).toBe(true); if (resp.ok) { - const data = resp.data as { echo: Record }; - expect(data.echo.id).toBe(uuid); + // No deep resource def for 'test' → falls back to the description. + expect(resp.data).toBe('test command (non-group resource)'); + } + }); + + it('carries the help text in `human` so clients print it verbatim', async () => { + const resp = await dispatch({ id: '1', command: 'test-cmd', args: { help: true } }, { caller: 'host' }); + + expect(resp.ok).toBe(true); + if (resp.ok) { + expect(typeof resp.data).toBe('string'); + expect(resp.human).toBe(resp.data); + } + }); + + it('never mints an approval card for --help on an approval-gated command', async () => { + mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' }); + mockGetSession.mockReturnValue({ id: 's1', agent_group_id: 'g1', messaging_group_id: 'mg1' }); + + const resp = await dispatch({ id: '1', command: 'approval-context-command', args: { help: true } }, agentCtx()); + + expect(resp.ok).toBe(true); + expect(approvalState.requestApproval).not.toHaveBeenCalled(); + expect(approvalState.observedContexts).toHaveLength(0); // handler never ran + }); + + it('renders deep verb help when the resource def is available', async () => { + mockGetResource.mockImplementation((plural: string) => + plural === 'groups' + ? { + name: 'group', + plural: 'groups', + table: 'agent_groups', + description: 'Agent groups.', + idColumn: 'id', + scopeField: 'id', + columns: [], + operations: {}, + customOperations: { + test: { + access: 'open', + description: 'Deep test op.', + args: [{ name: 'foo', type: 'string', description: 'A foo.', required: true }], + handler: async () => ({}), + }, + }, + } + : undefined, + ); + + const resp = await dispatch({ id: '1', command: 'groups-test', args: { help: true } }, { caller: 'host' }); + + expect(resp.ok).toBe(true); + if (resp.ok) { + expect(resp.data).toContain('ncl groups test'); + expect(resp.data).toContain('--foo'); + expect(resp.data).toContain('(required)'); + } + }); + + it('renders deep verb help for a multi-word custom-operation key (spaces vs dashes)', async () => { + // registerResource stores the op under 'config update' but registers the + // command as 'groups-config-update'; help must bridge the two. + mockGetResource.mockImplementation((plural: string) => + plural === 'groups' + ? { + name: 'group', + plural: 'groups', + table: 'agent_groups', + description: 'Agent groups.', + idColumn: 'id', + scopeField: 'id', + columns: [], + operations: {}, + customOperations: { + 'config update': { + access: 'open', + description: 'Update container config.', + args: [{ name: 'model', type: 'string', description: 'Model override.' }], + handler: async () => ({}), + }, + }, + } + : undefined, + ); + + const resp = await dispatch({ id: '1', command: 'groups-config-update', args: { help: true } }, { caller: 'host' }); + + expect(resp.ok).toBe(true); + if (resp.ok) { + expect(resp.data).toContain('ncl groups config update'); + expect(resp.data).toContain('--model'); + // Not the bare registry description fallback: + expect(resp.data).not.toBe('bare registry description (should not be the help answer)'); + } + }); + + it('still enforces group scope before answering --help', async () => { + mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' }); + + const resp = await dispatch({ id: '1', command: 'wirings-list', args: { help: true } }, agentCtx()); + + expect(resp.ok).toBe(false); + if (!resp.ok) expect(resp.error.code).toBe('forbidden'); + }); +}); + +// --- Unknown-command errors carry their fix --- + +describe('unknown-command errors', () => { + it('lists the resource verbs when the command names a known resource', async () => { + mockGetResource.mockImplementation((plural: string) => + plural === 'groups' + ? { + name: 'group', + plural: 'groups', + table: 'agent_groups', + description: 'Agent groups.', + idColumn: 'id', + columns: [], + operations: { list: 'open', get: 'open' }, + customOperations: { + restart: { access: 'approval', description: 'Restart.', handler: async () => ({}) }, + }, + } + : undefined, + ); + + const resp = await dispatch({ id: '1', command: 'groups-restrat', args: {} }, { caller: 'host' }); + + expect(resp.ok).toBe(false); + if (!resp.ok) { + expect(resp.error.code).toBe('unknown-command'); + expect(resp.error.message).toContain('verbs for groups: list, get, restart'); + expect(resp.error.message).toContain('ncl groups help'); + } + }); + + it('suggests the closest command name for near-miss typos', async () => { + const resp = await dispatch({ id: '1', command: 'test-cm', args: {} }, { caller: 'host' }); + + expect(resp.ok).toBe(false); + if (!resp.ok) { + expect(resp.error.code).toBe('unknown-command'); + expect(resp.error.message).toContain('did you mean "test-cmd"?'); + } + }); + + it('falls back to a plain pointer when nothing is close', async () => { + const resp = await dispatch({ id: '1', command: 'zzz-qqq-vvv', args: {} }, { caller: 'host' }); + + expect(resp.ok).toBe(false); + if (!resp.ok) { + expect(resp.error.message).toContain('no command "zzz-qqq-vvv"'); + expect(resp.error.message).toContain('ncl help'); + expect(resp.error.message).not.toContain('did you mean'); } }); }); + +// --- formatHuman hook: server-rendered human view on the frame --- + +describe('formatHuman hook', () => { + register({ + name: 'render-cmd', + description: 'command with a human renderer', + access: 'open', + parseArgs: (raw) => raw, + handler: async () => [{ id: 'x1', status: 'live' }], + formatHuman: (rows) => `TABLE(${(rows as { id: string }[]).map((r) => r.id).join(',')})`, + }); + + register({ + name: 'render-throws', + description: 'command whose renderer throws', + access: 'open', + parseArgs: (raw) => raw, + handler: async () => ({ fine: true }), + formatHuman: () => { + throw new Error('renderer bug'); + }, + }); + + it('attaches human alongside data', async () => { + const resp = await dispatch({ id: '1', command: 'render-cmd', args: {} }, { caller: 'host' }); + + expect(resp.ok).toBe(true); + if (resp.ok) { + expect(resp.human).toBe('TABLE(x1)'); + expect(resp.data).toEqual([{ id: 'x1', status: 'live' }]); // machine contract intact + } + }); + + it('a throwing renderer degrades to a plain frame, never an error', async () => { + const resp = await dispatch({ id: '1', command: 'render-throws', args: {} }, { caller: 'host' }); + + expect(resp.ok).toBe(true); + if (resp.ok) { + expect(resp.human).toBeUndefined(); + expect(resp.data).toEqual({ fine: true }); + } + }); + + it('commands without a renderer stay human-less', async () => { + const resp = await dispatch({ id: '1', command: 'test-cmd', args: {} }, { caller: 'host' }); + + expect(resp.ok).toBe(true); + if (resp.ok) expect(resp.human).toBeUndefined(); + }); +}); diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index f0dc430cb..5ea9f65c3 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -12,7 +12,8 @@ import { getSession } from '../db/sessions.js'; import { registerApprovalHandler, requestApproval } from '../modules/approvals/index.js'; import type { CallerContext, ErrorCode, RequestFrame, ResponseFrame } from './frame.js'; import { getResource } from './crud.js'; -import { lookup } from './registry.js'; +import { listVerbs, renderVerbHelp } from './help-render.js'; +import { GROUP_SCOPE_RESOURCES, listCommands, lookup } from './registry.js'; type DispatchOptions = { /** True when a command is being replayed after approval. */ @@ -26,21 +27,22 @@ export async function dispatch( ): Promise { let cmd = lookup(req.command); - // Fallback: if the full command isn't registered, split the dash-joined - // command and treat the longest registered prefix as the command, with the - // re-joined remainder as the target ID. Clients join all positional args - // with dashes (e.g. `ncl groups get abc123` → command "groups-get-abc123"), - // and generated ids (UUIDs, `sess-…`, `appr-…`) themselves contain dashes, - // so trimming a single trailing segment isn't enough — walk prefixes from - // longest to shortest so `groups-get-` still resolves to - // "groups-get" + id "". + // Fallback: if the full command isn't registered, find the LONGEST registered + // command that is a dash-prefix of req.command; the remainder is the target ID, + // kept intact (dashes and all). This lets clients join all positional args with + // dashes — e.g. `ncl groups get abc123` → "groups-get-abc123" → "groups-get" + + // id "abc123", and crucially `ncl tasks cancel task-374f-...-442` → + // "tasks-cancel" + id "task-374f-...-442" (a dashed id is no longer shredded). + // Trimming from the end (longest→shortest) means a multi-segment verb like + // "groups-config-add-mcp-server" still matches before any shorter prefix. if (!cmd) { - const parts = req.command.split('-'); - for (let i = parts.length - 1; i > 0; i--) { - const shortened = parts.slice(0, i).join('-'); + let shortened = req.command; + let idx: number; + while ((idx = shortened.lastIndexOf('-')) > 0) { + shortened = shortened.slice(0, idx); const fallback = lookup(shortened); if (fallback) { - const tail = parts.slice(i).join('-'); + const tail = req.command.slice(shortened.length + 1); // full remainder = id, dashes intact cmd = fallback; req = { ...req, command: shortened, args: { ...req.args, id: req.args.id ?? tail } }; break; @@ -49,7 +51,7 @@ export async function dispatch( } if (!cmd) { - return err(req.id, 'unknown-command', `no command "${req.command}"`); + return err(req.id, 'unknown-command', unknownCommandMessage(req.command)); } // CLI scope enforcement for agent callers @@ -62,9 +64,8 @@ export async function dispatch( } if (cliScope === 'group') { - const allowed = new Set(['groups', 'sessions', 'destinations', 'members']); // Only allow whitelisted resources and general commands (no resource, like help) - if (cmd.resource && !allowed.has(cmd.resource)) { + if (cmd.resource && !GROUP_SCOPE_RESOURCES.has(cmd.resource)) { return err(req.id, 'forbidden', `CLI access is scoped to this agent group. Cannot access "${cmd.resource}".`); } @@ -115,6 +116,17 @@ export async function dispatch( } } + // `--help` interception: answer with the command's generated help instead of + // executing. Placed after scope enforcement (a group-scoped agent can't probe + // forbidden resources) and BEFORE approval gating — asking for help on an + // approval-gated verb must never mint an approval card. + if (req.args.help === true) { + // Carry the help text in `human` too, so both clients print it verbatim + // as clean multi-line text instead of a JSON-stringified blob. + const helpText = commandHelp(cmd.name, cmd.resource, cmd.description); + return { id: req.id, ok: true, data: helpText, human: helpText }; + } + if (ctx.caller !== 'host' && cmd.access === 'approval' && !opts.approved) { const session = getSession(ctx.sessionId); if (!session) { @@ -186,6 +198,17 @@ export async function dispatch( } } + // Server-render the human view once, so every transport — host CLI and + // the Bun container client (which can't import host formatters) — prints + // one canonical rendering. Runs after scope filtering; a throwing + // formatter degrades to plain `data`, never fails the response. + if (cmd.formatHuman) { + try { + return { id: req.id, ok: true, data, human: cmd.formatHuman(data) }; + } catch { + // fall through to the plain frame + } + } return { id: req.id, ok: true, data }; } catch (e) { return err(req.id, 'handler-error', errMsg(e)); @@ -225,6 +248,83 @@ function parseCallerContext(value: unknown): CallerContext | undefined { return undefined; } +/** Help text for a resolved command: deep verb help when derivable, else description. */ +function commandHelp(name: string, resource: string | undefined, description: string): string { + if (resource && name.startsWith(`${resource}-`)) { + const res = getResource(resource); + const verb = name.slice(resource.length + 1); + const deep = res && renderVerbHelp(res, verb); + if (deep) return deep; + // Custom-operation KEYS may contain spaces ('config update') while command + // names are dash-joined ('groups-config-update'). Resolve by matching keys + // normalized the same way registerResource builds command names. + if (res?.customOperations) { + const spaced = Object.keys(res.customOperations).find((k) => k.replace(/ /g, '-') === verb); + const deepSpaced = spaced && renderVerbHelp(res, spaced); + if (deepSpaced) return deepSpaced; + } + } + return description; +} + +/** + * Unknown-command error that carries its fix: if the command names a known + * resource, list that resource's verbs; otherwise suggest the closest + * registered command. Resource detection walks dash-prefixes longest-first, + * same as the ID fallback above, so multi-word plurals (messaging-groups, + * user-dms) resolve. + */ +function unknownCommandMessage(command: string): string { + const parts = command.split('-'); + for (let i = parts.length; i > 0; i--) { + const prefix = parts.slice(0, i).join('-'); + const res = getResource(prefix); + if (res) { + return ( + `no command "${command}" — verbs for ${res.plural}: ${listVerbs(res).join(', ')}. ` + + `Run \`ncl ${res.plural} help \` for flags and examples.` + ); + } + } + const names = listCommands() + .filter((c) => c.access !== 'hidden') + .map((c) => c.name); + const closest = closestName(command, names); + return `no command "${command}"${closest ? ` — did you mean "${closest}"?` : ''} Run \`ncl help\`.`; +} + +/** Closest name by edit distance, only when convincingly close (≤2 edits). */ +function closestName(input: string, names: string[]): string | undefined { + let best: string | undefined; + let bestDist = 3; + for (const name of names) { + if (Math.abs(name.length - input.length) >= bestDist) continue; + const d = editDistance(input, name, bestDist); + if (d < bestDist) { + bestDist = d; + best = name; + } + } + return best; +} + +function editDistance(a: string, b: string, cap: number): number { + const prev = new Array(b.length + 1).fill(0).map((_, i) => i); + for (let i = 1; i <= a.length; i++) { + let diag = prev[0]; + prev[0] = i; + let rowMin = prev[0]; + for (let j = 1; j <= b.length; j++) { + const tmp = prev[j]; + prev[j] = Math.min(prev[j] + 1, prev[j - 1] + 1, diag + (a[i - 1] === b[j - 1] ? 0 : 1)); + diag = tmp; + rowMin = Math.min(rowMin, prev[j]); + } + if (rowMin >= cap) return cap; + } + return prev[b.length]; +} + function err(id: string, code: ErrorCode, message: string): ResponseFrame { return { id, ok: false, error: { code, message } }; } diff --git a/src/cli/frame.ts b/src/cli/frame.ts index 23e2fd70b..2365278eb 100644 --- a/src/cli/frame.ts +++ b/src/cli/frame.ts @@ -18,7 +18,14 @@ export type RequestFrame = { }; export type ResponseFrame = - | { id: string; ok: true; data: unknown } + // `human` is an optional server-rendered presentational string. It lets + // every transport — host CLI and the Bun container client — print one + // canonical rendering without importing host-only formatters (the two + // runtimes share no modules, so client-side formatters drift). `data` + // stays the machine contract; --json callers ignore `human`. Additive: + // old clients that don't know the field just fall back to their own + // rendering of `data`. + | { id: string; ok: true; data: unknown; human?: string } | { id: string; ok: false; error: { code: ErrorCode; message: string } }; export type ErrorCode = diff --git a/src/cli/help-render.test.ts b/src/cli/help-render.test.ts new file mode 100644 index 000000000..bd7ac4a95 --- /dev/null +++ b/src/cli/help-render.test.ts @@ -0,0 +1,95 @@ +import { describe, it, expect } from 'vitest'; + +import type { ResourceDef } from './crud.js'; +import { listVerbs, renderVerbHelp, summaryLine } from './help-render.js'; + +const res: ResourceDef = { + name: 'widget', + plural: 'widgets', + table: 'widgets', + description: 'Test widgets.', + idColumn: 'id', + columns: [ + { name: 'id', type: 'string', description: 'UUID.', generated: true }, + { name: 'name', type: 'string', description: 'Display name.', required: true, updatable: true }, + { name: 'size', type: 'string', description: 'Widget size.', enum: ['s', 'm', 'l'], default: 'm' }, + ], + operations: { list: 'open', get: 'open', create: 'approval', update: 'approval' }, + customOperations: { + ping: { + access: 'open', + description: 'Ping a widget.\nLonger prose that only deep help shows.', + args: [ + { name: 'target', type: 'string', description: 'Where to ping.', required: true }, + { name: 'count', type: 'number', description: 'How many times.', default: 1 }, + ], + examples: ['ncl widgets ping --target prod --count 3'], + handler: async () => ({}), + }, + }, +}; + +describe('listVerbs', () => { + it('lists enabled generics then custom verbs', () => { + expect(listVerbs(res)).toEqual(['list', 'get', 'create', 'update', 'ping']); + }); +}); + +describe('renderVerbHelp — custom operation', () => { + it('renders usage, full description, flags with tags, and examples', () => { + const out = renderVerbHelp(res, 'ping')!; + expect(out).toContain('ncl widgets ping'); + expect(out).toContain('Longer prose that only deep help shows.'); + expect(out).toContain('--target'); + expect(out).toContain('(required)'); + expect(out).toContain('--count'); + expect(out).toContain('default: 1'); + expect(out).toContain('Examples:'); + expect(out).toContain('ncl widgets ping --target prod --count 3'); + }); + + it('tags non-open access on the usage line', () => { + const gated: ResourceDef = { + ...res, + customOperations: { ping: { ...res.customOperations!.ping, access: 'approval' } }, + }; + expect(renderVerbHelp(gated, 'ping')).toContain('ncl widgets ping [approval]'); + }); +}); + +describe('renderVerbHelp — generic verbs', () => { + it('create renders non-generated columns as flags', () => { + const out = renderVerbHelp(res, 'create')!; + expect(out).toContain('ncl widgets create [approval]'); + expect(out).toContain('--name'); + expect(out).toContain('(required)'); + expect(out).toContain('--size'); + expect(out).toContain('values: s | m | l'); + expect(out).not.toContain('--id'); // generated + }); + + it('update renders only updatable columns and takes ', () => { + const out = renderVerbHelp(res, 'update')!; + expect(out).toContain('ncl widgets update [approval]'); + expect(out).toContain('--name'); + expect(out).not.toContain('--size'); + }); + + it('list renders filter flags plus --limit, never marked required', () => { + const out = renderVerbHelp(res, 'list')!; + expect(out).toContain('--limit'); + expect(out).toContain('--name'); + expect(out).not.toContain('(required)'); + }); + + it('returns undefined for verbs the resource does not have', () => { + expect(renderVerbHelp(res, 'delete')).toBeUndefined(); // not in operations + expect(renderVerbHelp(res, 'bogus')).toBeUndefined(); + }); +}); + +describe('summaryLine', () => { + it('returns only the first line', () => { + expect(summaryLine('Ping a widget.\nLonger prose.')).toBe('Ping a widget.'); + }); +}); diff --git a/src/cli/help-render.ts b/src/cli/help-render.ts new file mode 100644 index 000000000..570e65178 --- /dev/null +++ b/src/cli/help-render.ts @@ -0,0 +1,115 @@ +/** + * Pure renderers for command help. Single source for three surfaces that must + * never disagree: + * - `ncl help []` (commands/help.ts) + * - `--help` on any command (dispatch interception) + * - the usage block appended to invalid-args errors (crud.ts validation) + * + * Imports only types from crud.ts, so crud.ts can import these functions at + * runtime without a cycle. + */ +import type { ColumnDef, CustomOperation, ResourceDef } from './crud.js'; + +const GENERIC_VERBS = ['list', 'get', 'create', 'update', 'delete'] as const; +type GenericVerb = (typeof GENERIC_VERBS)[number]; + +export function flagName(col: Pick): string { + return `--${col.name.replace(/_/g, '-')}`; +} + +/** First line of a possibly multi-paragraph description. */ +export function summaryLine(description: string): string { + return description.split('\n', 1)[0]; +} + +/** Indent every non-empty line of a block by `pad`. */ +export function indent(text: string, pad: string): string { + return text + .split('\n') + .map((l) => (l ? pad + l : l)) + .join('\n'); +} + +function flagLine(col: ColumnDef, extraTags: string[] = []): string { + const tags: string[] = [...extraTags]; + if (col.required) tags.push('required'); + if (col.default !== undefined && col.default !== null) tags.push(`default: ${col.default}`); + if (col.enum) tags.push(`values: ${col.enum.join(' | ')}`); + const tagStr = tags.length > 0 ? ` (${tags.join(', ')})` : ''; + return ` ${flagName(col).padEnd(28)} ${summaryLine(col.description)}${tagStr}`; +} + +/** All verbs a resource exposes, generics first, in help order. */ +export function listVerbs(res: ResourceDef): string[] { + const verbs: string[] = GENERIC_VERBS.filter((v) => res.operations[v]); + if (res.customOperations) verbs.push(...Object.keys(res.customOperations)); + return verbs; +} + +/** Flags a generic verb accepts, derived from the resource's columns. */ +function genericFlags(res: ResourceDef, verb: GenericVerb): ColumnDef[] { + switch (verb) { + case 'create': + return res.columns.filter((c) => !c.generated); + case 'update': + return res.columns.filter((c) => c.updatable); + case 'list': + // Non-generated columns double as equality filters. + return [ + ...res.columns.filter((c) => !c.generated).map((c) => ({ ...c, required: false })), + { name: 'limit', type: 'number', description: 'Max rows returned.', default: 200 } as ColumnDef, + ]; + case 'get': + case 'delete': + return []; + } +} + +function genericSummary(res: ResourceDef, verb: GenericVerb): string { + switch (verb) { + case 'list': + return `List ${res.plural}. Flags below act as equality filters.`; + case 'get': + return `Get a ${res.name} by ID.`; + case 'create': + return `Create a new ${res.name}.`; + case 'update': + return `Update a ${res.name} by ID. Provide at least one updatable flag.`; + case 'delete': + return `Delete a ${res.name} by ID.`; + } +} + +/** + * Deep help for one verb: usage line, full description, flags, examples. + * `verb` is a custom-operation key or a generic CRUD verb. Returns undefined + * for a verb the resource doesn't have. + */ +export function renderVerbHelp(res: ResourceDef, verb: string): string | undefined { + const op: CustomOperation | undefined = res.customOperations?.[verb]; + const generic = !op && (GENERIC_VERBS as readonly string[]).includes(verb) ? (verb as GenericVerb) : undefined; + if (!op && !generic) return undefined; + if (generic && !res.operations[generic]) return undefined; + + const access = op ? op.access : res.operations[generic!]; + const accessTag = access && access !== 'open' ? ` [${access}]` : ''; + const needsId = generic === 'get' || generic === 'update' || generic === 'delete'; + + const lines: string[] = []; + lines.push(`ncl ${res.plural} ${verb}${needsId ? ' ' : ''}${accessTag}`); + lines.push(''); + lines.push(op ? op.description : genericSummary(res, generic!)); + + const flags = op ? (op.args ?? []) : genericFlags(res, generic!); + if (flags.length > 0) { + lines.push(''); + lines.push('Flags:'); + for (const f of flags) lines.push(flagLine(f)); + } + if (op?.examples?.length) { + lines.push(''); + lines.push('Examples:'); + for (const ex of op.examples) lines.push(indent(ex, ' ')); + } + return lines.join('\n'); +} diff --git a/src/cli/registry.ts b/src/cli/registry.ts index 8c0019d97..fff47ae4a 100644 --- a/src/cli/registry.ts +++ b/src/cli/registry.ts @@ -10,7 +10,14 @@ */ import type { CallerContext } from './frame.js'; -export type Access = 'open' | 'approval'; +/** + * Resources an agent under `cli_scope=group` may touch. Single source — + * consumed by both dispatch enforcement and `ncl help` filtering, so the + * agent is never shown a resource the gate would reject (or vice versa). + */ +export const GROUP_SCOPE_RESOURCES = new Set(['groups', 'sessions', 'destinations', 'members']); + +export type Access = 'open' | 'approval' | 'hidden'; export type CommandDef = { name: string; @@ -34,6 +41,14 @@ export type CommandDef = { /** Validates `frame.args` and produces the typed handler input. Throws on invalid. */ parseArgs: (raw: Record) => TArgs; handler: (args: TArgs, ctx: CallerContext) => Promise; + /** + * Optional presentational renderer. When set, dispatch attaches its output + * as the response frame's `human` field (server-rendered once, printed + * verbatim by every client in human mode). Runs after post-handler scope + * filtering, so it only ever sees data the caller is allowed to see. A + * throwing formatter is ignored — clients fall back to rendering `data`. + */ + formatHuman?: (data: TData) => string; }; const registry = new Map(); From a8554c6248a8df1089ac08133fc3bafc31044b82 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 8 Jul 2026 11:29:40 +0000 Subject: [PATCH 2/3] chore: bump version to 2.1.40 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 22b43bc96..5e850dcd7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "nanoclaw", - "version": "2.1.39", + "version": "2.1.40", "description": "Personal Claude assistant. Lightweight, secure, customizable.", "type": "module", "packageManager": "pnpm@10.33.0", From 9a103f4f932a57873d4fc808ab8efaeeb0353f69 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 8 Jul 2026 11:29:43 +0000 Subject: [PATCH 3/3] =?UTF-8?q?docs:=20update=20token=20count=20to=20213k?= =?UTF-8?q?=20tokens=20=C2=B7=20106%=20of=20context=20window?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- repo-tokens/badge.svg | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/repo-tokens/badge.svg b/repo-tokens/badge.svg index a9941f19b..cb3751d41 100644 --- a/repo-tokens/badge.svg +++ b/repo-tokens/badge.svg @@ -1,5 +1,5 @@ - - 209k tokens, 104% of context window + + 213k tokens, 106% of context window @@ -15,8 +15,8 @@ tokens - - 209k + + 213k