diff --git a/CHANGELOG.md b/CHANGELOG.md index dfce231..9265ab3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,25 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.11.1] - 2026-06-05 + +Stability pass (from an in-depth adversarial review). + +### Fixed + +- **Reliability:** pagination no longer loops forever on a non-numeric `pages` value (both API clients); the 429 backoff no longer busy-loops when `Retry-After` is non-numeric (e.g. an HTTP-date) — both now fall back to a sane delay / terminate. +- **MCP — safety & robustness:** + - Tool argument values can no longer be reinterpreted as CLI flags (argv injection): flags are passed as `--name=value` and positional args after a `--` separator. + - A signal-killed tool subprocess is reported as an error instead of silent success with partial output. + - Tool calls now have a timeout and an output-size cap, so a hung or runaway command can't hang or OOM the server. + - `mcp serve` forwards the active `--profile` to tool calls (previously they silently ran under the default account). + - Integer arguments accept JSON numbers (`{id: 123}`), not only strings. + - `auth login/logout/refresh/setup` and `docs auth` are no longer exposed as tools (they manage local credentials and can open a browser / bind a port on the host); `workflow run` is now flagged destructive; the inert `yes` input was removed from delete tools. +- **Homebrew:** `--jq` now works — the formula depends on `jq` and points node-jq at it (its bundled binary can't be downloaded in the Homebrew sandbox). +- **Reports:** `--output csv`/`table` now fail with a clear message (reports are nested JSON) instead of emitting nothing. +- **Docs API:** `--text @missing-file` now reports a clear error instead of a raw stack trace. +- **Keychain:** a write with no usable keychain (e.g. in a container) gives the friendly "keychain unavailable" guidance instead of a raw `PermissionDenied`. + ## [0.11.0] - 2026-06-05 ### Added diff --git a/docs/commands.md b/docs/commands.md index cdb216b..2881da0 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -5,7 +5,7 @@ description: Full command reference for the hscli command-line interface. -Reference for `hscli` v0.11.0 (89 commands). Every command also accepts the global flags `--output table|json|yaml|csv`, `--jq`, `--fields`, `--profile`, `--no-color`, `--verbose`, `--no-retry`, and `--timeout`. +Reference for `hscli` v0.11.1 (89 commands). Every command also accepts the global flags `--output table|json|yaml|csv`, `--jq`, `--fields`, `--profile`, `--no-color`, `--verbose`, `--no-retry`, and `--timeout`. ## Top-level diff --git a/package-lock.json b/package-lock.json index 93817be..fad3400 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@wavyx/hscli", - "version": "0.11.0", + "version": "0.11.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@wavyx/hscli", - "version": "0.11.0", + "version": "0.11.1", "license": "MIT", "dependencies": { "@inquirer/prompts": "8.5.2", diff --git a/package.json b/package.json index 74ec075..6296a95 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@wavyx/hscli", - "version": "0.11.0", + "version": "0.11.1", "publishConfig": { "access": "public" }, diff --git a/scripts/gen-dist.mjs b/scripts/gen-dist.mjs index 0f36c52..c91ac0c 100644 --- a/scripts/gen-dist.mjs +++ b/scripts/gen-dist.mjs @@ -19,15 +19,23 @@ export function renderHomebrewFormula({ url, sha256 }) { sha256 "${sha256}" license "MIT" + depends_on "jq" depends_on "node" def install system "npm", "install", *std_npm_args - bin.install_symlink Dir["#{libexec}/bin/*"] + # hscli's --jq flag uses node-jq, which normally downloads its own jq binary + # via a postinstall script. std_npm_args passes --ignore-scripts and the + # Homebrew build sandbox blocks network, so point node-jq at the Homebrew jq + # instead (node-jq honors $JQ_PATH at runtime). + (bin/"hscli").write_env_script libexec/"bin/hscli", + JQ_PATH: Formula["jq"].opt_bin/"jq" end test do assert_match "hscli", shell_output("#{bin}/hscli version") + # Exercise --jq so the node-jq / Homebrew-jq wiring can't silently regress. + system bin/"hscli", "config", "list", "--output", "json", "--jq", "." end end ` diff --git a/src/commands/mcp/serve.js b/src/commands/mcp/serve.js index 2dd8da8..593ed1f 100644 --- a/src/commands/mcp/serve.js +++ b/src/commands/mcp/serve.js @@ -64,10 +64,12 @@ export default class MCPServeCommand extends BaseCommand { async run() { const { flags } = await this.parse(MCPServeCommand) // Each tool call re-invokes this same CLI as a child process, keeping the - // parent's stdout (the MCP stdio channel) free of command output. + // parent's stdout (the MCP stdio channel) free of command output. Forward + // the active profile so tools run under the same account as the server. const exec = makeExec({ command: process.execPath, args: [process.argv[1]], + env: { HSCLI_PROFILE: this.activeProfile }, }) await startMcpServer({ config: this.config, diff --git a/src/commands/report/company.js b/src/commands/report/company.js index 887db32..b24eee3 100644 --- a/src/commands/report/company.js +++ b/src/commands/report/company.js @@ -1,5 +1,6 @@ import { Flags } from '@oclif/core' import BaseCommand from '../../base-command.js' +import { assertReportFormat } from '../../lib/report-format.js' export default class ReportCompanyCommand extends BaseCommand { static description = 'Get company report' @@ -23,6 +24,7 @@ export default class ReportCompanyCommand extends BaseCommand { async run() { const { flags } = await this.parse(ReportCompanyCommand) this.flags.output = this.flags.output || 'json' + assertReportFormat(this.flags.output) const query = { start: flags.start, diff --git a/src/commands/report/conversations.js b/src/commands/report/conversations.js index cce0523..70f1131 100644 --- a/src/commands/report/conversations.js +++ b/src/commands/report/conversations.js @@ -1,5 +1,6 @@ import { Flags } from '@oclif/core' import BaseCommand from '../../base-command.js' +import { assertReportFormat } from '../../lib/report-format.js' export default class ReportConversationsCommand extends BaseCommand { static description = 'Get conversations report' @@ -23,6 +24,7 @@ export default class ReportConversationsCommand extends BaseCommand { async run() { const { flags } = await this.parse(ReportConversationsCommand) this.flags.output = this.flags.output || 'json' + assertReportFormat(this.flags.output) const query = { start: flags.start, diff --git a/src/commands/report/user.js b/src/commands/report/user.js index 9263709..758ef12 100644 --- a/src/commands/report/user.js +++ b/src/commands/report/user.js @@ -1,5 +1,6 @@ import { Flags } from '@oclif/core' import BaseCommand from '../../base-command.js' +import { assertReportFormat } from '../../lib/report-format.js' export default class ReportUserCommand extends BaseCommand { static description = 'Get user report' @@ -21,6 +22,7 @@ export default class ReportUserCommand extends BaseCommand { async run() { const { flags } = await this.parse(ReportUserCommand) this.flags.output = this.flags.output || 'json' + assertReportFormat(this.flags.output) const query = { start: flags.start, diff --git a/src/lib/client.js b/src/lib/client.js index 1562645..4f889cd 100644 --- a/src/lib/client.js +++ b/src/lib/client.js @@ -76,7 +76,9 @@ export function createClient({ debug('%s %s → %d', method, path, res.status) if (res.status === 429) { - const wait = Number(res.headers.get('x-ratelimit-retry-after') || 10) + const raw = res.headers.get('x-ratelimit-retry-after') + const parsed = raw == null ? NaN : Number(raw) + const wait = Number.isFinite(parsed) && parsed >= 0 ? parsed : 10 if (!retry) throw new RateLimitError(wait) debug('rate limited, waiting %ds', wait) await sleep(wait * 1000) @@ -124,7 +126,10 @@ export function createClient({ query: { ...query, page }, }) const items = data?._embedded?.[resourceKey] ?? [] - const totalPages = data?.page?.totalPages ?? 1 + // Guard against a non-numeric/missing total so we never loop forever. + const rawPages = Number(data?.page?.totalPages) + const totalPages = + Number.isFinite(rawPages) && rawPages >= 1 ? rawPages : page if (opts.onProgress) opts.onProgress({ page, totalPages }) yield* items if (page >= totalPages) break diff --git a/src/lib/docs-client.js b/src/lib/docs-client.js index ff293ac..55a6724 100644 --- a/src/lib/docs-client.js +++ b/src/lib/docs-client.js @@ -73,11 +73,10 @@ export function createDocsClient({ debug('%s %s → %d', method, path, res.status) if (res.status === 429) { - const wait = Number( - res.headers.get('x-ratelimit-reset') || - res.headers.get('retry-after') || - 10, - ) + const raw = + res.headers.get('x-ratelimit-reset') || res.headers.get('retry-after') + const parsed = raw == null ? NaN : Number(raw) + const wait = Number.isFinite(parsed) && parsed >= 0 ? parsed : 10 if (!retry) throw new RateLimitError(wait) debug('rate limited, waiting %ds', wait) await sleep(wait * 1000) @@ -116,7 +115,10 @@ export function createDocsClient({ const data = await request('GET', path, { query: { ...query, page } }) const wrap = data?.[resourceKey] ?? {} const items = wrap.items ?? [] - const totalPages = wrap.pages ?? 1 + // Guard against a non-numeric/missing `pages` so we never loop forever. + const rawPages = Number(wrap.pages) + const totalPages = + Number.isFinite(rawPages) && rawPages >= 1 ? rawPages : page if (opts.onProgress) opts.onProgress({ page, totalPages }) yield* items if (page >= totalPages) break diff --git a/src/lib/docs-input.js b/src/lib/docs-input.js index fdb6d6f..bc792bd 100644 --- a/src/lib/docs-input.js +++ b/src/lib/docs-input.js @@ -1,4 +1,5 @@ import { readFileSync } from 'node:fs' +import { CliError } from './errors.js' /** * Resolve article text: a leading `@` reads the rest as a file path; otherwise @@ -8,7 +9,14 @@ import { readFileSync } from 'node:fs' */ export function readText(value) { if (value && value.startsWith('@')) { - return readFileSync(value.slice(1), 'utf8') + const path = value.slice(1) + try { + return readFileSync(path, 'utf8') + } catch (err) { + throw new CliError(`Cannot read --text file '${path}': ${err.message}`, { + exitCode: 66, + }) + } } return value } diff --git a/src/lib/keychain.js b/src/lib/keychain.js index 01f08b0..968e324 100644 --- a/src/lib/keychain.js +++ b/src/lib/keychain.js @@ -60,7 +60,13 @@ export async function getTokens(profile) { export async function setTokens(profile, tokens) { if (!Entry) keychainRequired() const account = `${profile}/tokens` - getEntry(account).setPassword(JSON.stringify(tokens)) + try { + getEntry(account).setPassword(JSON.stringify(tokens)) + } catch (err) { + // e.g. PermissionDenied in a container with no Secret Service. + debug('setTokens error: %s', err.message) + keychainRequired() + } } /** @param {string} profile */ @@ -91,7 +97,12 @@ export async function getDocsKey(profile) { */ export async function setDocsKey(profile, apiKey) { if (!Entry) keychainRequired() - getEntry(`${profile}/docs-key`).setPassword(apiKey) + try { + getEntry(`${profile}/docs-key`).setPassword(apiKey) + } catch (err) { + debug('setDocsKey error: %s', err.message) + keychainRequired() + } } /** @param {string} profile */ diff --git a/src/lib/mcp/catalog.js b/src/lib/mcp/catalog.js index 35dacb0..7d83cfe 100644 --- a/src/lib/mcp/catalog.js +++ b/src/lib/mcp/catalog.js @@ -8,7 +8,23 @@ // - `conv:watch` is a long-running stream that doesn't fit request/response. // - `doctor` is a local-environment diagnostic (live network probe), not useful to an agent. // - `mcp:serve` is this server itself — exposing it would let a tool spawn another server. -export const EXCLUDED = new Set(['api', 'conv:watch', 'doctor', 'mcp:serve']) +// - `auth:*` / `docs:auth` manage the operator's LOCAL credentials (login opens a +// browser + binds a port on the host); they make no sense as agent tools. +export const EXCLUDED = new Set([ + 'api', + 'conv:watch', + 'doctor', + 'mcp:serve', + 'auth:login', + 'auth:logout', + 'auth:refresh', + 'auth:setup', + 'docs:auth', +]) + +// Commands that mutate broadly and must carry the destructive hint even though +// their leaf verb isn't delete/remove/bulk. +const DESTRUCTIVE_IDS = new Set(['workflow:run']) // Topics whose every command is read-only. const READ_TOPICS = new Set(['report', 'beacon']) @@ -45,7 +61,11 @@ export function classifyKind(id) { const leaf = id.split(':').pop() // delete/remove and bulk operations hit data destructively — flag them so MCP // clients prompt before running them. - if (/^(delete|remove)(-|$)/.test(leaf) || leaf.startsWith('bulk')) { + if ( + DESTRUCTIVE_IDS.has(id) || + /^(delete|remove)(-|$)/.test(leaf) || + leaf.startsWith('bulk') + ) { return 'destructive' } if (WRITE_OVERRIDE.has(id)) return 'write' diff --git a/src/lib/mcp/invoke.js b/src/lib/mcp/invoke.js index f6cd450..6125c79 100644 --- a/src/lib/mcp/invoke.js +++ b/src/lib/mcp/invoke.js @@ -1,10 +1,10 @@ import { spawn } from 'node:child_process' /** - * Turn a validated tool input into an argv for the hscli CLI. Positional args - * first (in definition order), then flags, then a forced `--output json`. For - * commands that support `--yes`, it is appended so confirm prompts never block - * (there is no TTY in MCP stdio mode). + * Turn a validated tool input into an argv for the hscli CLI: flags as + * `--name=value` (so a value can never be reinterpreted as a flag), then a + * forced `--output=json` and `--yes` (no TTY for confirms), then positional + * args after a `--` separator (so a value starting with `-` stays a positional). * @param {{id: string, args?: object, flags?: object}} entry * @param {Record} input * @returns {string[]} @@ -12,9 +12,10 @@ import { spawn } from 'node:child_process' export function toArgv(entry, input) { const argv = entry.id.split(':') + const positionals = [] for (const name of Object.keys(entry.args || {})) { const v = input[name] - if (v !== undefined && v !== null) argv.push(String(v)) + if (v !== undefined && v !== null) positionals.push(String(v)) } for (const [name, flag] of Object.entries(entry.flags || {})) { @@ -24,14 +25,15 @@ export function toArgv(entry, input) { if (flag.type === 'boolean') { if (v) argv.push(`--${name}`) } else if (Array.isArray(v)) { - for (const item of v) argv.push(`--${name}`, String(item)) + for (const item of v) argv.push(`--${name}=${item}`) } else { - argv.push(`--${name}`, String(v)) + argv.push(`--${name}=${v}`) } } - argv.push('--output', 'json') + argv.push('--output=json') if (entry.flags && 'yes' in entry.flags) argv.push('--yes') + if (positionals.length) argv.push('--', ...positionals) return argv } @@ -43,22 +45,21 @@ function wrap(data) { /** * Run a tool by executing the underlying command and shaping its output into an - * MCP tool result. + * MCP tool result. A non-zero exit OR a signal termination is reported as an + * error (a signal-killed child must never look like success). * @param {object} entry catalog entry * @param {Record} input - * @param {(argv: string[]) => Promise<{stdout: string, stderr: string, code: number}>} exec + * @param {(argv: string[]) => Promise<{stdout: string, stderr: string, code: number, signal?: string|null}>} exec */ export async function runTool(entry, input, exec) { const argv = toArgv(entry, input) - const { stdout, stderr, code } = await exec(argv) + const { stdout, stderr, code, signal } = await exec(argv) - if (code !== 0) { - return { - content: [ - { type: 'text', text: (stderr || stdout || `exited ${code}`).trim() }, - ], - isError: true, - } + if (signal || code !== 0) { + const text = signal + ? `terminated: ${signal}` + : (stderr || stdout || `exited ${code}`).trim() + return { content: [{ type: 'text', text }], isError: true } } const text = stdout.trim() @@ -71,12 +72,6 @@ export async function runTool(entry, input, exec) { return result } -/** - * Build an executor that spawns the hscli CLI as a child process. Keeping - * command output in a child process keeps the parent's stdout (the MCP stdio - * channel) clean. - * @param {{command: string, args?: string[]}} options - */ /** A process killed by a signal reports a null exit code; treat that as 0. */ export function normalizeExit(code) { return code ?? 0 @@ -87,21 +82,49 @@ export function errMessage(e) { return String(e?.message || e) } -export function makeExec({ command, args = [] }) { +/** + * Build an executor that spawns the hscli CLI as a child process. Keeping + * command output in a child process keeps the parent's stdout (the MCP stdio + * channel) clean. Guards against hangs (timeout) and runaway output (maxBuffer). + * @param {{command: string, args?: string[], env?: object, timeout?: number, maxBuffer?: number}} options + */ +export function makeExec({ + command, + args = [], + env, + timeout = 120_000, + maxBuffer = 16 * 1024 * 1024, +}) { return (argv) => new Promise((resolve) => { const child = spawn(command, [...args, ...argv], { stdio: ['ignore', 'pipe', 'pipe'], + env: env ? { ...process.env, ...env } : process.env, }) let stdout = '' let stderr = '' - child.stdout.on('data', (d) => (stdout += d)) + let limit = false + const timer = setTimeout(() => child.kill('SIGKILL'), timeout) + child.stdout.on('data', (d) => { + stdout += d + if (stdout.length > maxBuffer) { + limit = true + child.kill('SIGKILL') + } + }) child.stderr.on('data', (d) => (stderr += d)) - child.on('error', (e) => - resolve({ stdout: '', stderr: errMessage(e), code: 1 }), - ) - child.on('close', (code) => - resolve({ stdout, stderr, code: normalizeExit(code) }), - ) + child.on('error', (e) => { + clearTimeout(timer) + resolve({ stdout: '', stderr: errMessage(e), code: 1, signal: null }) + }) + child.on('close', (code, signal) => { + clearTimeout(timer) + resolve({ + stdout, + stderr, + code: normalizeExit(code), + signal: limit ? 'output limit exceeded' : signal, + }) + }) }) } diff --git a/src/lib/mcp/schema.js b/src/lib/mcp/schema.js index 034903f..4476912 100644 --- a/src/lib/mcp/schema.js +++ b/src/lib/mcp/schema.js @@ -16,6 +16,9 @@ export const NOISE_FLAGS = new Set([ 'profile', 'api-key', 'help', + // `yes` is force-injected for destructive tools (no TTY); exposing it as a + // settable input would be inert and misleading. + 'yes', ]) function flagSchema(flag) { @@ -29,7 +32,9 @@ function flagSchema(flag) { if (Array.isArray(flag.options) && flag.options.length) { s = z.enum(flag.options) } else { - s = z.string() + // Accept numbers too — an LLM will naturally send {limit: 50}, {id: 123}. + // toArgv stringifies before spawning, so either form is fine. + s = z.union([z.string(), z.number()]) } if (flag.multiple) s = z.array(s) if (flag.description) s = s.describe(flag.description) @@ -37,7 +42,7 @@ function flagSchema(flag) { } function argSchema(arg) { - let s = z.string() + let s = z.union([z.string(), z.number()]) if (arg.description) s = s.describe(arg.description) return arg.required ? s : s.optional() } diff --git a/src/lib/report-format.js b/src/lib/report-format.js new file mode 100644 index 0000000..e81f100 --- /dev/null +++ b/src/lib/report-format.js @@ -0,0 +1,15 @@ +import { CliError } from './errors.js' + +/** + * Reports return nested data with no natural columns, so `csv`/`table` would + * silently produce empty output. Reject them with a clear message instead. + * @param {string | undefined} output + */ +export function assertReportFormat(output) { + if (output === 'csv' || output === 'table') { + throw new CliError( + `Reports are nested data; '--output ${output}' isn't supported. Use --output json or yaml (optionally with --jq).`, + { exitCode: 64 }, + ) + } +} diff --git a/test/gen-dist.test.js b/test/gen-dist.test.js index af52989..1fd592d 100644 --- a/test/gen-dist.test.js +++ b/test/gen-dist.test.js @@ -18,6 +18,8 @@ describe('renderHomebrewFormula', () => { ) expect(formula).toContain('sha256 "deadbeef"') expect(formula).toContain('depends_on "node"') + expect(formula).toContain('depends_on "jq"') + expect(formula).toContain('JQ_PATH') expect(formula).toContain('hscli version') }) }) diff --git a/test/lib/client.test.js b/test/lib/client.test.js index 4c5f711..256d89c 100644 --- a/test/lib/client.test.js +++ b/test/lib/client.test.js @@ -124,6 +124,18 @@ describe('createClient', () => { expect(err.exitCode).toBe(75) } }) + + it('falls back to 10 when the 429 retry-after header is non-numeric', async () => { + nock(API_BASE) + .get('/v2/limited') + .reply(429, '', { 'x-ratelimit-retry-after': 'soon' }) + try { + await client.get('/v2/limited') + expect.unreachable('should have thrown') + } catch (err) { + expect(err.retryAfter).toBe(10) + } + }) }) describe('retry behavior', () => { @@ -237,6 +249,26 @@ describe('createClient', () => { }) describe('paginate', () => { + it('terminates when totalPages is non-numeric (no infinite loop)', async () => { + const scope = nock(API_BASE) + .get('/v2/conversations') + .query({ page: '1' }) + .reply(200, { + _embedded: { conversations: [{ id: 1 }] }, + page: { totalPages: 'lots' }, + }) + const items = [] + for await (const item of client.paginate( + '/v2/conversations', + {}, + 'conversations', + )) { + items.push(item) + } + expect(items).toEqual([{ id: 1 }]) + expect(scope.isDone()).toBe(true) + }) + it('yields items across multiple pages', async () => { const scope = nock(API_BASE) .get('/v2/conversations') diff --git a/test/lib/docs-client.test.js b/test/lib/docs-client.test.js index 5f494ea..af91ff5 100644 --- a/test/lib/docs-client.test.js +++ b/test/lib/docs-client.test.js @@ -89,6 +89,20 @@ describe('docs-client', () => { expect(out).toEqual(['a', 'b']) }) + it('terminates when pages is non-numeric (no infinite loop)', async () => { + nock(BASE) + .get('/v1/collections') + .query({ page: 1 }) + .reply(200, { + collections: { page: 1, pages: 'unknown', items: [{ id: 'a' }] }, + }) + const out = [] + for await (const c of client().paginate('collections', {}, 'collections')) { + out.push(c.id) + } + expect(out).toEqual(['a']) + }) + it('reports pagination progress via onProgress', async () => { nock(BASE) .get('/v1/collections') @@ -147,6 +161,13 @@ describe('docs-client', () => { }) }) + it('uses a 10s wait when the rate-limit header is non-numeric', async () => { + nock(BASE).get('/v1/c').reply(429, '', { 'retry-after': 'later' }) + await expect(client({ retry: false }).get('c')).rejects.toMatchObject({ + retryAfter: 10, + }) + }) + it('returns null for an empty (non-204) body', async () => { nock(BASE).get('/v1/empty').reply(200, '') expect(await client().get('empty')).toBeNull() diff --git a/test/lib/docs-input.test.js b/test/lib/docs-input.test.js index b8e1c57..2864550 100644 --- a/test/lib/docs-input.test.js +++ b/test/lib/docs-input.test.js @@ -19,6 +19,12 @@ describe('docs-input', () => { expect(readText(undefined)).toBeUndefined() }) + it('readText throws a clear CliError when the @file is missing', () => { + expect(() => readText('@/no/such/hscli-file.html')).toThrow( + /Cannot read --text file/, + ) + }) + it('csvList splits and trims', () => { expect(csvList('a, b ,c')).toEqual(['a', 'b', 'c']) }) diff --git a/test/lib/keychain-write-error.test.js b/test/lib/keychain-write-error.test.js new file mode 100644 index 0000000..e5efde6 --- /dev/null +++ b/test/lib/keychain-write-error.test.js @@ -0,0 +1,35 @@ +import { describe, it, expect } from 'vitest' + +// The native keyring loads but writes fail (e.g. PermissionDenied in a +// container with no Secret Service). setTokens/setDocsKey must surface the +// friendly "keychain unavailable" CliError, not a raw backend error. +vi.mock('@napi-rs/keyring', () => ({ + Entry: class { + getPassword() { + return null + } + setPassword() { + throw new Error('PermissionDenied') + } + deletePassword() {} + }, +})) + +const { setTokens, setDocsKey } = await import('../../src/lib/keychain.js') + +describe('keychain when the keyring errors on write', () => { + it('setTokens surfaces a friendly keychain error (exit 78)', async () => { + await expect( + setTokens('p', { + accessToken: 'x', + expiresAt: 0, + authMode: 'authorization_code', + credentialSource: 'byo', + }), + ).rejects.toMatchObject({ exitCode: 78 }) + }) + + it('setDocsKey surfaces a friendly keychain error (exit 78)', async () => { + await expect(setDocsKey('p', 'key')).rejects.toMatchObject({ exitCode: 78 }) + }) +}) diff --git a/test/lib/mcp/catalog.test.js b/test/lib/mcp/catalog.test.js index 90fe5c1..4fd1663 100644 --- a/test/lib/mcp/catalog.test.js +++ b/test/lib/mcp/catalog.test.js @@ -12,6 +12,7 @@ describe('classifyKind', () => { ['docs:article:delete-draft', 'destructive'], ['webhook:delete', 'destructive'], ['conv:bulk-status', 'destructive'], // bulk ops hit many records — flag for confirm + ['workflow:run', 'destructive'], // triggers a workflow that can mass-mutate ['conv:status', 'write'], // can mutate via --set, so gated ['conv:reply', 'write'], ['backup', 'write'], @@ -55,7 +56,15 @@ describe('buildCatalog', () => { 'conv:list', 'version', ]) - for (const id of ['api', 'conv:watch', 'doctor', 'mcp:serve']) { + for (const id of [ + 'api', + 'conv:watch', + 'doctor', + 'mcp:serve', + 'auth:login', + 'auth:setup', + 'docs:auth', + ]) { expect(EXCLUDED.has(id)).toBe(true) } }) diff --git a/test/lib/mcp/invoke.test.js b/test/lib/mcp/invoke.test.js index 6b76830..929266d 100644 --- a/test/lib/mcp/invoke.test.js +++ b/test/lib/mcp/invoke.test.js @@ -23,12 +23,31 @@ const writeEntry = { } describe('toArgv', () => { - it('splits the id into a command path and forces --output json', () => { + it('splits the id into a command path and forces --output=json', () => { const argv = toArgv(readEntry, { status: 'active' }) expect(argv.slice(0, 2)).toEqual(['conv', 'list']) - expect(argv).toContain('--status') - expect(argv).toContain('active') - expect(argv.slice(-2)).toEqual(['--output', 'json']) + expect(argv).toContain('--status=active') + expect(argv).toContain('--output=json') + }) + + it('emits flag values as --name=value so they cannot be read as flags', () => { + // A value starting with `-` must not become a CLI flag. + const argv = toArgv(readEntry, { status: '--help' }) + expect(argv).toContain('--status=--help') + expect(argv).not.toContain('--help') + }) + + it('puts positional args after a -- separator', () => { + // An arg value starting with `-` must stay a positional. + const argv = toArgv( + { id: 'conv:get', flags: {}, args: { id: {} } }, + { + id: '--help', + }, + ) + const sep = argv.indexOf('--') + expect(sep).toBeGreaterThan(-1) + expect(argv.slice(sep + 1)).toEqual(['--help']) }) it('emits a boolean flag only when true', () => { @@ -36,37 +55,37 @@ describe('toArgv', () => { expect(toArgv(readEntry, { verbose: false })).not.toContain('--verbose') }) - it('repeats a multiple flag per value', () => { + it('repeats a multiple flag per value as --name=value', () => { const argv = toArgv(readEntry, { embed: ['threads', 'tags'] }) - expect(argv.filter((a) => a === '--embed')).toHaveLength(2) - expect(argv).toContain('threads') - expect(argv).toContain('tags') + expect(argv.filter((a) => a.startsWith('--embed='))).toEqual([ + '--embed=threads', + '--embed=tags', + ]) }) - it('passes positional args and auto-appends --yes when the command supports it', () => { + it('passes positional args after -- and auto-appends --yes', () => { const argv = toArgv(writeEntry, { id: '5abc' }) expect(argv).toEqual([ 'docs', 'article', 'delete', - '5abc', - '--output', - 'json', + '--output=json', '--yes', + '--', + '5abc', ]) }) - it('omits an optional arg that was not provided', () => { + it('omits an optional arg that was not provided (no -- separator)', () => { const argv = toArgv({ id: 'conv:get', flags: {}, args: { id: {} } }, {}) - expect(argv).toEqual(['conv', 'get', '--output', 'json']) + expect(argv).toEqual(['conv', 'get', '--output=json']) }) it('tolerates entries with no flags/args maps', () => { expect(toArgv({ id: 'conv:list' }, { status: null })).toEqual([ 'conv', 'list', - '--output', - 'json', + '--output=json', ]) }) @@ -79,7 +98,7 @@ describe('toArgv', () => { }, { id: null, status: null }, ) - expect(argv).toEqual(['conv', 'get', '--output', 'json']) + expect(argv).toEqual(['conv', 'get', '--output=json']) }) }) @@ -162,6 +181,21 @@ describe('runTool', () => { expect(res.isError).toBe(true) expect(res.content[0].text).toBe('exited 5') }) + + it('reports a signal-terminated child as an error (not silent success)', async () => { + const res = await runTool( + readEntry, + {}, + fakeExec({ + stdout: 'partial output', + stderr: '', + code: 0, + signal: 'SIGKILL', + }), + ) + expect(res.isError).toBe(true) + expect(res.content[0].text).toContain('SIGKILL') + }) }) describe('makeExec', () => { @@ -193,6 +227,37 @@ describe('makeExec', () => { expect(r.code).toBe(1) expect(r.stderr).toBeTruthy() }) + + it('passes env through to the child', async () => { + const exec = makeExec({ + command: process.execPath, + args: ['-e', 'process.stdout.write(process.env.HSCLI_PROFILE||"none")'], + env: { HSCLI_PROFILE: 'work' }, + }) + expect((await exec([])).stdout).toBe('work') + }) + + it('kills a child that exceeds the timeout', async () => { + const exec = makeExec({ + command: process.execPath, + args: ['-e', 'setTimeout(()=>{}, 10000)'], + timeout: 150, + }) + expect((await exec([])).signal).toBe('SIGKILL') + }) + + it('kills a child that exceeds maxBuffer', async () => { + const exec = makeExec({ + command: process.execPath, + args: [ + '-e', + 'const b="x".repeat(100000); setInterval(()=>process.stdout.write(b), 1)', + ], + maxBuffer: 50000, + timeout: 5000, + }) + expect((await exec([])).signal).toBe('output limit exceeded') + }) }) describe('normalizeExit', () => { diff --git a/test/lib/mcp/schema.test.js b/test/lib/mcp/schema.test.js index d701ea6..6cd9523 100644 --- a/test/lib/mcp/schema.test.js +++ b/test/lib/mcp/schema.test.js @@ -13,29 +13,30 @@ const entry = { description: 'St', }, tag: { type: 'option', description: 'Tag', required: true }, - yes: { type: 'boolean', description: 'Skip prompt' }, + hidden: { type: 'boolean', description: 'Hide' }, embed: { type: 'option', multiple: true, description: 'Embed' }, // noise — must be dropped: output: { type: 'option', options: ['json', 'table'] }, fields: { type: 'option' }, 'no-color': { type: 'boolean' }, profile: { type: 'option' }, + yes: { type: 'boolean' }, }, } describe('buildInputSchema', () => { const shape = buildInputSchema(entry) - it('drops global/noise flags', () => { - for (const n of ['output', 'fields', 'no-color', 'profile']) { + it('drops global/noise flags (including yes)', () => { + for (const n of ['output', 'fields', 'no-color', 'profile', 'yes']) { expect(Object.keys(shape)).not.toContain(n) } - expect(NOISE_FLAGS.has('output')).toBe(true) + expect(NOISE_FLAGS.has('yes')).toBe(true) }) it('includes args and meaningful flags', () => { expect(Object.keys(shape).sort()).toEqual( - ['embed', 'id', 'note', 'status', 'tag', 'yes'].sort(), + ['embed', 'hidden', 'id', 'note', 'status', 'tag'].sort(), ) }) @@ -45,6 +46,11 @@ describe('buildInputSchema', () => { expect(shape.id.safeParse('5abc').success).toBe(true) }) + it('accepts a number for an id/option (LLMs send {id: 123})', () => { + expect(shape.id.safeParse(123).success).toBe(true) + expect(shape.tag.safeParse(7).success).toBe(true) + }) + it('maps an options flag to an enum', () => { expect(shape.status.safeParse('active').success).toBe(true) expect(shape.status.safeParse('nope').success).toBe(false) @@ -56,9 +62,9 @@ describe('buildInputSchema', () => { }) it('maps a boolean flag (always optional)', () => { - expect(shape.yes.safeParse(true).success).toBe(true) - expect(shape.yes.safeParse('x').success).toBe(false) - expect(shape.yes.safeParse(undefined).success).toBe(true) + expect(shape.hidden.safeParse(true).success).toBe(true) + expect(shape.hidden.safeParse('x').success).toBe(false) + expect(shape.hidden.safeParse(undefined).success).toBe(true) }) it('maps a multiple option flag to a string array', () => { diff --git a/test/lib/mcp/server.test.js b/test/lib/mcp/server.test.js index 9305972..503012f 100644 --- a/test/lib/mcp/server.test.js +++ b/test/lib/mcp/server.test.js @@ -121,7 +121,7 @@ describe('buildServer', () => { arguments: { status: 'active' }, }) expect(res.structuredContent).toEqual({ results: [{ id: 1 }] }) - expect(seenArgv).toContain('--status') + expect(seenArgv).toContain('--status=active') await client.close() }) }) diff --git a/test/lib/report-format.test.js b/test/lib/report-format.test.js new file mode 100644 index 0000000..bcf51c0 --- /dev/null +++ b/test/lib/report-format.test.js @@ -0,0 +1,15 @@ +import { describe, it, expect } from 'vitest' +import { assertReportFormat } from '../../src/lib/report-format.js' + +describe('assertReportFormat', () => { + it('allows structured formats', () => { + expect(() => assertReportFormat('json')).not.toThrow() + expect(() => assertReportFormat('yaml')).not.toThrow() + expect(() => assertReportFormat(undefined)).not.toThrow() + }) + + it('rejects csv and table with a clear message', () => { + expect(() => assertReportFormat('csv')).toThrow(/isn't supported/) + expect(() => assertReportFormat('table')).toThrow(/Use --output json/) + }) +}) diff --git a/website/src/content/docs/automation/mcp.mdx b/website/src/content/docs/automation/mcp.mdx index fa872d9..506413e 100644 --- a/website/src/content/docs/automation/mcp.mdx +++ b/website/src/content/docs/automation/mcp.mdx @@ -55,8 +55,9 @@ tool is available; when not, write tools simply aren't listed. :::note[Two layers of safety] Write tools are gated by `--allow-writes`, **and** the destructive ones — `delete`, `remove`, and `bulk` operations — carry an MCP `destructiveHint` so the client (e.g. Claude Desktop) -prompts you before running them. Reads are marked `readOnlyHint` and run without prompting; -other writes (`create`/`update`) carry no hint either way. +prompts you before running them. Reads are marked `readOnlyHint`; other writes +(`create`/`update`) are marked `readOnlyHint:false` + `destructiveHint:false`, so clients +neither auto-approve nor specially warn on them. ::: ## How it maps diff --git a/website/src/content/docs/guides/reporting.mdx b/website/src/content/docs/guides/reporting.mdx index 95b70e2..f55fa9c 100644 --- a/website/src/content/docs/guides/reporting.mdx +++ b/website/src/content/docs/guides/reporting.mdx @@ -29,12 +29,19 @@ hscli report company --start 2026-05-01 --end 2026-05-31 hscli report user --start 2026-05-01 --end 2026-05-31 --user 10 ``` -## Straight into a spreadsheet +## Pull metrics into your stack -```bash frame="terminal" title="export to CSV" -hscli report conversations --start 2026-05-01 --end 2026-05-31 --output csv > may.csv +Reports return nested JSON — stream them into a notebook, a BI tool, or a `jq` pipeline: + +```bash frame="terminal" +hscli report conversations --start 2026-05-01 --end 2026-05-31 --output json > may.json ``` +:::note +Reports are nested, so `--output csv`/`table` aren't supported here — use `json` or +`yaml`, and reshape with `--jq` when you need a flat CSV. +::: + :::tip -Schedule this in [CI](/automation/ci/) to keep a dashboard or Google Sheet fresh. +Schedule this in [CI](/automation/ci/) to keep a dashboard fresh. ::: diff --git a/website/src/content/docs/reference/commands.mdx b/website/src/content/docs/reference/commands.mdx index 7dbc147..cbbcbc3 100644 --- a/website/src/content/docs/reference/commands.mdx +++ b/website/src/content/docs/reference/commands.mdx @@ -12,7 +12,7 @@ hscli [target] [flags] ``` Run `hscli --help` for the live, self-describing version of any command. -This page lists all 89 commands in `hscli` v0.11.0. +This page lists all 89 commands in `hscli` v0.11.1. ## alias diff --git a/website/src/pages/index.astro b/website/src/pages/index.astro index 872b884..1cb7d13 100644 --- a/website/src/pages/index.astro +++ b/website/src/pages/index.astro @@ -201,13 +201,13 @@ const description =
Report -

Pull metrics into a spreadsheet

-

Stream any report as CSV and pipe it straight into your BI tool or a Google Sheet.

+

Pull metrics into your stack

+

Stream any report as JSON straight into a notebook, your BI tool, or a jq pipeline.

$ hscli report conversations \
  --start 2026-05-01 --end 2026-05-31 \
-
  --output csv > may.csv
+
  --output json > may.json