From 350e08c1ab0fec78e32e93121a71bea5fe23dd2d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 20:03:45 +0000 Subject: [PATCH 1/8] feat(cli): table-driven global flags, --pretty and --verbose; compact JSON and no meta by default Replaces the scattered --json/--no-color plumbing with a single declarative GlobalFlags table (cli/global-flags.ts) that also registers cac options and resolves flags from argv for the pre-parse-failure fallback path. Adds two new global flags, --pretty and --verbose, and collapses the ad-hoc `io` bag into one CliEnv object (`env`) threaded through every route. Centralizes the two pieces of logic that used to be duplicated across runner.ts: the result envelope (cli/envelope.ts's finalizeResult, which owns the `meta` block) and JSON serialization (global-flags.ts's formatJson, which owns compact-vs-indented output). Breaking (CLI): - `--json` output is now compact (single-line) by default; `--pretty` restores the old 2-space indentation. - The `meta` block (command/timestamp/duration_ms) is no longer emitted by default, in either human or --json output; `--verbose` restores it. NDJSON event lines (`appduct events`) stay single-line always, regardless of --pretty, per the streaming-consumer contract. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_018zpvXWBBSJZnNVrqbzpFB4 --- CHANGELOG.md | 11 + docs/ARCHITECTURE.md | 7 +- packages/appduct/README.md | 2 +- .../__snapshots__/output.test.ts.snap | 71 +--- .../appduct/src/__tests__/envelope.test.ts | 53 +++ .../global-flags.integration.test.ts | 54 +++ .../src/__tests__/global-flags.test.ts | 87 +++++ packages/appduct/src/__tests__/output.test.ts | 206 +++++----- packages/appduct/src/__tests__/router.test.ts | 5 +- packages/appduct/src/__tests__/runner.test.ts | 194 ++++++---- packages/appduct/src/cli/create-cli.ts | 7 +- packages/appduct/src/cli/dispatch.ts | 19 +- packages/appduct/src/cli/envelope.ts | 42 +++ packages/appduct/src/cli/global-flags.ts | 111 ++++++ packages/appduct/src/cli/router.ts | 12 +- packages/appduct/src/cli/routes/daemon/run.ts | 6 +- .../appduct/src/cli/routes/daemon/start.ts | 6 +- .../appduct/src/cli/routes/daemon/status.ts | 6 +- .../appduct/src/cli/routes/daemon/stop.ts | 6 +- packages/appduct/src/cli/routes/doctor.ts | 2 +- packages/appduct/src/cli/routes/events.ts | 17 +- packages/appduct/src/cli/routes/init.ts | 2 +- packages/appduct/src/cli/routes/invoke.ts | 2 +- packages/appduct/src/cli/routes/keygen.ts | 2 +- packages/appduct/src/cli/routes/link.ts | 6 +- packages/appduct/src/cli/routes/ls.ts | 2 +- packages/appduct/src/cli/routes/mcp.ts | 14 +- packages/appduct/src/cli/routes/revoke.ts | 2 +- packages/appduct/src/cli/routes/tools.ts | 3 +- packages/appduct/src/cli/runner.ts | 109 +++--- packages/appduct/src/cli/types.ts | 25 +- packages/appduct/src/output.ts | 352 +++++++++++------- 32 files changed, 932 insertions(+), 511 deletions(-) create mode 100644 packages/appduct/src/__tests__/envelope.test.ts create mode 100644 packages/appduct/src/__tests__/global-flags.integration.test.ts create mode 100644 packages/appduct/src/__tests__/global-flags.test.ts create mode 100644 packages/appduct/src/cli/envelope.ts create mode 100644 packages/appduct/src/cli/global-flags.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b3d4a294..f2036f31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,17 @@ package versions for a release. liveness probe now also reads `/proc//status` on Linux and treats `State: Z` as dead; everywhere `/proc` is absent or unreadable the previous behaviour is unchanged. +- **Breaking (CLI): `--json` output is compact by default.** Every `appduct --json` + invocation used to pretty-print its JSON with 2-space indentation; it now prints it on a single + line (`JSON.stringify`, no whitespace). Any JSON parser is unaffected. A script that greps or + diffs the indented text directly is not — pass the new `--pretty` flag to restore the old + indentation. +- **Breaking (CLI): the `meta` block (`command`, `timestamp`, `duration_ms`) is no longer emitted + by default**, in either human or `--json` output. Pass the new `--verbose` flag to restore it — + the trailing `Meta` lines in human mode, the `meta` field on the `--json` envelope. +- **New: `--pretty` and `--verbose` global flags**, alongside `--json` and `--no-color`. See the + [`appduct` README](packages/appduct/README.md) for the full description of each. + ## 0.10.0 (2026-09-16) - **New: native SDKs for apps without React Native.** The same Appduct core the React Native diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 75b6c536..d55ba449 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -492,8 +492,11 @@ owns no keys. Every command is one RPC call plus formatting, which is why the CL server can't drift in behavior: they are the same calls. The per-command reference lives in the [`appduct` package README](../packages/appduct/README.md), -which is where it stays current. Global flags: `--json` (machine output, NDJSON for streams), -`--no-color`, `--state-dir`, `--daemon-restart` (force a version-drift restart, §4). +which is where it stays current. Global flags (`cli/global-flags.ts`'s declarative table): `--json` +(machine output, NDJSON for streams; compact by default), `--pretty` (indent `--json` output and +embedded JSON values, never NDJSON lines), `--verbose` (include the `meta` block — omitted by +default in both human and `--json` output), `--no-color`, `--state-dir`, `--daemon-restart` (force +a version-drift restart, §4). The deep-link scheme used to compose a link is resolved by `scheme.ts`, shared by `appduct link`, `appduct mcp`, `appduct/client`'s `link()` and the MCP `appduct_connect` tool so diff --git a/packages/appduct/README.md b/packages/appduct/README.md index afcf1ab2..96aa65c6 100644 --- a/packages/appduct/README.md +++ b/packages/appduct/README.md @@ -47,7 +47,7 @@ That's the whole loop. There is no host process to start — `appduct` auto-spaw | `appduct mcp [--scheme ]` | start a stdio MCP server proxying connected apps' tools to MCP clients | | `appduct doctor [--assert-present\|--assert-absent]` | release-gate step: report or assert whether a built `.app`/`.ipa`/`.apk`/`.aab` contains Appduct | -Every command that targets a session accepts an optional `selector` (a session id or an alias from `appduct ls`); omit it when exactly one session is active. Global flags: `--json` (machine-readable output), `--no-color`, `--state-dir ` (default `~/.appduct`), `--daemon-restart` (on a daemon/CLI version mismatch, restart the daemon even though that drops live sessions and unclaimed links — `APPDUCT_DAEMON_RESTART=1` and `config.json`'s `restartDaemonOnVersionMismatch` do the same for every command, and `--no-daemon-restart` overrules both for one). Run `appduct --help` for the exact flags of any command. +Every command that targets a session accepts an optional `selector` (a session id or an alias from `appduct ls`); omit it when exactly one session is active. Global flags: `--json` (machine-readable output; compact by default, one line), `--pretty` (indent `--json` output, and JSON values embedded in human output, 2 spaces — never NDJSON event lines), `--verbose` (include the `meta` block — `command`, `timestamp`, `duration_ms` — omitted by default in both human and `--json` output), `--no-color`, `--state-dir ` (default `~/.appduct`), `--daemon-restart` (on a daemon/CLI version mismatch, restart the daemon even though that drops live sessions and unclaimed links — `APPDUCT_DAEMON_RESTART=1` and `config.json`'s `restartDaemonOnVersionMismatch` do the same for every command, and `--no-daemon-restart` overrules both for one). Run `appduct --help` for the exact flags of any command. `--timeout` on `invoke` is clamped to 1,000–600,000 ms and can only **shorten** the deadline, never extend it past the app's own timer: the app aborts the handler at the tool's declared `timeoutMs`, or 10 seconds for a tool that declares none, regardless of what the caller asks for. If a tool needs more room, declare `timeoutMs` on its registration. diff --git a/packages/appduct/src/__tests__/__snapshots__/output.test.ts.snap b/packages/appduct/src/__tests__/__snapshots__/output.test.ts.snap index 3bda3eef..3bcd41bb 100644 --- a/packages/appduct/src/__tests__/__snapshots__/output.test.ts.snap +++ b/packages/appduct/src/__tests__/__snapshots__/output.test.ts.snap @@ -18,11 +18,6 @@ Policy Audit Path /tmp/state/audit Failed writes 3 - -Meta - Command: daemon status - Timestamp: 2026-03-17T10:00:00.000Z - Duration: 4 ms " `; @@ -48,11 +43,6 @@ Audit Size 1.5 MiB Failed writes 0 Failed prunes 2 - -Meta - Command: daemon status - Timestamp: 2026-03-17T10:00:00.000Z - Duration: 4 ms " `; @@ -61,14 +51,7 @@ exports[`output rendering > human errors render on stderr 1`] = ` Error Type tool_execution_error Message The tool handler threw. - Details { - "hint": "test" -} - -Meta - Command: invoke - Timestamp: 2026-03-17T10:00:00.000Z - Duration: 2 ms + Details {"hint":"test"} " `; @@ -96,24 +79,12 @@ MCP server entry Next 1. Add \`import "@appduct/react-native/auto";\` to your app entry. - -Meta - Command: init - Timestamp: 2026-03-17T10:00:00.000Z - Duration: 3 ms " `; exports[`output rendering > invoke human output prints the raw tool result 1`] = ` "Result -{ - "echoed": "hello" -} - -Meta - Command: invoke - Timestamp: 2026-03-17T10:00:00.000Z - Duration: 6 ms +{"echoed":"hello"} " `; @@ -122,11 +93,6 @@ exports[`output rendering > keygen success output includes the key path and fing Key Path /tmp/appduct-key.pem Fingerprint sha256/example - -Meta - Command: keygen - Timestamp: 2026-03-17T10:00:00.000Z - Duration: 3 ms " `; @@ -138,22 +104,12 @@ Link Endpoint wss://192.168.1.10:8443 Pin sha256/example Expires 2026-03-17T10:00:30.000Z - -Meta - Command: link - Timestamp: 2026-03-17T10:00:00.000Z - Duration: 5 ms " `; exports[`output rendering > ls human output handles an empty session list 1`] = ` "Sessions No Appduct sessions are registered. - -Meta - Command: ls - Timestamp: 2026-03-17T10:00:00.000Z - Duration: 1 ms " `; @@ -161,28 +117,14 @@ exports[`output rendering > ls human output includes alias, state, device, tools "Sessions Alias State Device Tools Age pixel-8 active Google Pixel 8 (Android 14) 3 1m - -Meta - Command: ls - Timestamp: 2026-03-17T10:00:00.000Z - Duration: 2 ms " `; exports[`output rendering > tools detail output renders full schema/annotations 1`] = ` "Tool: echo Description Echo a payload on the connected device. - Input schema { - "type": "object" -} - Annotations { - "readOnlyHint": true -} - -Meta - Command: tools - Timestamp: 2026-03-17T10:00:00.000Z - Duration: 4 ms + Input schema {"type":"object"} + Annotations {"readOnlyHint":true} " `; @@ -190,10 +132,5 @@ exports[`output rendering > tools list output stays structured 1`] = ` "Tools Name Description echo Echo a payload on the connected device. - -Meta - Command: tools - Timestamp: 2026-03-17T10:00:00.000Z - Duration: 4 ms " `; diff --git a/packages/appduct/src/__tests__/envelope.test.ts b/packages/appduct/src/__tests__/envelope.test.ts new file mode 100644 index 00000000..e591264d --- /dev/null +++ b/packages/appduct/src/__tests__/envelope.test.ts @@ -0,0 +1,53 @@ +/** + * `cli/envelope.ts`: the result envelope. `finalizeResult` only attaches `meta` under + * `--verbose`, and never leaves a `meta: undefined` key behind otherwise. + */ + +import { describe, expect, test } from "vitest"; + +import { createCommandMeta, finalizeResult } from "../cli/envelope.js"; +import { FIXED_NOW } from "./fixtures.js"; + +describe("createCommandMeta", () => { + test("computes duration_ms from the two dates and stamps the finish time", () => { + const startedAt = FIXED_NOW; + const finishedAt = new Date(FIXED_NOW.getTime() + 42); + + expect(createCommandMeta("ls", startedAt, finishedAt)).toEqual({ + command: "ls", + timestamp: finishedAt.toISOString(), + duration_ms: 42, + }); + }); +}); + +describe("finalizeResult", () => { + const timing = { command: "ls", startedAt: FIXED_NOW, finishedAt: new Date(FIXED_NOW.getTime() + 5) }; + + test("attaches meta under --verbose", () => { + const finalized = finalizeResult({ ok: true, data: [] }, timing, { verbose: true }); + + expect(finalized).toEqual({ + ok: true, + data: [], + meta: { command: "ls", timestamp: timing.finishedAt.toISOString(), duration_ms: 5 }, + }); + }); + + test("leaves no meta key at all when not --verbose (not meta: undefined)", () => { + const finalized = finalizeResult({ ok: true, data: [] }, timing, { verbose: false }); + + expect(finalized).toEqual({ ok: true, data: [] }); + expect("meta" in finalized).toBe(false); + }); + + test("also attaches meta to a failure result under --verbose, and omits it otherwise", () => { + const error = { ok: false as const, error: { type: "usage_error", message: "bad" } }; + + const verbose = finalizeResult(error, timing, { verbose: true }); + expect(verbose.meta).toEqual({ command: "ls", timestamp: timing.finishedAt.toISOString(), duration_ms: 5 }); + + const quiet = finalizeResult(error, timing, { verbose: false }); + expect("meta" in quiet).toBe(false); + }); +}); diff --git a/packages/appduct/src/__tests__/global-flags.integration.test.ts b/packages/appduct/src/__tests__/global-flags.integration.test.ts new file mode 100644 index 00000000..ccac64ad --- /dev/null +++ b/packages/appduct/src/__tests__/global-flags.integration.test.ts @@ -0,0 +1,54 @@ +/** + * End-to-end coverage for the global output flags (`--json`, `--pretty`, `--verbose`, + * `--no-color`) that needs no daemon: an unknown command is rejected before any RPC, through the + * same `dispatch.ts` parse-failure/unmatched-command paths every other command goes through. + */ + +import { describe, expect, test } from "vitest"; + +import { runCliWithCapture } from "./fixtures.js"; + +describe("global flags (no daemon required)", () => { + test("--json error output is a single line, with no meta by default", async () => { + const result = await runCliWithCapture(["bogus-command", "--json"]); + + expect(result.exitCode).not.toBe(0); + expect(result.stdout.replace(/\n$/u, "").split("\n")).toHaveLength(1); + + const parsed = JSON.parse(result.stdout); + expect(parsed.ok).toBe(false); + expect(parsed.error.type).toBe("usage_error"); + expect(parsed).not.toHaveProperty("meta"); + }); + + test("--json --pretty error output is indented", async () => { + const result = await runCliWithCapture(["bogus-command", "--json", "--pretty"]); + + expect(result.exitCode).not.toBe(0); + expect(result.stdout.split("\n").length).toBeGreaterThan(1); + expect(JSON.parse(result.stdout).ok).toBe(false); + }); + + test("--json --verbose error output carries meta.command", async () => { + const result = await runCliWithCapture(["bogus-command", "--json", "--verbose"]); + + expect(result.exitCode).not.toBe(0); + const parsed = JSON.parse(result.stdout); + expect(parsed.meta.command).toBe("cli"); + }); + + test("human-mode error output never mentions Meta without --verbose", async () => { + const result = await runCliWithCapture(["bogus-command"]); + + expect(result.exitCode).not.toBe(0); + expect(result.stderr).not.toContain("Meta"); + }); + + test("human-mode --verbose error output includes the Meta block", async () => { + const result = await runCliWithCapture(["bogus-command", "--verbose"]); + + expect(result.exitCode).not.toBe(0); + expect(result.stderr).toContain("Meta"); + expect(result.stderr).toContain("Command: cli"); + }); +}); diff --git a/packages/appduct/src/__tests__/global-flags.test.ts b/packages/appduct/src/__tests__/global-flags.test.ts new file mode 100644 index 00000000..23329512 --- /dev/null +++ b/packages/appduct/src/__tests__/global-flags.test.ts @@ -0,0 +1,87 @@ +/** + * `cli/global-flags.ts`: the declarative global-flags table. Checks that `resolveGlobalFlags` + * (the `cac`-options path) and `resolveGlobalFlagsFromArgv` (the parse-failure fallback path) + * agree for every flag, and that `formatJson` picks compact vs. indented correctly. + */ + +import { describe, expect, test } from "vitest"; + +import { + formatJson, + resolveGlobalFlags, + resolveGlobalFlagsFromArgv, + type GlobalFlags, +} from "../cli/global-flags.js"; + +type Case = { + name: string; + argv: string[]; + options: Record; + expected: GlobalFlags; +}; + +const cases: Case[] = [ + { + name: "defaults when nothing is passed", + argv: [], + options: {}, + expected: { json: false, pretty: false, verbose: false, color: true }, + }, + { + name: "--json", + argv: ["ls", "--json"], + options: { json: true }, + expected: { json: true, pretty: false, verbose: false, color: true }, + }, + { + name: "--pretty", + argv: ["ls", "--pretty"], + options: { pretty: true }, + expected: { json: false, pretty: true, verbose: false, color: true }, + }, + { + name: "--verbose", + argv: ["ls", "--verbose"], + options: { verbose: true }, + expected: { json: false, pretty: false, verbose: true, color: true }, + }, + { + name: "--no-color", + argv: ["ls", "--no-color"], + options: { color: false }, + expected: { json: false, pretty: false, verbose: false, color: false }, + }, + { + name: "every flag combined", + argv: ["ls", "--json", "--pretty", "--verbose", "--no-color"], + options: { json: true, pretty: true, verbose: true, color: false }, + expected: { json: true, pretty: true, verbose: true, color: false }, + }, +]; + +describe("resolveGlobalFlags / resolveGlobalFlagsFromArgv", () => { + for (const { name, argv, options, expected } of cases) { + test(`${name}: both resolution paths agree`, () => { + expect(resolveGlobalFlags(options)).toEqual(expected); + expect(resolveGlobalFlagsFromArgv(argv)).toEqual(expected); + }); + } + + test("an option cac never set (undefined) reads as its default, not as truthy", () => { + expect(resolveGlobalFlags({})).toEqual({ json: false, pretty: false, verbose: false, color: true }); + }); +}); + +describe("formatJson", () => { + const value = { a: 1, b: { c: [1, 2] } }; + + test("compact by default", () => { + expect(formatJson(value, { pretty: false })).toBe(JSON.stringify(value)); + expect(formatJson(value, { pretty: false })).not.toContain("\n"); + }); + + test("indented under --pretty", () => { + expect(formatJson(value, { pretty: true })).toBe(JSON.stringify(value, null, 2)); + expect(formatJson(value, { pretty: true })).toContain("\n"); + }); +}); diff --git a/packages/appduct/src/__tests__/output.test.ts b/packages/appduct/src/__tests__/output.test.ts index a4ab96fe..bab3c4c4 100644 --- a/packages/appduct/src/__tests__/output.test.ts +++ b/packages/appduct/src/__tests__/output.test.ts @@ -1,9 +1,20 @@ import { describe, expect, test } from "vitest"; +import type { GlobalFlags } from "../cli/global-flags.js"; import type { DaemonStatusCommandData } from "../cli/result-types.js"; import { renderEventLine, renderEventsCursorLine, renderResult } from "../output.js"; import { FIXED_NOW } from "./fixtures.js"; +/** `GlobalFlags` with sensible test defaults (no color, no --json/--pretty/--verbose), overridable + * per call so each test states only the flags it cares about. */ +const flags = (overrides: Partial = {}): GlobalFlags => ({ + json: false, + pretty: false, + verbose: false, + color: false, + ...overrides, +}); + describe("output rendering", () => { test("tools list output stays structured", () => { const rendered = renderResult( @@ -17,16 +28,10 @@ describe("output rendering", () => { output_schema: { echoed: "unknown" }, }, ], - meta: { - command: "tools", - timestamp: FIXED_NOW.toISOString(), - duration_ms: 4, - }, }, { command: "tools", - json: false, - color: false, + flags: flags(), }, ); @@ -43,9 +48,8 @@ describe("output rendering", () => { description: "Signs a test user in.", ...(timeoutMs !== undefined ? { timeout_ms: timeoutMs } : {}), }, - meta: { command: "tools", timestamp: FIXED_NOW.toISOString(), duration_ms: 4 }, }, - { command: "tools", json: false, color: false }, + { command: "tools", flags: flags() }, ).stdout; expect(renderDetail(60_000)).toContain("Timeout (ms) 60000"); @@ -63,16 +67,10 @@ describe("output rendering", () => { input_schema: { type: "object" }, annotations: { readOnlyHint: true }, }, - meta: { - command: "tools", - timestamp: FIXED_NOW.toISOString(), - duration_ms: 4, - }, }, { command: "tools", - json: false, - color: false, + flags: flags(), }, ); @@ -90,23 +88,17 @@ describe("output rendering", () => { expiresAt: Math.floor(FIXED_NOW.getTime() / 1000) + 30, pin: "sha256/example", }, - meta: { - command: "link", - timestamp: FIXED_NOW.toISOString(), - duration_ms: 5, - }, }, { command: "link", - json: false, - color: false, + flags: flags(), }, ); expect(rendered.stdout).toMatchSnapshot(); }); - test("link json output only includes the trimmed link payload (no QR)", () => { + test("link json output only includes the trimmed link payload (no QR, no meta)", () => { const rendered = renderResult( { ok: true, @@ -117,16 +109,10 @@ describe("output rendering", () => { expiresAt: Math.floor(FIXED_NOW.getTime() / 1000) + 30, pin: "sha256/example", }, - meta: { - command: "link", - timestamp: FIXED_NOW.toISOString(), - duration_ms: 5, - }, }, { command: "link", - json: true, - color: false, + flags: flags({ json: true }), qr: true, }, ); @@ -140,11 +126,6 @@ describe("output rendering", () => { expiresAt: Math.floor(FIXED_NOW.getTime() / 1000) + 30, pin: "sha256/example", }, - meta: { - command: "link", - timestamp: FIXED_NOW.toISOString(), - duration_ms: 5, - }, }); }); @@ -163,16 +144,11 @@ describe("output rendering", () => { toolCount: 3, }, ], - meta: { - command: "ls", - timestamp: FIXED_NOW.toISOString(), - duration_ms: 2, - }, }, { command: "ls", - json: false, - color: false, + flags: flags(), + now: FIXED_NOW, }, ); @@ -184,16 +160,11 @@ describe("output rendering", () => { { ok: true, data: [], - meta: { - command: "ls", - timestamp: FIXED_NOW.toISOString(), - duration_ms: 1, - }, }, { command: "ls", - json: false, - color: false, + flags: flags(), + now: FIXED_NOW, }, ); @@ -208,16 +179,10 @@ describe("output rendering", () => { path: "/tmp/appduct-key.pem", pin: "sha256/example", }, - meta: { - command: "keygen", - timestamp: FIXED_NOW.toISOString(), - duration_ms: 3, - }, }, { command: "keygen", - json: false, - color: false, + flags: flags(), }, ); @@ -229,22 +194,31 @@ describe("output rendering", () => { { ok: true, data: { echoed: "hello" }, - meta: { - command: "invoke", - timestamp: FIXED_NOW.toISOString(), - duration_ms: 6, - }, }, { command: "invoke", - json: false, - color: false, + flags: flags(), }, ); expect(rendered.stdout).toMatchSnapshot(); }); + test("human invoke output embeds compact JSON by default, and indented JSON under --pretty", () => { + const result = { + ok: true as const, + data: { echoed: { nested: { value: true } } }, + }; + + const compact = renderResult(result, { command: "invoke", flags: flags() }).stdout ?? ""; + expect(compact).toContain('{"nested":{"value":true}}'); + expect(compact).toBe("Result\n{\"echoed\":{\"nested\":{\"value\":true}}}\n"); + + const pretty = renderResult(result, { command: "invoke", flags: flags({ pretty: true }) }).stdout ?? ""; + expect(pretty).toContain('"nested": {'); + expect(pretty).toContain('"value": true'); + }); + test("human errors render on stderr", () => { const rendered = renderResult( { @@ -256,23 +230,17 @@ describe("output rendering", () => { hint: "test", }, }, - meta: { - command: "invoke", - timestamp: FIXED_NOW.toISOString(), - duration_ms: 2, - }, }, { command: "invoke", - json: false, - color: false, + flags: flags(), }, ); expect(rendered.stderr).toMatchSnapshot(); }); - test("json errors preserve the wire error type verbatim", () => { + test("json errors preserve the wire error type verbatim, with no meta by default", () => { const rendered = renderResult( { ok: false, @@ -280,20 +248,41 @@ describe("output rendering", () => { type: "tool_execution_error", message: "The tool handler threw.", }, - meta: { - command: "invoke", - timestamp: FIXED_NOW.toISOString(), - duration_ms: 2, - }, }, { command: "invoke", - json: true, - color: false, + flags: flags({ json: true }), }, ); - expect(JSON.parse(rendered.stdout ?? "").error.type).toBe("tool_execution_error"); + const parsed = JSON.parse(rendered.stdout ?? ""); + expect(parsed.error.type).toBe("tool_execution_error"); + expect(parsed).not.toHaveProperty("meta"); + }); + + test("the Meta block renders (human and --json) exactly when the result carries meta", () => { + const meta = { command: "invoke", timestamp: FIXED_NOW.toISOString(), duration_ms: 4 }; + + const human = renderResult( + { ok: true, data: { echoed: "hi" }, meta }, + { command: "invoke", flags: flags() }, + ).stdout; + expect(human).toContain("Meta"); + expect(human).toContain(`Command: ${meta.command}`); + expect(human).toContain(`Timestamp: ${meta.timestamp}`); + expect(human).toContain(`Duration: ${meta.duration_ms} ms`); + + const humanNoMeta = renderResult( + { ok: true, data: { echoed: "hi" } }, + { command: "invoke", flags: flags() }, + ).stdout; + expect(humanNoMeta).not.toContain("Meta"); + + const json = renderResult( + { ok: true, data: { echoed: "hi" }, meta }, + { command: "invoke", flags: flags({ json: true }) }, + ).stdout ?? ""; + expect(JSON.parse(json).meta).toEqual(meta); }); /** `appduct init`'s whole point is what it prints, so the human rendering is pinned. */ @@ -309,21 +298,16 @@ describe("output rendering", () => { mcpServerEntry: { command: "appduct", args: ["mcp", "--scheme", "myapp"] }, nextSteps: ['Add `import "@appduct/react-native/auto";` to your app entry.'], }, - meta: { - command: "init", - timestamp: FIXED_NOW.toISOString(), - duration_ms: 3, - }, }); test("init output shows the config, the pasteable MCP entry and the next steps", () => { - const rendered = renderResult(initResult(true), { command: "init", json: false, color: false }); + const rendered = renderResult(initResult(true), { command: "init", flags: flags() }); expect(rendered.stdout).toMatchSnapshot(); }); test("init output distinguishes an idempotent re-run from a write", () => { - const rendered = renderResult(initResult(false), { command: "init", json: false, color: false }); + const rendered = renderResult(initResult(false), { command: "init", flags: flags() }); expect(rendered.stdout).toContain("Project Already Initialized"); expect(rendered.stdout).toContain("unchanged"); @@ -333,34 +317,61 @@ describe("output rendering", () => { const base = initResult(false); const rendered = renderResult( { ...base, data: { ...base.data, note: 'app.json declares "renamed"' } }, - { command: "init", json: false, color: false }, + { command: "init", flags: flags() }, ); expect(rendered.stdout).toContain('Note: app.json declares "renamed"'); }); test("init --json exposes the MCP entry structurally rather than as a pre-rendered string", () => { - const rendered = renderResult(initResult(true), { command: "init", json: true, color: false }); + const rendered = renderResult(initResult(true), { command: "init", flags: flags({ json: true }) }); expect(JSON.parse(rendered.stdout ?? "").data.mcpServerEntry).toEqual({ command: "appduct", args: ["mcp", "--scheme", "myapp"], }); }); + + test("--json output is a single line by default, and indented under --pretty", () => { + const result = { ok: true as const, data: { a: 1, b: { c: 2 } } }; + + const compact = renderResult(result, { command: "invoke", flags: flags({ json: true }) }).stdout ?? ""; + expect(compact.replace(/\n$/u, "").split("\n")).toHaveLength(1); + expect(JSON.parse(compact)).toEqual(result); + + const pretty = renderResult(result, { command: "invoke", flags: flags({ json: true, pretty: true }) }).stdout ?? ""; + expect(pretty.split("\n").length).toBeGreaterThan(1); + expect(JSON.parse(pretty)).toEqual(result); + }); }); describe("renderEventLine", () => { test("NDJSON mode emits parseable, verbatim JSON", () => { const event = { kind: "session_claimed" as const, sessionId: "s1", alias: "pixel-8", ts: 1_700_000_000_000, data: {}, seq: 1 }; - const line = renderEventLine(event, { json: true, color: false }); + const line = renderEventLine(event, flags({ json: true })); expect(JSON.parse(line)).toEqual(event); }); + test("NDJSON stays a single line even under --pretty", () => { + const event = { + kind: "tools_changed" as const, + sessionId: "s1", + alias: "pixel-8", + ts: 1_700_000_000_000, + data: { toolCount: 2, nested: { a: 1 } }, + seq: 1, + }; + const line = renderEventLine(event, flags({ json: true, pretty: true })); + + expect(line.split("\n")).toHaveLength(1); + expect(JSON.parse(line)).toEqual(event); + }); + test("human mode includes the kind and alias", () => { const line = renderEventLine( { kind: "tools_changed", sessionId: "s1", alias: "pixel-8", ts: 1_700_000_000_000, data: { toolCount: 2 }, seq: 1 }, - { json: false, color: false }, + flags(), ); expect(line).toContain("tools_changed"); @@ -370,12 +381,18 @@ describe("renderEventLine", () => { describe("renderEventsCursorLine", () => { test("NDJSON mode emits a parseable { cursor } object", () => { - const line = renderEventsCursorLine(42, { json: true, color: false }); + const line = renderEventsCursorLine(42, flags({ json: true })); + expect(JSON.parse(line)).toEqual({ cursor: 42 }); + }); + + test("NDJSON stays a single line even under --pretty", () => { + const line = renderEventsCursorLine(42, flags({ json: true, pretty: true })); + expect(line.split("\n")).toHaveLength(1); expect(JSON.parse(line)).toEqual({ cursor: 42 }); }); test("human mode includes the cursor value and the resume flag", () => { - const line = renderEventsCursorLine(42, { json: false, color: false }); + const line = renderEventsCursorLine(42, flags()); expect(line).toContain("42"); expect(line).toContain("--since 42"); }); @@ -398,9 +415,8 @@ describe("daemon status rendering", () => { policy: { default: "allow", destructive: "deny" }, audit, } satisfies DaemonStatusCommandData, - meta: { command: "daemon status", timestamp: FIXED_NOW.toISOString(), duration_ms: 4 }, }, - { command: "daemon status", json: false, color: false }, + { command: "daemon status", flags: flags() }, ).stdout ?? ""; }; diff --git a/packages/appduct/src/__tests__/router.test.ts b/packages/appduct/src/__tests__/router.test.ts index 0cdb3959..05a95938 100644 --- a/packages/appduct/src/__tests__/router.test.ts +++ b/packages/appduct/src/__tests__/router.test.ts @@ -18,9 +18,8 @@ const makeContext = (args: string[]): { context: RouteContext; stdout: () => str path: [], args, options: {}, - io: { - json: true, - color: false, + env: { + flags: { json: true, pretty: false, verbose: true, color: false }, clock: fixedClock, stdout: { isTTY: false, diff --git a/packages/appduct/src/__tests__/runner.test.ts b/packages/appduct/src/__tests__/runner.test.ts index 45ee9cf1..e91b91af 100644 --- a/packages/appduct/src/__tests__/runner.test.ts +++ b/packages/appduct/src/__tests__/runner.test.ts @@ -1,12 +1,38 @@ import { describe, expect, test } from "vitest"; +import type { GlobalFlags } from "../cli/global-flags.js"; import { executeHostedCommand } from "../cli/runner.js"; +import type { CliEnv } from "../cli/types.js"; import { sessionError } from "../errors.js"; import { FIXED_NOW, fixedClock } from "./fixtures.js"; +const flags = (overrides: Partial = {}): GlobalFlags => ({ + json: false, + pretty: false, + verbose: false, + color: false, + ...overrides, +}); + describe("executeHostedCommand", () => { test("reporter is disposed after hosted command completion", async () => { let disposed = 0; + const env: CliEnv = { + flags: flags(), + clock: fixedClock, + stdout: { + isTTY: true, + write() { + return true; + }, + }, + stderr: { + write() { + return true; + }, + }, + }; + const exitCode = await executeHostedCommand( "daemon run", async () => ({ @@ -23,27 +49,12 @@ describe("executeHostedCommand", () => { completion: Promise.resolve(), stop: () => {}, }), + env, { - json: false, - color: false, - clock: fixedClock, - stdout: { - isTTY: true, - write() { - return true; - }, - }, - stderr: { - write() { - return true; - }, - }, - reporter: { - kind: "plain", - onEvent() {}, - dispose() { - disposed += 1; - }, + kind: "plain", + onEvent() {}, + dispose() { + disposed += 1; }, }, ); @@ -54,6 +65,22 @@ describe("executeHostedCommand", () => { test("a live (non-plain) reporter suppresses the default bootstrap render", async () => { let stdout = ""; + const env: CliEnv = { + flags: flags(), + clock: fixedClock, + stdout: { + isTTY: false, + write(chunk) { + stdout += typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8"); + return true; + }, + }, + stderr: { + write() { + return true; + }, + }, + }; const exitCode = await executeHostedCommand( "events", @@ -62,27 +89,11 @@ describe("executeHostedCommand", () => { completion: Promise.resolve(), stop: () => {}, }), + env, { - json: false, - color: false, - clock: fixedClock, - stdout: { - isTTY: false, - write(chunk) { - stdout += typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8"); - return true; - }, - }, - stderr: { - write() { - return true; - }, - }, - reporter: { - kind: "interactive", - onEvent() {}, - dispose() {}, - }, + kind: "interactive", + onEvent() {}, + dispose() {}, }, ); @@ -90,9 +101,28 @@ describe("executeHostedCommand", () => { expect(stdout).toBe(""); }); - test("json output stays single-shot when the hosted runtime later fails", async () => { + test("json output stays single-shot when the hosted runtime later fails, with no meta by default", async () => { let stdout = ""; let stderr = ""; + const env: CliEnv = { + flags: flags({ json: true }), + clock: { + now: () => new Date(FIXED_NOW.getTime() + 1_000), + }, + stdout: { + isTTY: false, + write(chunk) { + stdout += typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8"); + return true; + }, + }, + stderr: { + write(chunk) { + stderr += typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8"); + return true; + }, + }, + }; const exitCode = await executeHostedCommand( "daemon run", @@ -112,29 +142,12 @@ describe("executeHostedCommand", () => { ), stop: () => {}, }), - { - json: true, - color: false, - clock: { - now: () => new Date(FIXED_NOW.getTime() + 1_000), - }, - stdout: { - isTTY: false, - write(chunk) { - stdout += typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8"); - return true; - }, - }, - stderr: { - write(chunk) { - stderr += typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8"); - return true; - }, - }, - }, + env, ); expect(exitCode).toBe(71); + // Single line on stdout (compact JSON, no --pretty) and no `meta` key (no --verbose). + expect(stdout.replace(/\n$/u, "").split("\n")).toHaveLength(1); expect(JSON.parse(stdout)).toEqual({ ok: true, data: { @@ -144,20 +157,65 @@ describe("executeHostedCommand", () => { socket_path: "/tmp/appduct-state/daemon.sock", }, }, - meta: { - command: "daemon run", - timestamp: new Date(FIXED_NOW.getTime() + 1_000).toISOString(), - duration_ms: 0, - }, }); // The v1 defect this guards against: a bare, unparseable text line on stderr in --json mode. // The fix (runner.ts) emits a full JSON object instead, once stdout's single-object contract is - // already fulfilled by the bootstrap render above. - expect(() => JSON.parse(stderr)).not.toThrow(); + // already fulfilled by the bootstrap render above. Also a single line: one JSON document. + expect(stderr.replace(/\n$/u, "").split("\n")).toHaveLength(1); const parsedStderr = JSON.parse(stderr); expect(parsedStderr.ok).toBe(false); expect(parsedStderr.error.type).toBe("session_error"); expect(parsedStderr.error.message).toBe("Pending session TTL expired before any app connected."); + expect(parsedStderr).not.toHaveProperty("meta"); + }); + + test("--verbose puts meta on both the bootstrap render and a late json failure", async () => { + let stdout = ""; + let stderr = ""; + const env: CliEnv = { + flags: flags({ json: true, verbose: true }), + clock: { + now: () => new Date(FIXED_NOW.getTime() + 1_000), + }, + stdout: { + isTTY: false, + write(chunk) { + stdout += typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8"); + return true; + }, + }, + stderr: { + write(chunk) { + stderr += typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8"); + return true; + }, + }, + }; + + const exitCode = await executeHostedCommand( + "daemon run", + async () => ({ + result: { + ok: true, + data: { + daemon: { + pid: 4242, + state_dir: "/tmp/appduct-state", + socket_path: "/tmp/appduct-state/daemon.sock", + }, + }, + }, + completion: Promise.reject( + sessionError("Pending session TTL expired before any app connected."), + ), + stop: () => {}, + }), + env, + ); + + expect(exitCode).toBe(71); + expect(JSON.parse(stdout).meta.command).toBe("daemon run"); + expect(JSON.parse(stderr).meta.command).toBe("daemon run"); }); }); diff --git a/packages/appduct/src/cli/create-cli.ts b/packages/appduct/src/cli/create-cli.ts index 653ea33a..bb25523a 100644 --- a/packages/appduct/src/cli/create-cli.ts +++ b/packages/appduct/src/cli/create-cli.ts @@ -1,13 +1,16 @@ import { cac } from "cac"; import { getPackageVersion } from "../package-version.js"; +import { registerGlobalFlags } from "./global-flags.js"; export const createCli = () => { const version = getPackageVersion(); const cli = cac("appduct"); - cli.option("--json", "Print machine-readable JSON (NDJSON for streaming commands)."); - cli.option("--no-color", "Disable terminal color in human-readable output."); + // `--json`/`--pretty`/`--verbose`/`--no-color`: the declarative table in `global-flags.ts`. + // `--state-dir`/`--daemon-restart` stay registered by hand below — they are not output flags + // and are resolved differently (state-dir resolution, version-drift guard). + registerGlobalFlags(cli); cli.option("--state-dir ", "Override the Appduct state directory (default: ~/.appduct)."); cli.option( "--daemon-restart", diff --git a/packages/appduct/src/cli/dispatch.ts b/packages/appduct/src/cli/dispatch.ts index 886a9304..f052f658 100644 --- a/packages/appduct/src/cli/dispatch.ts +++ b/packages/appduct/src/cli/dispatch.ts @@ -14,10 +14,11 @@ import { getStateDirPaths, resolveStateDir } from "../daemon/state-dir.js"; import { usageError } from "../errors.js"; import { getPackageVersion } from "../package-version.js"; import { createCli } from "./create-cli.js"; +import { resolveGlobalFlags, resolveGlobalFlagsFromArgv } from "./global-flags.js"; import { createRouter, unknownCommandError, type RouteContext } from "./router.js"; import { executeCommand } from "./runner.js"; import { systemClock } from "./types.js"; -import type { RunCliOptions } from "./types.js"; +import type { CliEnv, RunCliOptions } from "./types.js"; /** `APPDUCT_DAEMON_RESTART=1` forces a version-mismatch restart for one run — the env-var form * of `--daemon-restart`, so an MCP launch config (which passes no CLI flags) can opt in. */ @@ -69,8 +70,7 @@ export const runCli = async (argv: string[], options: RunCliOptions = {}): Promi throw error; }, { - json: argv.includes("--json"), - color: !argv.includes("--no-color"), + flags: resolveGlobalFlagsFromArgv(argv), stdout: writers.stdout, stderr: writers.stderr, clock, @@ -86,6 +86,8 @@ export const runCli = async (argv: string[], options: RunCliOptions = {}): Promi return 0; } + const flags = resolveGlobalFlags(parsedOptions); + if (!matchedCommand) { if (parsedArgs[0]) { return executeCommand( @@ -94,8 +96,7 @@ export const runCli = async (argv: string[], options: RunCliOptions = {}): Promi throw usageError(`Unknown command "${parsedArgs[0]}".`); }, { - json: Boolean(parsedOptions.json), - color: parsedOptions.color !== false, + flags, stdout: writers.stdout, stderr: writers.stderr, clock, @@ -107,9 +108,7 @@ export const runCli = async (argv: string[], options: RunCliOptions = {}): Promi return 0; } - const json = Boolean(parsedOptions.json); - const color = parsedOptions.color !== false; - const io = { json, color, stdout: writers.stdout, stderr: writers.stderr, clock }; + const env: CliEnv = { flags, stdout: writers.stdout, stderr: writers.stderr, clock }; const stateDir = resolveStateDir( typeof parsedOptions.stateDir === "string" ? parsedOptions.stateDir : undefined, ); @@ -150,7 +149,7 @@ export const runCli = async (argv: string[], options: RunCliOptions = {}): Promi * still behaves identically, it just says nothing. (A structured warning channel would be the * better answer; the runner has none today.) */ - const cliWarning = json ? () => {} : (message: string) => void writers.stderr.write(message); + const cliWarning = flags.json ? () => {} : (message: string) => void writers.stderr.write(message); const context: RouteContext = { path: [], @@ -158,7 +157,7 @@ export const runCli = async (argv: string[], options: RunCliOptions = {}): Promi // consumes it the same way the nested routers consume theirs. args: [matchedCommand, ...parsedArgs], options: parsedOptions, - io, + env, stateDir, versionCheck: { clientVersion: getPackageVersion(), diff --git a/packages/appduct/src/cli/envelope.ts b/packages/appduct/src/cli/envelope.ts new file mode 100644 index 00000000..f9dca382 --- /dev/null +++ b/packages/appduct/src/cli/envelope.ts @@ -0,0 +1,42 @@ +/** + * Owns the CLI result envelope — the {@link CommandMeta} block a rendered result carries, and + * when it's attached. This module sits on the eager path (`runner.ts` calls it for every + * command), so it imports nothing beyond `result-types`'s types and `global-flags`'s types. + */ + +import type { CliResult, CommandMeta } from "./result-types.js"; +import type { GlobalFlags } from "./global-flags.js"; + +export const createCommandMeta = (command: string, startedAt: Date, finishedAt: Date): CommandMeta => { + return { + command, + timestamp: finishedAt.toISOString(), + duration_ms: finishedAt.getTime() - startedAt.getTime(), + }; +}; + +export type CommandTiming = { + command: string; + startedAt: Date; + finishedAt: Date; +}; + +/** + * Attaches `meta` to a command's result, but only under `--verbose` — otherwise the result is + * returned exactly as the handler produced it, with no `meta` key at all (never `meta: undefined`, + * so `--json` output carries no `meta` property to filter out). + */ +export const finalizeResult = ( + result: CliResult, + timing: CommandTiming, + flags: Pick, +): CliResult => { + if (!flags.verbose) { + return result; + } + + return { + ...result, + meta: createCommandMeta(timing.command, timing.startedAt, timing.finishedAt), + }; +}; diff --git a/packages/appduct/src/cli/global-flags.ts b/packages/appduct/src/cli/global-flags.ts new file mode 100644 index 00000000..1b53dd78 --- /dev/null +++ b/packages/appduct/src/cli/global-flags.ts @@ -0,0 +1,111 @@ +/** + * The CLI's global output flags (`--json`, `--pretty`, `--verbose`, `--no-color`) as a single + * declarative table. This module sits on the eager path (ARCHITECTURE.md §10 "Startup cost") — + * every invocation resolves these flags before a command word is even known — so it imports + * nothing beyond `cac`'s types (erased at build time) and stays free of runtime dependencies. + * + * Add a flag by adding one entry to {@link globalFlagDefinitions}: `create-cli.ts` registers it + * with `cac` via {@link registerGlobalFlags}, and `dispatch.ts` resolves it into the + * {@link GlobalFlags} object every route receives as `env.flags` via {@link resolveGlobalFlags} / + * {@link resolveGlobalFlagsFromArgv} — no other file needs to change. + */ + +import type { CAC } from "cac"; + +export type GlobalFlags = { + json: boolean; + pretty: boolean; + verbose: boolean; + color: boolean; +}; + +type GlobalFlagDefinition = { + /** The flag's key in {@link GlobalFlags}. */ + key: K; + /** What `cli.option()` receives, e.g. `"--json"` or `"--no-color"`. */ + spec: string; + /** Shown by `--help`. */ + description: string; + /** Derives the flag's value from `cac`'s parsed options object (camelCased keys). */ + fromOptions: (options: Readonly>) => GlobalFlags[K]; + /** Derives the flag's value directly from argv — needed for the path where `cac` itself throws + * before a parsed options object exists (`dispatch.ts`'s parse-failure fallback). */ + fromArgv: (argv: readonly string[]) => GlobalFlags[K]; +}; + +/** A `GlobalFlagDefinition` for some `K`, with `K` erased — lets every definition live in one + * array while each individual entry keeps its own key/value type tied together. */ +type AnyGlobalFlagDefinition = { [K in keyof GlobalFlags]: GlobalFlagDefinition }[keyof GlobalFlags]; + +const globalFlagDefinitions: readonly AnyGlobalFlagDefinition[] = [ + { + key: "json", + spec: "--json", + description: "Print machine-readable JSON (NDJSON for streaming commands).", + fromOptions: (options) => Boolean(options.json), + fromArgv: (argv) => argv.includes("--json"), + }, + { + key: "pretty", + spec: "--pretty", + description: + "Indent JSON output. Applies to --json results and to JSON values embedded in human " + + "output; never to NDJSON event lines.", + fromOptions: (options) => Boolean(options.pretty), + fromArgv: (argv) => argv.includes("--pretty"), + }, + { + key: "verbose", + spec: "--verbose", + description: "Include command metadata (command, timestamp, duration) in the output.", + fromOptions: (options) => Boolean(options.verbose), + fromArgv: (argv) => argv.includes("--verbose"), + }, + { + key: "color", + spec: "--no-color", + description: "Disable terminal color in human-readable output.", + // `cac` reports a `--no-` option as `: false` when passed, and omits it (leaving + // the parsed value `undefined`) otherwise — so "not explicitly disabled" is the default `true`. + fromOptions: (options) => options.color !== false, + fromArgv: (argv) => !argv.includes("--no-color"), + }, +]; + +/** Registers every table entry with `cli.option()`, in table order. */ +export const registerGlobalFlags = (cli: CAC): void => { + for (const definition of globalFlagDefinitions) { + cli.option(definition.spec, definition.description); + } +}; + +/** Resolves {@link GlobalFlags} from `cac`'s parsed options object (the normal path). */ +export const resolveGlobalFlags = (options: Readonly>): GlobalFlags => { + const flags = {} as GlobalFlags; + + for (const definition of globalFlagDefinitions) { + flags[definition.key] = definition.fromOptions(options); + } + + return flags; +}; + +/** Resolves {@link GlobalFlags} directly from argv — used when `cac.parse()` itself throws before + * a parsed options object exists. */ +export const resolveGlobalFlagsFromArgv = (argv: readonly string[]): GlobalFlags => { + const flags = {} as GlobalFlags; + + for (const definition of globalFlagDefinitions) { + flags[definition.key] = definition.fromArgv(argv); + } + + return flags; +}; + +/** + * The single place that decides compact vs. indented JSON. Compact (`JSON.stringify(value)`) by + * default; `--pretty` restores the 2-space indentation the CLI used to always apply. + */ +export const formatJson = (value: unknown, flags: Pick): string => { + return flags.pretty ? JSON.stringify(value, null, 2) : JSON.stringify(value); +}; diff --git a/packages/appduct/src/cli/router.ts b/packages/appduct/src/cli/router.ts index bdb23499..c3f95755 100644 --- a/packages/appduct/src/cli/router.ts +++ b/packages/appduct/src/cli/router.ts @@ -13,7 +13,7 @@ * that must stay lean. */ -import type { CliRenderContext } from "./types.js"; +import type { CliEnv } from "./types.js"; import { usageError } from "../errors.js"; import { executeCommand } from "./runner.js"; @@ -44,13 +44,14 @@ export type VersionCheckInputs = { */ export type RouteContext = { /** The command words matched so far, e.g. `["daemon", "status"]`. Joined with spaces it is the - * `meta.command` a rendered result reports. */ + * command name used to pick a success-data renderer, and (under `--verbose`) the `meta.command` + * a rendered result reports. */ readonly path: readonly string[]; /** The positional arguments left after {@link path}: what the matched command itself receives. */ readonly args: readonly string[]; /** Every parsed flag (global and per-command), as `cac` reports them (camelCased). */ readonly options: Readonly>; - readonly io: CliRenderContext; + readonly env: CliEnv; /** The resolved state directory (`--state-dir` / `APPDUCT_STATE_DIR` / default). */ readonly stateDir: string; readonly versionCheck: VersionCheckInputs; @@ -67,7 +68,8 @@ export type RouterOptions = { readonly unknown: (word: string | undefined, context: RouteContext) => Error; }; -/** The `meta.command` string for a context: the matched words joined, or the level's own name. */ +/** The command name for a context (the `meta.command` a rendered result reports under + * `--verbose`): the matched words joined, or the level's own name. */ export const commandName = (context: RouteContext): string => { return context.path.join(" "); }; @@ -90,7 +92,7 @@ export const createRouter = (table: RouteTable, options: RouterOptions): Route = () => { throw options.unknown(word, context); }, - context.io, + context.env, ); } diff --git a/packages/appduct/src/cli/routes/daemon/run.ts b/packages/appduct/src/cli/routes/daemon/run.ts index dbd27e52..98b1cd09 100644 --- a/packages/appduct/src/cli/routes/daemon/run.ts +++ b/packages/appduct/src/cli/routes/daemon/run.ts @@ -7,11 +7,11 @@ import { commandName } from "../../router.js"; import { executeHostedCommand } from "../../runner.js"; export const route: Route = async (context) => { - const { stateDir, io } = context; + const { stateDir, env } = context; return executeHostedCommand( commandName(context), - () => handleDaemonRunCommand({ stateDir, clock: io.clock }), - io, + () => handleDaemonRunCommand({ stateDir, clock: env.clock }), + env, ); }; diff --git a/packages/appduct/src/cli/routes/daemon/start.ts b/packages/appduct/src/cli/routes/daemon/start.ts index 50565ccc..ff24c9d5 100644 --- a/packages/appduct/src/cli/routes/daemon/start.ts +++ b/packages/appduct/src/cli/routes/daemon/start.ts @@ -8,11 +8,11 @@ import { executeCommand } from "../../runner.js"; import { guarded } from "../../version-guard.js"; export const route: Route = async (context) => { - const { stateDir, io } = context; + const { stateDir, env } = context; return executeCommand( commandName(context), - guarded(context)(() => handleDaemonStartCommand({ stateDir, clock: io.clock })), - io, + guarded(context)(() => handleDaemonStartCommand({ stateDir, clock: env.clock })), + env, ); }; diff --git a/packages/appduct/src/cli/routes/daemon/status.ts b/packages/appduct/src/cli/routes/daemon/status.ts index 4845cde1..370fed8b 100644 --- a/packages/appduct/src/cli/routes/daemon/status.ts +++ b/packages/appduct/src/cli/routes/daemon/status.ts @@ -8,11 +8,11 @@ import { commandName } from "../../router.js"; import { executeCommand } from "../../runner.js"; export const route: Route = async (context) => { - const { stateDir, io } = context; + const { stateDir, env } = context; return executeCommand( commandName(context), - () => handleDaemonStatusCommand({ stateDir, clock: io.clock }), - io, + () => handleDaemonStatusCommand({ stateDir, clock: env.clock }), + env, ); }; diff --git a/packages/appduct/src/cli/routes/daemon/stop.ts b/packages/appduct/src/cli/routes/daemon/stop.ts index 4dd0d604..36d31330 100644 --- a/packages/appduct/src/cli/routes/daemon/stop.ts +++ b/packages/appduct/src/cli/routes/daemon/stop.ts @@ -8,11 +8,11 @@ import { commandName } from "../../router.js"; import { executeCommand } from "../../runner.js"; export const route: Route = async (context) => { - const { stateDir, io } = context; + const { stateDir, env } = context; return executeCommand( commandName(context), - () => handleDaemonStopCommand({ stateDir, clock: io.clock }), - io, + () => handleDaemonStopCommand({ stateDir, clock: env.clock }), + env, ); }; diff --git a/packages/appduct/src/cli/routes/doctor.ts b/packages/appduct/src/cli/routes/doctor.ts index e74930e6..6994a9c2 100644 --- a/packages/appduct/src/cli/routes/doctor.ts +++ b/packages/appduct/src/cli/routes/doctor.ts @@ -17,6 +17,6 @@ export const route: Route = async (context) => { assertPresent: Boolean(options.assertPresent), assertAbsent: Boolean(options.assertAbsent), }), - context.io, + context.env, ); }; diff --git a/packages/appduct/src/cli/routes/events.ts b/packages/appduct/src/cli/routes/events.ts index 22307472..f9fadd55 100644 --- a/packages/appduct/src/cli/routes/events.ts +++ b/packages/appduct/src/cli/routes/events.ts @@ -13,11 +13,10 @@ import { executeHostedCommand } from "../runner.js"; import { guarded } from "../version-guard.js"; export const route: Route = async (context) => { - const { options, stateDir, io } = context; + const { options, stateDir, env } = context; const { selector } = splitOptionalSelector(context.args, "events [selector]"); const since = parseNonNegativeIntegerOption(options.since, "--since"); const follow = Boolean(options.follow); - const render = { json: io.json, color: io.color }; return executeHostedCommand( commandName(context), @@ -34,21 +33,19 @@ export const route: Route = async (context) => { { stateDir, onEvent: (event: EventNotification) => { - io.stdout.write(`${renderEventLine(event, render)}\n`); + env.stdout.write(`${renderEventLine(event, env.flags)}\n`); }, onCursor: (cursor) => { - io.stdout.write(`${renderEventsCursorLine(cursor, render)}\n`); + env.stdout.write(`${renderEventsCursorLine(cursor, env.flags)}\n`); }, }, ); }), + env, { - ...io, - reporter: { - kind: "interactive", - onEvent: () => {}, - dispose: () => {}, - }, + kind: "interactive", + onEvent: () => {}, + dispose: () => {}, }, ); }; diff --git a/packages/appduct/src/cli/routes/init.ts b/packages/appduct/src/cli/routes/init.ts index 5b2ed3bc..5dc72c74 100644 --- a/packages/appduct/src/cli/routes/init.ts +++ b/packages/appduct/src/cli/routes/init.ts @@ -21,6 +21,6 @@ export const route: Route = async (context) => { // refuse to write a "safe to commit" project config into the daemon's own state. { stateDir }, ), - context.io, + context.env, ); }; diff --git a/packages/appduct/src/cli/routes/invoke.ts b/packages/appduct/src/cli/routes/invoke.ts index 55fe0ee3..bb79ee64 100644 --- a/packages/appduct/src/cli/routes/invoke.ts +++ b/packages/appduct/src/cli/routes/invoke.ts @@ -40,7 +40,7 @@ export const route: Route = async (context) => { cancelController.signal, ), ), - context.io, + context.env, ); } finally { process.off("SIGINT", onSigint); diff --git a/packages/appduct/src/cli/routes/keygen.ts b/packages/appduct/src/cli/routes/keygen.ts index 05f4874c..ea251d9b 100644 --- a/packages/appduct/src/cli/routes/keygen.ts +++ b/packages/appduct/src/cli/routes/keygen.ts @@ -19,6 +19,6 @@ export const route: Route = async (context) => { }, { stateDir }, ), - context.io, + context.env, ); }; diff --git a/packages/appduct/src/cli/routes/link.ts b/packages/appduct/src/cli/routes/link.ts index 450b20de..b58ad80b 100644 --- a/packages/appduct/src/cli/routes/link.ts +++ b/packages/appduct/src/cli/routes/link.ts @@ -36,9 +36,7 @@ export const route: Route = async (context) => { { stateDir }, ), ), - { - ...context.io, - qr: Boolean(options.qr), - }, + context.env, + { qr: Boolean(options.qr) }, ); }; diff --git a/packages/appduct/src/cli/routes/ls.ts b/packages/appduct/src/cli/routes/ls.ts index fab012c3..85dc47c1 100644 --- a/packages/appduct/src/cli/routes/ls.ts +++ b/packages/appduct/src/cli/routes/ls.ts @@ -13,6 +13,6 @@ export const route: Route = async (context) => { return executeCommand( commandName(context), guarded(context)(() => handleLsCommand({ stateDir })), - context.io, + context.env, ); }; diff --git a/packages/appduct/src/cli/routes/mcp.ts b/packages/appduct/src/cli/routes/mcp.ts index 0db40dbc..4705c7cd 100644 --- a/packages/appduct/src/cli/routes/mcp.ts +++ b/packages/appduct/src/cli/routes/mcp.ts @@ -10,7 +10,7 @@ import { executeHostedCommand } from "../runner.js"; import { versionCheckOptions } from "../version-guard.js"; export const route: Route = async (context) => { - const { options, stateDir, io } = context; + const { options, stateDir, env } = context; return executeHostedCommand( commandName(context), @@ -23,15 +23,13 @@ export const route: Route = async (context) => { handleMcpCommand({ stateDir, scheme: typeof options.scheme === "string" ? options.scheme : undefined, - checkVersion: await versionCheckOptions(context, (message) => void io.stderr.write(message)), + checkVersion: await versionCheckOptions(context, (message) => void env.stderr.write(message)), }), + env, { - ...io, - reporter: { - kind: "interactive", - onEvent: () => {}, - dispose: () => {}, - }, + kind: "interactive", + onEvent: () => {}, + dispose: () => {}, }, ); }; diff --git a/packages/appduct/src/cli/routes/revoke.ts b/packages/appduct/src/cli/routes/revoke.ts index 2a5d32cb..1850f84b 100644 --- a/packages/appduct/src/cli/routes/revoke.ts +++ b/packages/appduct/src/cli/routes/revoke.ts @@ -15,6 +15,6 @@ export const route: Route = async (context) => { return executeCommand( commandName(context), guarded(context)(() => handleRevokeCommand({ selector }, { stateDir })), - context.io, + context.env, ); }; diff --git a/packages/appduct/src/cli/routes/tools.ts b/packages/appduct/src/cli/routes/tools.ts index 7c0ef3a5..0166137c 100644 --- a/packages/appduct/src/cli/routes/tools.ts +++ b/packages/appduct/src/cli/routes/tools.ts @@ -20,6 +20,7 @@ export const route: Route = async (context) => { guarded(context)(() => handleToolsCommand({ selector: selector ?? selectorOrTarget, name: target }, { stateDir }), ), - { ...context.io, full: Boolean(options.full) }, + context.env, + { full: Boolean(options.full) }, ); }; diff --git a/packages/appduct/src/cli/runner.ts b/packages/appduct/src/cli/runner.ts index ec0a6d64..b69efbb5 100644 --- a/packages/appduct/src/cli/runner.ts +++ b/packages/appduct/src/cli/runner.ts @@ -1,8 +1,10 @@ import type { CliResult } from "./result-types.js"; import { getExitCodeForError, toCliError } from "../errors.js"; +import { formatJson } from "./global-flags.js"; import { renderResult, type RenderOptions } from "../output.js"; -import { createCommandMeta, type Clock, type CliIoWriters } from "./types.js"; +import { finalizeResult } from "./envelope.js"; +import type { CliEnv, CliIoWriters } from "./types.js"; /** * Long-lived command result shape shared by any CLI command that keeps a process alive @@ -43,42 +45,40 @@ const writeRenderedOutput = ( export const executeCommand = async ( command: string, handler: () => CliResult | Promise>, - options: CliIoWriters & - Pick & - Partial> & { - clock: Clock; - }, + env: CliEnv, + render: Partial> = {}, ): Promise => { - const startedAt = options.clock.now(); - const renderOptions: RenderOptions = { - command, - json: options.json, - color: options.color, - qr: options.qr, - full: options.full, - }; + const startedAt = env.clock.now(); try { const result = await handler(); - const finishedAt = options.clock.now(); - const withMeta: CliResult = { - ...result, - meta: createCommandMeta(command, startedAt, finishedAt), + const finishedAt = env.clock.now(); + const finalized = finalizeResult(result, { command, startedAt, finishedAt }, env.flags); + const renderOptions: RenderOptions = { + command, + flags: env.flags, + now: finishedAt, + qr: render.qr, + full: render.full, }; - writeRenderedOutput(renderResult(withMeta, renderOptions), options); + writeRenderedOutput(renderResult(finalized, renderOptions), env); return 0; } catch (error) { - const finishedAt = options.clock.now(); + const finishedAt = env.clock.now(); const cliError = toCliError(error); - const result: CliResult = { - ok: false, - error: cliError, - meta: createCommandMeta(command, startedAt, finishedAt), + const result: CliResult = { ok: false, error: cliError }; + const finalized = finalizeResult(result, { command, startedAt, finishedAt }, env.flags); + const renderOptions: RenderOptions = { + command, + flags: env.flags, + now: finishedAt, + qr: render.qr, + full: render.full, }; - writeRenderedOutput(renderResult(result, renderOptions), options); + writeRenderedOutput(renderResult(finalized, renderOptions), env); return getExitCodeForError(error); } @@ -87,38 +87,26 @@ export const executeCommand = async ( export const executeHostedCommand = async ( command: string, handler: () => Promise, - options: CliIoWriters & { - json: boolean; - color: boolean; - clock: Clock; - reporter?: HostedCommandReporter; - }, + env: CliEnv, + reporter?: HostedCommandReporter, ): Promise => { - const startedAt = options.clock.now(); + const startedAt = env.clock.now(); let renderedSuccess = false; try { const hosted = await handler(); - const finishedAt = options.clock.now(); - const withMeta: CliResult = { - ...hosted.result, - meta: createCommandMeta(command, startedAt, finishedAt), - }; + const finishedAt = env.clock.now(); + const finalized = finalizeResult(hosted.result, { command, startedAt, finishedAt }, env.flags); // A live reporter (e.g. `events`'s streaming NDJSON/human lines) renders its own output as it // goes; only the absence of a reporter (or an explicit "plain" one) falls back to the default // one-shot `renderResult` bootstrap rendering. - const liveReporter = options.reporter; - const shouldRenderBootstrap = !liveReporter || liveReporter.kind === "plain"; + const shouldRenderBootstrap = !reporter || reporter.kind === "plain"; if (shouldRenderBootstrap) { writeRenderedOutput( - renderResult(withMeta, { - command, - json: options.json, - color: options.color, - }), - options, + renderResult(finalized, { command, flags: env.flags, now: finishedAt }), + env, ); renderedSuccess = true; } @@ -142,37 +130,30 @@ export const executeHostedCommand = async ( resolved = true; process.off("SIGINT", stop); process.off("SIGTERM", stop); - options.reporter?.dispose(); + reporter?.dispose(); } return 0; } catch (error) { - options.reporter?.dispose(); + reporter?.dispose(); - const finishedAt = options.clock.now(); + const finishedAt = env.clock.now(); const cliError = toCliError(error); - const result: CliResult = { - ok: false, - error: cliError, - meta: createCommandMeta(command, startedAt, finishedAt), - }; + const result: CliResult = { ok: false, error: cliError }; + const finalized = finalizeResult(result, { command, startedAt, finishedAt }, env.flags); - if (renderedSuccess && options.json) { - // The single-JSON-object-on-stdout contract was already fulfilled by the bootstrap render + if (renderedSuccess && env.flags.json) { + // The single-JSON-document-on-stdout contract was already fulfilled by the bootstrap render // above; a failure that happens later (e.g. during a long-running `completion`) must still - // be a JSON object, just on stderr instead — bare text here was v1's defect (leaked - // unparseable output onto stderr in `--json` mode). - options.stderr.write(`${JSON.stringify(result)}\n`); + // be a JSON document, just on stderr instead — bare text here was v1's defect (leaked + // unparseable output onto stderr in `--json` mode). One document, so `--pretty` may apply. + env.stderr.write(`${formatJson(finalized, env.flags)}\n`); return getExitCodeForError(error); } writeRenderedOutput( - renderResult(result, { - command, - json: options.json, - color: options.color, - }), - options, + renderResult(finalized, { command, flags: env.flags, now: finishedAt }), + env, ); return getExitCodeForError(error); diff --git a/packages/appduct/src/cli/types.ts b/packages/appduct/src/cli/types.ts index 234d0fa7..d2081246 100644 --- a/packages/appduct/src/cli/types.ts +++ b/packages/appduct/src/cli/types.ts @@ -1,4 +1,4 @@ -import type { CommandMeta } from "./result-types.js"; +import type { GlobalFlags } from "./global-flags.js"; export type Clock = { now: () => Date; @@ -8,18 +8,6 @@ export const systemClock: Clock = { now: () => new Date(), }; -export const createCommandMeta = ( - command: string, - startedAt: Date, - finishedAt: Date, -): CommandMeta => { - return { - command, - timestamp: finishedAt.toISOString(), - duration_ms: finishedAt.getTime() - startedAt.getTime(), - }; -}; - export type RunCliOptions = { stdout?: Pick; stderr?: Pick; @@ -28,9 +16,14 @@ export type RunCliOptions = { export type CliIoWriters = Required>; -export type CliRenderContext = { - json: boolean; - color: boolean; +/** + * Everything a route's handler needs about how to behave and where to write, in one object: + * the resolved global flags plus the writers and clock. A future global flag is added to + * `global-flags.ts`'s table and becomes available as `env.flags.` everywhere this is + * threaded, with no other plumbing change. + */ +export type CliEnv = { + flags: GlobalFlags; stdout: CliIoWriters["stdout"]; stderr: CliIoWriters["stderr"]; clock: Clock; diff --git a/packages/appduct/src/output.ts b/packages/appduct/src/output.ts index c8fbd83f..060aab9d 100644 --- a/packages/appduct/src/output.ts +++ b/packages/appduct/src/output.ts @@ -18,24 +18,24 @@ import type { RevokeCommandData, ToolsCommandData, } from "./cli/result-types.js"; +import { formatJson, type GlobalFlags } from "./cli/global-flags.js"; import { renderQrToTerminal } from "./qr-terminal.js"; type ColorPalette = ReturnType; export type RenderOptions = { command: string; - json: boolean; - color: boolean; + flags: GlobalFlags; + /** "Now", for the one renderer (`ls`'s Age column) that needs a reference time absent `meta` + * (which is only attached under `--verbose`). Defaults to the wall clock when omitted, matching + * the pre-`--verbose` fallback behavior for a caller that doesn't care (e.g. non-`ls` tests). */ + now?: Date; /** `link`-only: also render the deep link as terminal QR art (never affects `--json` output). */ qr?: boolean; /** `tools`-only: render full schemas/annotations for every listed tool, not just name+description. */ full?: boolean; }; -const indentJson = (value: unknown): string => { - return JSON.stringify(value, null, 2); -}; - const renderMetaLines = (meta?: CommandMeta): string[] => { if (!meta) { return []; @@ -49,7 +49,7 @@ const renderMetaLines = (meta?: CommandMeta): string[] => { ]; }; -const formatScalar = (value: unknown): string => { +const formatScalar = (value: unknown, flags: GlobalFlags): string => { if (typeof value === "string") { return value; } @@ -62,10 +62,14 @@ const formatScalar = (value: unknown): string => { return "null"; } - return indentJson(value); + return formatJson(value, flags); }; -const renderFields = (title: string, fields: Array<[label: string, value: unknown]>): string[] => { +const renderFields = ( + title: string, + fields: Array<[label: string, value: unknown]>, + flags: GlobalFlags, +): string[] => { const visibleFields = fields.filter(([, value]) => value !== undefined); if (visibleFields.length === 0) { @@ -76,7 +80,7 @@ const renderFields = (title: string, fields: Array<[label: string, value: unknow return [ title, - ...visibleFields.map(([label, value]) => ` ${label.padEnd(width)} ${formatScalar(value)}`), + ...visibleFields.map(([label, value]) => ` ${label.padEnd(width)} ${formatScalar(value, flags)}`), ]; }; @@ -111,19 +115,19 @@ const formatDevice = (device: SessionSummary["device"]): string => { return device.os ? `${label} (${device.os})` : label; }; -const renderLsData = (colors: ColorPalette, data: LsCommandData, meta?: CommandMeta): string[] => { +const renderLsData = (colors: ColorPalette, data: LsCommandData, now: Date): string[] => { if (data.length === 0) { return [colors.dim("Sessions"), " No Appduct sessions are registered."]; } - const now = meta?.timestamp ?? new Date().toISOString(); + const nowIso = now.toISOString(); const headers = ["Alias", "State", "Device", "Tools", "Age"] as const; const rows = data.map((session) => [ session.alias, session.state, formatDevice(session.device), String(session.toolCount), - formatAge(session.claimedAt ?? session.createdAt, now), + formatAge(session.claimedAt ?? session.createdAt, nowIso), ]); const widths = headers.map((header, index) => @@ -151,47 +155,69 @@ const renderToolSummaryTable = (tools: ToolDescriptor[]): string[] => { ]; }; -const renderToolDetail = (colors: ColorPalette, tool: ToolDescriptor): string[] => { - return renderFields(colors.green(`Tool: ${tool.name}`), [ - ["Description", tool.description], - ["Input schema", tool.input_schema], - ["Output schema", tool.output_schema], - ["Annotations", tool.annotations], - // Only rendered for a tool that declares one; `renderFields` drops undefined rows, so a tool - // on the daemon's default deadline shows no line at all rather than a misleading "10000". - ["Timeout (ms)", tool.timeout_ms], - ]); +const renderToolDetail = (colors: ColorPalette, tool: ToolDescriptor, flags: GlobalFlags): string[] => { + return renderFields( + colors.green(`Tool: ${tool.name}`), + [ + ["Description", tool.description], + ["Input schema", tool.input_schema], + ["Output schema", tool.output_schema], + ["Annotations", tool.annotations], + // Only rendered for a tool that declares one; `renderFields` drops undefined rows, so a + // tool on the daemon's default deadline shows no line at all rather than a misleading + // "10000". + ["Timeout (ms)", tool.timeout_ms], + ], + flags, + ); }; -const renderToolsData = (colors: ColorPalette, data: ToolsCommandData, full?: boolean): string[] => { +const renderToolsData = ( + colors: ColorPalette, + data: ToolsCommandData, + flags: GlobalFlags, + full?: boolean, +): string[] => { if (!Array.isArray(data)) { - return renderToolDetail(colors, data); + return renderToolDetail(colors, data, flags); } if (full) { return data.length === 0 ? ["Tools", " No tools registered."] - : data.flatMap((tool, index) => [...(index > 0 ? [""] : []), ...renderToolDetail(colors, tool)]); + : data.flatMap((tool, index) => [ + ...(index > 0 ? [""] : []), + ...renderToolDetail(colors, tool, flags), + ]); } return [colors.green("Tools"), ...renderToolSummaryTable(data).slice(1)]; }; -const renderInvokeData = (colors: ColorPalette, data: InvokeCommandData): string[] => { - return [colors.green("Result"), formatScalar(data)]; +const renderInvokeData = (colors: ColorPalette, data: InvokeCommandData, flags: GlobalFlags): string[] => { + return [colors.green("Result"), formatScalar(data, flags)]; }; -const renderLinkData = (colors: ColorPalette, data: LinkCommandData, qr?: boolean): string[] => { +const renderLinkData = ( + colors: ColorPalette, + data: LinkCommandData, + flags: GlobalFlags, + qr?: boolean, +): string[] => { const lines = [ colors.green("Link Created"), - ...renderFields("Link", [ - ["Session", data.sessionId], - ["Deep link", data.deepLink], - ["Endpoint", formatAgentWebSocketUrl(data.endpoint)], - ["Pin", data.pin], - ["Expires", new Date(data.expiresAt * 1000).toISOString()], - ["Delivered", data.delivered ? `yes (${data.target})` : undefined], - ]), + ...renderFields( + "Link", + [ + ["Session", data.sessionId], + ["Deep link", data.deepLink], + ["Endpoint", formatAgentWebSocketUrl(data.endpoint)], + ["Pin", data.pin], + ["Expires", new Date(data.expiresAt * 1000).toISOString()], + ["Delivered", data.delivered ? `yes (${data.target})` : undefined], + ], + flags, + ), ]; if (qr) { @@ -205,17 +231,23 @@ const renderRevokeData = (colors: ColorPalette, _data: RevokeCommandData): strin return [colors.green("Session Revoked")]; }; -const renderKeygenData = (colors: ColorPalette, data: KeygenCommandData): string[] => { +const renderKeygenData = (colors: ColorPalette, data: KeygenCommandData, flags: GlobalFlags): string[] => { return [ colors.green("Key Ready"), - ...renderFields("Key", [ - ["Path", data.path], - ["Fingerprint", data.pin], - ]), + ...renderFields( + "Key", + [ + ["Path", data.path], + ["Fingerprint", data.pin], + ], + flags, + ), ]; }; -const renderInitData = (colors: ColorPalette, data: InitCommandData): string[] => { +const renderInitData = (colors: ColorPalette, data: InitCommandData, flags: GlobalFlags): string[] => { + // The pasteable MCP snippet is always shown indented, regardless of `--pretty` — it is meant to + // be copied straight into a JSON config file, not machine-parsed output. const snippet = JSON.stringify( { mcpServers: { appduct: data.mcpServerEntry } }, null, @@ -224,15 +256,19 @@ const renderInitData = (colors: ColorPalette, data: InitCommandData): string[] = return [ colors.green(data.changed ? "Project Initialized" : "Project Already Initialized"), - ...renderFields("Config", [ - ["Path", data.path], - ["Scheme", data.scheme], - ["Source", data.source], - // Only present for a scheme discovery read off a static project file (app.json, or one of - // the native Android/iOS probes) — names the exact file/key, not just which platform. - ["Read from", data.origin], - ["Written", data.changed ? (data.created ? "created" : "updated") : "unchanged"], - ]), + ...renderFields( + "Config", + [ + ["Path", data.path], + ["Scheme", data.scheme], + ["Source", data.source], + // Only present for a scheme discovery read off a static project file (app.json, or one of + // the native Android/iOS probes) — names the exact file/key, not just which platform. + ["Read from", data.origin], + ["Written", data.changed ? (data.created ? "created" : "updated") : "unchanged"], + ], + flags, + ), ...(data.note === undefined ? [] : ["", colors.yellow(`Note: ${data.note}`)]), "", "MCP server entry", @@ -243,32 +279,52 @@ const renderInitData = (colors: ColorPalette, data: InitCommandData): string[] = ]; }; -const renderDaemonRunData = (colors: ColorPalette, data: DaemonRunCommandData): string[] => { +const renderDaemonRunData = ( + colors: ColorPalette, + data: DaemonRunCommandData, + flags: GlobalFlags, +): string[] => { return [ colors.green("Daemon Running"), - ...renderFields("Daemon", [ - ["PID", data.daemon.pid], - ["State dir", data.daemon.state_dir], - ["Socket", data.daemon.socket_path], - ]), + ...renderFields( + "Daemon", + [ + ["PID", data.daemon.pid], + ["State dir", data.daemon.state_dir], + ["Socket", data.daemon.socket_path], + ], + flags, + ), ]; }; -const renderDaemonStartData = (colors: ColorPalette, data: DaemonStartCommandData): string[] => { +const renderDaemonStartData = ( + colors: ColorPalette, + data: DaemonStartCommandData, + flags: GlobalFlags, +): string[] => { return [ colors.green("Daemon Started"), - ...renderFields("Daemon", [ - ["PID", data.daemon.pid], - ["WSS port", data.daemon.wss_port], - ["Started at", data.daemon.started_at], - ]), + ...renderFields( + "Daemon", + [ + ["PID", data.daemon.pid], + ["WSS port", data.daemon.wss_port], + ["Started at", data.daemon.started_at], + ], + flags, + ), ]; }; -const renderDaemonStopData = (colors: ColorPalette, data: DaemonStopCommandData): string[] => { +const renderDaemonStopData = ( + colors: ColorPalette, + data: DaemonStopCommandData, + flags: GlobalFlags, +): string[] => { return [ colors.green("Daemon Stopped"), - ...renderFields("Daemon", [["Method", data.daemon.method]]), + ...renderFields("Daemon", [["Method", data.daemon.method]], flags), ]; }; @@ -288,99 +344,119 @@ const formatByteSize = (bytes: number): string => { return `${unit === 0 ? value : value.toFixed(1)} ${units[unit]}`; }; -const renderDaemonStatusData = (colors: ColorPalette, data: DaemonStatusCommandData): string[] => { +const renderDaemonStatusData = ( + colors: ColorPalette, + data: DaemonStatusCommandData, + flags: GlobalFlags, +): string[] => { return [ colors.green("Daemon Status"), - ...renderFields("Daemon", [ - ["Version", data.daemon.version], - ["PID", data.daemon.pid], - ["Started at", data.daemon.started_at], - ["WSS port", data.daemon.wss_port], - ["Pinned keys", data.daemon.pinned_keys.length], - ["Sessions", data.daemon.session_count], - ]), + ...renderFields( + "Daemon", + [ + ["Version", data.daemon.version], + ["PID", data.daemon.pid], + ["Started at", data.daemon.started_at], + ["WSS port", data.daemon.wss_port], + ["Pinned keys", data.daemon.pinned_keys.length], + ["Sessions", data.daemon.session_count], + ], + flags, + ), "", - ...renderFields("Policy", [ - ["Default", data.policy.default], - ["Destructive", data.policy.destructive], - ["Overrides", data.policy.tools ? Object.keys(data.policy.tools).length : 0], - ]), + ...renderFields( + "Policy", + [ + ["Default", data.policy.default], + ["Destructive", data.policy.destructive], + ["Overrides", data.policy.tools ? Object.keys(data.policy.tools).length : 0], + ], + flags, + ), "", // The retention rows drop out entirely against a daemon that predates them (`renderFields` // skips `undefined`), rather than printing "undefined" or an invented zero. - ...renderFields("Audit", [ - ["Path", data.audit.path], - ["Retention", data.audit.retention_days === undefined ? undefined : `${data.audit.retention_days} days`], - ["Files", data.audit.files], - ["Size", data.audit.bytes === undefined ? undefined : formatByteSize(data.audit.bytes)], - ["Failed writes", data.audit.failed_writes], - ["Failed prunes", data.audit.failed_prunes], - ]), + ...renderFields( + "Audit", + [ + ["Path", data.audit.path], + ["Retention", data.audit.retention_days === undefined ? undefined : `${data.audit.retention_days} days`], + ["Files", data.audit.files], + ["Size", data.audit.bytes === undefined ? undefined : formatByteSize(data.audit.bytes)], + ["Failed writes", data.audit.failed_writes], + ["Failed prunes", data.audit.failed_prunes], + ], + flags, + ), // Version drift (issue #30) is the one thing here an operator has to act on, so it goes last // — the line still on screen after the block scrolls — and in yellow, not a quiet field row. ...(data.warning ? ["", colors.yellow(`Warning: ${data.warning}`)] : []), ]; }; -const renderDoctorData = (colors: ColorPalette, data: DoctorCommandData): string[] => { +const renderDoctorData = (colors: ColorPalette, data: DoctorCommandData, flags: GlobalFlags): string[] => { return [ data.present ? colors.green("Appduct Present") : colors.yellow("Appduct Absent"), - ...renderFields("Artifact", [ - ["Path", data.artifact], - ["Platform", data.platform], - ["Format", data.format], - ["Present", data.present], - ["Signals", data.signals.length > 0 ? data.signals.join(", ") : "none"], - ["Assertion", data.assertion ? `${data.assertion.expected} (holds)` : undefined], - ]), + ...renderFields( + "Artifact", + [ + ["Path", data.artifact], + ["Platform", data.platform], + ["Format", data.format], + ["Present", data.present], + ["Signals", data.signals.length > 0 ? data.signals.join(", ") : "none"], + ["Assertion", data.assertion ? `${data.assertion.expected} (holds)` : undefined], + ], + flags, + ), ]; }; -const renderSuccessData = ( - colors: ColorPalette, - command: string, - data: unknown, - options: RenderOptions, - meta?: CommandMeta, -): string[] => { +const renderSuccessData = (colors: ColorPalette, command: string, data: unknown, options: RenderOptions): string[] => { + const flags = options.flags; + switch (command) { case "init": - return renderInitData(colors, data as InitCommandData); + return renderInitData(colors, data as InitCommandData, flags); case "keygen": - return renderKeygenData(colors, data as KeygenCommandData); + return renderKeygenData(colors, data as KeygenCommandData, flags); case "link": - return renderLinkData(colors, data as LinkCommandData, options.qr); + return renderLinkData(colors, data as LinkCommandData, flags, options.qr); case "ls": - return renderLsData(colors, data as LsCommandData, meta); + return renderLsData(colors, data as LsCommandData, options.now ?? new Date()); case "tools": - return renderToolsData(colors, data as ToolsCommandData, options.full); + return renderToolsData(colors, data as ToolsCommandData, flags, options.full); case "invoke": - return renderInvokeData(colors, data as InvokeCommandData); + return renderInvokeData(colors, data as InvokeCommandData, flags); case "revoke": return renderRevokeData(colors, data as RevokeCommandData); case "daemon run": - return renderDaemonRunData(colors, data as DaemonRunCommandData); + return renderDaemonRunData(colors, data as DaemonRunCommandData, flags); case "daemon start": - return renderDaemonStartData(colors, data as DaemonStartCommandData); + return renderDaemonStartData(colors, data as DaemonStartCommandData, flags); case "daemon stop": - return renderDaemonStopData(colors, data as DaemonStopCommandData); + return renderDaemonStopData(colors, data as DaemonStopCommandData, flags); case "daemon status": - return renderDaemonStatusData(colors, data as DaemonStatusCommandData); + return renderDaemonStatusData(colors, data as DaemonStatusCommandData, flags); case "doctor": - return renderDoctorData(colors, data as DoctorCommandData); + return renderDoctorData(colors, data as DoctorCommandData, flags); default: - return [colors.green("Command Complete"), indentJson(data)]; + return [colors.green("Command Complete"), formatJson(data, flags)]; } }; -const renderHumanError = (colors: ColorPalette, error: CliError, meta?: CommandMeta): string => { +const renderHumanError = (colors: ColorPalette, error: CliError, flags: GlobalFlags, meta?: CommandMeta): string => { return [ colors.red("Command Failed"), - ...renderFields("Error", [ - ["Type", error.type], - ["Message", error.message], - ["Details", error.details], - ]), + ...renderFields( + "Error", + [ + ["Type", error.type], + ["Message", error.message], + ["Details", error.details], + ], + flags, + ), ...(meta ? ["", ...renderMetaLines(meta)] : []), ].join("\n"); }; @@ -392,22 +468,24 @@ export const renderResult = ( stdout?: string; stderr?: string; } => { - if (options.json) { + if (options.flags.json) { return { - stdout: `${indentJson(result)}\n`, + stdout: `${formatJson(result, options.flags)}\n`, }; } - const colors = pc.createColors(options.color); + const colors = pc.createColors(options.flags.color); if (!result.ok) { return { - stderr: `${renderHumanError(colors, result.error, result.meta)}\n`, + stderr: `${renderHumanError(colors, result.error, options.flags, result.meta)}\n`, }; } + // `result.meta` is only present under `--verbose` (`cli/envelope.ts`'s `finalizeResult`), so + // rendering it here keys purely off presence — no separate verbose check needed. const lines = [ - ...renderSuccessData(colors, options.command, result.data, options, result.meta), + ...renderSuccessData(colors, options.command, result.data, options), "", ...renderMetaLines(result.meta), ].filter((line, index, collection) => { @@ -424,18 +502,17 @@ export const renderResult = ( }; /** Renders one `appduct events` line: NDJSON under `--json`, a compact human line otherwise. */ -export const renderEventLine = ( - event: EventNotification, - options: { json: boolean; color: boolean }, -): string => { - if (options.json) { +export const renderEventLine = (event: EventNotification, flags: GlobalFlags): string => { + if (flags.json) { + // NDJSON is one object per line by contract (a streaming consumer reads it line-by-line, and + // the cursor resume logic depends on that) — always compact, `--pretty` never applies here. return JSON.stringify(event); } - const colors = pc.createColors(options.color); + const colors = pc.createColors(flags.color); const timestamp = new Date(event.ts).toISOString(); const target = event.alias ?? event.sessionId; - const dataSuffix = event.data === undefined ? "" : ` ${indentJson(event.data)}`; + const dataSuffix = event.data === undefined ? "" : ` ${formatJson(event.data, flags)}`; return `${colors.dim(timestamp)} ${colors.green(event.kind)}${target ? ` ${target}` : ""}${dataSuffix}`; }; @@ -443,11 +520,12 @@ export const renderEventLine = ( /** Renders the trailing cursor line for `appduct events --since` (issue #6): NDJSON under * `--json` so a scripted caller can parse the resume point without maxing `seq` over the printed * events (impossible when the response is empty), a human note otherwise. */ -export const renderEventsCursorLine = (cursor: number, options: { json: boolean; color: boolean }): string => { - if (options.json) { +export const renderEventsCursorLine = (cursor: number, flags: GlobalFlags): string => { + if (flags.json) { + // Same NDJSON rule as renderEventLine above: always one compact line, never `--pretty`. return JSON.stringify({ cursor }); } - const colors = pc.createColors(options.color); + const colors = pc.createColors(flags.color); return colors.dim(`cursor: ${cursor} (pass --since ${cursor} to resume from here)`); }; From b19b95487eaf7b35694a3673da9d6be4c556e8ab Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 20:29:52 +0000 Subject: [PATCH 2/8] feat(cli): signature-based tools listing with --filter, --limit, --offset; agent skill defaults to plain text `appduct tools` now renders each tool as a one-line call signature (`name(params) -> result`, via new `@appduct/shared` renderToolSignature) plus the description's first line, instead of a bare name/description table, and gains --filter/--limit/--offset so a large registry stays cheap for an agent to read. `tools.list` sorts by name, filters, and pages daemon-side, and now returns `{ tools, total }` instead of a bare array (a breaking CLI --json change) so the CLI can report how many tools were left off a page. The appduct agent skill's example commands default to plain text, adding --json only where a script will parse the output. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_018zpvXWBBSJZnNVrqbzpFB4 --- CHANGELOG.md | 12 + docs/ARCHITECTURE.md | 7 +- packages/appduct/README.md | 20 +- .../__snapshots__/output.test.ts.snap | 7 +- .../src/__tests__/cli-v2.integration.test.ts | 107 +++++- .../src/__tests__/e2e/cold-start.e2e.test.ts | 5 +- packages/appduct/src/__tests__/output.test.ts | 170 ++++++++- .../tool-invocation.integration.test.ts | 84 ++++- packages/appduct/src/cli/create-cli.ts | 5 +- packages/appduct/src/cli/result-types.ts | 15 +- packages/appduct/src/cli/routes/tools.ts | 14 +- packages/appduct/src/client/app-client.ts | 5 +- packages/appduct/src/commands/tools.ts | 58 ++- packages/appduct/src/daemon/daemon.ts | 65 +++- packages/appduct/src/mcp/daemon-tools.ts | 6 +- packages/appduct/src/output.ts | 95 ++++- .../src/__tests__/tool-signature.test.ts | 345 ++++++++++++++++++ packages/shared/src/domains/rpc.ts | 21 +- packages/shared/src/domains/tool-signature.ts | 221 +++++++++++ packages/shared/src/index.ts | 1 + skills/appduct/SKILL.md | 32 +- 21 files changed, 1221 insertions(+), 74 deletions(-) create mode 100644 packages/shared/src/__tests__/tool-signature.test.ts create mode 100644 packages/shared/src/domains/tool-signature.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f2036f31..6626df64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,18 @@ package versions for a release. the trailing `Meta` lines in human mode, the `meta` field on the `--json` envelope. - **New: `--pretty` and `--verbose` global flags**, alongside `--json` and `--no-color`. See the [`appduct` README](packages/appduct/README.md) for the full description of each. +- **Breaking (CLI): `appduct tools --json` for a listing now returns `{ tools, total }`** instead + of a bare array. The single-tool form (`appduct tools `) is unchanged — it still + returns the bare tool descriptor. +- **New: a signature-based `tools` listing, with `--filter`/`--limit`/`--offset`.** The human + listing now shows one call signature (`name(params) -> result`) plus a one-line description per + tool instead of a bare name/description table, and `appduct tools` gains `--filter ` to + narrow by name/description, and `--limit `/`--offset ` to page through a large registry — + making it cheap to read `appduct tools` against an app that registers hundreds of tools. See the + [`appduct` README](packages/appduct/README.md)'s "`appduct tools`: a signature per tool" section + for details. +- The `appduct` agent skill now defaults its example commands to plain-text output, adding + `--json` only where a script (not the agent itself) will parse the result. ## 0.10.0 (2026-09-16) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index d55ba449..3e3cb2f9 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -262,7 +262,7 @@ Methods: | `sessions.list` | — | `SessionSummary[]` | | `sessions.describe` | `{ selector? }` | full session detail incl. device metadata, state timestamps, tool count | | `sessions.revoke` | `{ selector? }` | `{ ok: true }` — closes socket (code 1000), frees alias | -| `tools.list` | `{ selector? }` | `ToolsListEntry[]` — `ToolDescriptor` (full schema + annotations) plus the tool's effective `policy: "allow" \| "deny" \| "prompt"` (§12), resolved daemon-side | +| `tools.list` | `{ selector?, filter?, limit?, offset? }` | `{ tools: ToolsListEntry[], total }` — `tools` is the registry sorted by `name` (code-point order), `filter`ed (case-insensitive substring match against name/description) and paged with `limit`/`offset`; each entry is a `ToolDescriptor` (full schema + annotations) plus the tool's effective `policy: "allow" \| "deny" \| "prompt"` (§12), resolved daemon-side. `total` is the filtered count *before* paging, so a caller can tell how much a page left out | | `tools.call` | `{ selector?, name, args, timeoutMs?, caller?: "cli" \| "mcp", consent?: "client" \| "elicitation" }` | `{ result, callId }` on success — `callId` lets a caller with several in-flight calls match `tool_call_progress`/`tool_call_finished` events back to this call; JSON-RPC error with `data.type` preserving the wire error type on failure. `caller` attributes the audit record (§12); `consent` is the MCP server's evidence of a `"prompt"`-policy human gate (§12) — `"client"` (the flag-based gate) or `"elicitation"` (the elicitation-based gate), absent for the CLI. | | `tools.cancel` | `{ selector?, callId, reason? }` | `{ cancelled: boolean }` — sends `tool_cancel` (§7) to the app for a still-pending call; `false` for an unknown/already-finished `callId` or no active socket (a no-op, not an error) | | `events.subscribe` | `{ sessionSelector?, kinds? }` | `{ ok: true }`, then `event` notifications on this connection | @@ -492,7 +492,10 @@ owns no keys. Every command is one RPC call plus formatting, which is why the CL server can't drift in behavior: they are the same calls. The per-command reference lives in the [`appduct` package README](../packages/appduct/README.md), -which is where it stays current. Global flags (`cli/global-flags.ts`'s declarative table): `--json` +which is where it stays current. `appduct tools`'s human listing renders each tool through +`@appduct/shared`'s `renderToolSignature` (a one-line call signature derived from the tool's JSON +Schema) rather than printing the raw schema, so it stays cheap to read against an app that +registers hundreds of tools. Global flags (`cli/global-flags.ts`'s declarative table): `--json` (machine output, NDJSON for streams; compact by default), `--pretty` (indent `--json` output and embedded JSON values, never NDJSON lines), `--verbose` (include the `meta` block — omitted by default in both human and `--json` output), `--no-color`, `--state-dir`, `--daemon-restart` (force diff --git a/packages/appduct/README.md b/packages/appduct/README.md index 96aa65c6..a4d66c82 100644 --- a/packages/appduct/README.md +++ b/packages/appduct/README.md @@ -39,7 +39,7 @@ That's the whole loop. There is no host process to start — `appduct` auto-spaw | `appduct keygen [--out ] [--force]` | generate a daemon private key, print its app pin | | `appduct link [--ttl ] [--qr] [--open android\|ios-sim\|ios-device] [--device ] [--bundle-id ] [--scheme ]` | mint a pending session and print its deep link | | `appduct ls` | list sessions: alias, state, device, tool count | -| `appduct tools [selector] [name] [--full]` | list a session's tools, or show one tool's full schema | +| `appduct tools [selector] [name] [--full] [--filter ] [--limit ] [--offset ]` | list a session's tools (one call signature + description per line), or show one tool's full schema | | `appduct invoke [selector] --input '' [--timeout ]` | call a tool | | `appduct events [selector] [--follow] [--since ]` | stream session/tool events (default), or one-shot pull everything retained since `` (`--since`); `--json` emits NDJSON | | `appduct revoke [selector]` | revoke a session | @@ -51,6 +51,24 @@ Every command that targets a session accepts an optional `selector` (a session i `--timeout` on `invoke` is clamped to 1,000–600,000 ms and can only **shorten** the deadline, never extend it past the app's own timer: the app aborts the handler at the tool's declared `timeoutMs`, or 10 seconds for a tool that declares none, regardless of what the caller asks for. If a tool needs more room, declare `timeoutMs` on its registration. +### `appduct tools`: a signature per tool + +`appduct tools` prints one call signature plus a one-line description per tool, not the full schema — cheap to read even against an app that registers hundreds of tools: + +``` +Tools + seed_cart(items: int, sku?: string, clear?: bool = true) -> { added: int, cartId: string } + Fill the cart with test items for the current user. + set_flag(name: "dark_mode" | "new_checkout", enabled: bool) [prompt] + Toggle a feature flag. + +Run `appduct tools ` for a tool's full schema. +``` + +A signature is derived straight from the tool's JSON Schema: required params are `name: type`, optional ones `name?: type` (with `= ` when the schema declares a short one), and `-> type` is the result when the tool declares an `output_schema`. `...` anywhere means the schema shape wasn't one this renderer could summarize — the tool's full schema (`appduct tools `) still has it. A `[prompt]`/`[deny]` tag follows a tool whose effective policy isn't `"allow"`. + +Use `--filter ` to narrow the listing to tools whose name or description contains `` (case-insensitive), and `--limit `/`--offset ` to page through it; a truncated listing prints a trailing `Showing n of total tools (offset o). Narrow with --filter or page with --offset .` line so you know more were left out. `appduct tools --json` returns `{ tools, total }` — `total` is the count after `--filter` but before `--limit`/`--offset`. `appduct tools ` (a single tool) is unaffected by any of this and always returns the bare tool descriptor. + ### The deep-link scheme `appduct link`, `appduct mcp`, and the MCP `appduct_connect` tool all resolve the scheme the same way, first match wins: diff --git a/packages/appduct/src/__tests__/__snapshots__/output.test.ts.snap b/packages/appduct/src/__tests__/__snapshots__/output.test.ts.snap index 3bcd41bb..c3d3cd2a 100644 --- a/packages/appduct/src/__tests__/__snapshots__/output.test.ts.snap +++ b/packages/appduct/src/__tests__/__snapshots__/output.test.ts.snap @@ -122,6 +122,7 @@ exports[`output rendering > ls human output includes alias, state, device, tools exports[`output rendering > tools detail output renders full schema/annotations 1`] = ` "Tool: echo + Signature echo(...) Description Echo a payload on the connected device. Input schema {"type":"object"} Annotations {"readOnlyHint":true} @@ -130,7 +131,9 @@ exports[`output rendering > tools detail output renders full schema/annotations exports[`output rendering > tools list output stays structured 1`] = ` "Tools - Name Description - echo Echo a payload on the connected device. + echo(...) -> ... + Echo a payload on the connected device. + +Run \`appduct tools \` for a tool's full schema. " `; diff --git a/packages/appduct/src/__tests__/cli-v2.integration.test.ts b/packages/appduct/src/__tests__/cli-v2.integration.test.ts index 50ee9744..9e0f74f6 100644 --- a/packages/appduct/src/__tests__/cli-v2.integration.test.ts +++ b/packages/appduct/src/__tests__/cli-v2.integration.test.ts @@ -93,6 +93,18 @@ const runCliJson = async (args: string[], stateDir: string): Promise => { + const proc = spawnCliBinary(args, { stateDir }); + + const [stdout, stderr] = await Promise.all([text(proc.stdout), text(proc.stderr)]); + await waitForExit(proc); + + return { stdout, stderr }; +}; + const connectFakeApp = (port: number): Promise => { return new Promise((resolve, reject) => { const socket = new WebSocket(`wss://127.0.0.1:${port}`, { rejectUnauthorized: false }); @@ -201,7 +213,9 @@ describe("appduct CLI v2: end-to-end command table", () => { // tools: list, then detail by name. const toolsList = await runCliJson(["tools", alias], stateDir); expect(toolsList.ok).toBe(true); - expect((toolsList.data as Array<{ name: string }>).map((tool) => tool.name)).toEqual(["echo"]); + const toolsListData = toolsList.data as { tools: Array<{ name: string }>; total: number }; + expect(toolsListData.tools.map((tool) => tool.name)).toEqual(["echo"]); + expect(toolsListData.total).toBe(1); const toolsDetail = await runCliJson(["tools", alias, "echo"], stateDir); expect(toolsDetail.ok).toBe(true); @@ -305,4 +319,95 @@ describe("appduct CLI v2: end-to-end command table", () => { }, 15_000, ); + + test( + "tools --filter/--limit/--offset page a large registry, a name lookup still resolves under paging, and human output is a signature listing", + async () => { + const stateDir = await makeTempStateDir(); + const port = JSON.parse(await readFile(path.join(stateDir, "config.json"), "utf8")).wssPort as number; + + const status = await runCliJson(["daemon", "status"], stateDir); + expect(status.ok).toBe(true); + daemonPids.push((status.data as { daemon: { pid: number } }).daemon.pid); + + const linkResult = await runCliJson(["link", "--ttl", "60"], stateDir); + const linkData = linkResult.data as { deepLink: string }; + const linkPayload = linkData.deepLink + .slice(linkData.deepLink.indexOf("appduct=") + "appduct=".length) + .split("&")[0]!; + const decoded = decodeBootstrap(linkPayload)!; + + const socket = await connectFakeApp(port); + socket.send( + JSON.stringify({ + type: "session_claim", + protocol_version: 2, + session_id: decoded.sessionId, + token: decoded.token, + device_model: "Pixel 8", + }), + ); + const ack = await nextMessage(socket); + const alias = ack.alias as string; + + // ~30 tools, `tool_00`..`tool_29`, already sorted so the ordering assertions below double as + // a smoke check that the daemon's own sort doesn't re-scramble an already-sorted registry. + const toolNames = Array.from({ length: 30 }, (_, index) => `tool_${String(index).padStart(2, "0")}`); + socket.send( + JSON.stringify({ + type: "tool_registry_snapshot", + session_id: decoded.sessionId, + tools: toolNames.map((name) => ({ + name, + description: `Does something with ${name}.`, + input_schema: { type: "object", properties: { value: { type: "string" } }, required: ["value"] }, + })), + }), + ); + await new Promise((resolve) => setTimeout(resolve, 100)); + + // Unpaged: `{ tools, total }`, total tools registered. + const unpaged = await runCliJson(["tools", alias], stateDir); + expect(unpaged.ok).toBe(true); + const unpagedData = unpaged.data as { tools: Array<{ name: string }>; total: number }; + expect(unpagedData.tools).toHaveLength(30); + expect(unpagedData.total).toBe(30); + + // --filter narrows both the page and `total`. + const filtered = await runCliJson(["tools", alias, "--filter", "tool_1"], stateDir); + expect(filtered.ok).toBe(true); + const filteredData = filtered.data as { tools: Array<{ name: string }>; total: number }; + // tool_10..tool_19 (substring match on the name), still name-sorted. + expect(filteredData.total).toBe(10); + expect(filteredData.tools.map((tool) => tool.name)).toEqual(toolNames.slice(10, 20)); + + // --limit/--offset returns the right slice of the sorted, unfiltered registry. + const paged = await runCliJson(["tools", alias, "--limit", "5", "--offset", "5"], stateDir); + expect(paged.ok).toBe(true); + const pagedData = paged.data as { tools: Array<{ name: string }>; total: number }; + expect(pagedData.tools.map((tool) => tool.name)).toEqual(toolNames.slice(5, 10)); + expect(pagedData.total).toBe(30); + + // A name lookup still resolves even though it would fall outside a small page. + const detailUnderPaging = await runCliJson(["tools", alias, "tool_29", "--limit", "1"], stateDir); + expect(detailUnderPaging.ok).toBe(false); + expect(detailUnderPaging.error?.type).toBe("usage_error"); + + const detail = await runCliJson(["tools", alias, "tool_29"], stateDir); + expect(detail.ok).toBe(true); + expect((detail.data as { name: string }).name).toBe("tool_29"); + + // Human output: a signature line per tool, and the "Showing" line once the page truncates. + const human = await runCliHuman(["tools", alias, "--limit", "5"], stateDir); + expect(human.stdout).toContain("tool_00(value: string)"); + expect(human.stdout).toContain("Showing 5 of 30 tools (offset 0)."); + expect(human.stdout).toContain("Run `appduct tools ` for a tool's full schema."); + + socket.close(); + + const stopResult = await runCliJson(["daemon", "stop"], stateDir); + expect(stopResult.ok).toBe(true); + }, + 20_000, + ); }); diff --git a/packages/appduct/src/__tests__/e2e/cold-start.e2e.test.ts b/packages/appduct/src/__tests__/e2e/cold-start.e2e.test.ts index 00c9e188..006bc03b 100644 --- a/packages/appduct/src/__tests__/e2e/cold-start.e2e.test.ts +++ b/packages/appduct/src/__tests__/e2e/cold-start.e2e.test.ts @@ -90,9 +90,10 @@ describe("e2e: cold start", () => { expect(lsResult.data![0]!.toolCount).toBe(1); // tools: list, then detail by name. - const toolsList = await runCliJson>(["tools", alias], stateDir); + const toolsList = await runCliJson<{ tools: Array<{ name: string }>; total: number }>(["tools", alias], stateDir); expect(toolsList.ok).toBe(true); - expect(toolsList.data!.map((tool) => tool.name)).toEqual(["echo"]); + expect(toolsList.data!.tools.map((tool) => tool.name)).toEqual(["echo"]); + expect(toolsList.data!.total).toBe(1); // invoke: round-trip through the fake app. app.answerCalls((call) => ({ result: { echoed: (call.args as Record).text } })); diff --git a/packages/appduct/src/__tests__/output.test.ts b/packages/appduct/src/__tests__/output.test.ts index bab3c4c4..73695448 100644 --- a/packages/appduct/src/__tests__/output.test.ts +++ b/packages/appduct/src/__tests__/output.test.ts @@ -20,14 +20,18 @@ describe("output rendering", () => { const rendered = renderResult( { ok: true, - data: [ - { - name: "echo", - description: "Echo a payload on the connected device.", - input_schema: {}, - output_schema: { echoed: "unknown" }, - }, - ], + data: { + tools: [ + { + name: "echo", + description: "Echo a payload on the connected device.", + input_schema: {}, + output_schema: { echoed: "unknown" }, + policy: "allow", + }, + ], + total: 1, + }, }, { command: "tools", @@ -38,6 +42,156 @@ describe("output rendering", () => { expect(rendered.stdout).toMatchSnapshot(); }); + test("tools list output shows a signature per tool, a policy tag for non-allow tools, and the trailing hint", () => { + const rendered = renderResult( + { + ok: true, + data: { + tools: [ + { + name: "seed_cart", + description: "Fill the cart with test items for the current user.\nSecond line ignored.", + input_schema: { + type: "object", + properties: { + items: { type: "integer" }, + sku: { type: "string" }, + clear: { type: "boolean", default: true }, + }, + required: ["items"], + }, + output_schema: { + type: "object", + properties: { added: { type: "integer" }, cartId: { type: "string" } }, + required: ["added", "cartId"], + }, + policy: "allow", + }, + { + name: "set_flag", + description: "Toggle a feature flag.", + input_schema: { + type: "object", + properties: { + name: { enum: ["dark_mode", "new_checkout"] }, + enabled: { type: "boolean" }, + }, + required: ["name", "enabled"], + }, + policy: "prompt", + }, + ], + total: 2, + }, + }, + { command: "tools", flags: flags() }, + ); + + expect(rendered.stdout).toBe( + [ + "Tools", + " seed_cart(items: int, sku?: string, clear?: bool = true) -> { added: int, cartId: string }", + " Fill the cart with test items for the current user.", + ' set_flag(name: "dark_mode" | "new_checkout", enabled: bool) [prompt]', + " Toggle a feature flag.", + "", + "Run `appduct tools ` for a tool's full schema.", + "", + ].join("\n"), + ); + }); + + test("tools list output truncated by paging shows the Showing line with the given offset", () => { + const rendered = renderResult( + { + ok: true, + data: { + tools: [{ name: "echo", description: "Echoes input.", policy: "allow" }], + total: 5, + offset: 2, + limit: 1, + }, + }, + { command: "tools", flags: flags() }, + ).stdout ?? ""; + + expect(rendered).toContain( + "Showing 1 of 5 tools (offset 2). Narrow with --filter or page with --offset .", + ); + }); + + test("tools list output with a filter and no matches says so", () => { + const rendered = renderResult( + { ok: true, data: { tools: [], total: 0, filter: "nope" } }, + { command: "tools", flags: flags() }, + ).stdout ?? ""; + + expect(rendered).toContain('No tools match "nope".'); + }); + + test("tools list output with no tools and no filter keeps the original message", () => { + const rendered = renderResult( + { ok: true, data: { tools: [], total: 0 } }, + { command: "tools", flags: flags() }, + ).stdout ?? ""; + + expect(rendered).toContain("No tools registered."); + expect(rendered).not.toContain("Run `appduct tools `"); + }); + + test("a description longer than 120 characters is cut with a trailing ellipsis", () => { + const longDescription = `A. ${"x".repeat(130)}`; + const rendered = + renderResult( + { + ok: true, + data: { tools: [{ name: "verbose", description: longDescription, policy: "allow" }], total: 1 }, + }, + { command: "tools", flags: flags() }, + ).stdout ?? ""; + + const descriptionLine = rendered.split("\n").find((line) => line.startsWith(" A.")); + expect(descriptionLine).toBeDefined(); + expect(descriptionLine!.length).toBe(4 + 120 + 1); // 4-space indent + 120 chars + "…" + expect(descriptionLine!.endsWith("…")).toBe(true); + }); + + test("a tool with no input/output schema at all still gets a signature", () => { + const rendered = + renderResult( + { ok: true, data: { tools: [{ name: "ping", description: "Health check.", policy: "allow" }], total: 1 } }, + { command: "tools", flags: flags() }, + ).stdout ?? ""; + + expect(rendered).toContain(" ping(...)"); + }); + + test("--full listing renders full detail blocks and still shows the Showing line, with no trailing hint", () => { + const rendered = renderResult( + { + ok: true, + data: { + tools: [ + { + name: "echo", + description: "Echoes its input.", + input_schema: { type: "object", properties: { text: { type: "string" } } }, + policy: "allow", + }, + ], + total: 3, + offset: 0, + }, + }, + { command: "tools", flags: flags(), full: true }, + ).stdout ?? ""; + + expect(rendered).toContain("Tool: echo"); + expect(rendered).toMatch(/Signature\s+echo\(text\?: string\)/); + expect(rendered).toContain("Showing 1 of 3 tools (offset 0)."); + expect(rendered).not.toContain("Run `appduct tools `"); + }); + test("tools detail shows a declared timeout_ms, and no timeout line when the tool declares none", () => { const renderDetail = (timeoutMs?: number): string | undefined => renderResult( diff --git a/packages/appduct/src/__tests__/tool-invocation.integration.test.ts b/packages/appduct/src/__tests__/tool-invocation.integration.test.ts index 12af5e65..f5a7f9a9 100644 --- a/packages/appduct/src/__tests__/tool-invocation.integration.test.ts +++ b/packages/appduct/src/__tests__/tool-invocation.integration.test.ts @@ -279,10 +279,11 @@ describe("tools.list / tools.call: round trip", () => { }, ]); - const listed = (await rpcCall(daemon.paths.socketPath, "tools.list", { + const { tools: listed, total } = (await rpcCall(daemon.paths.socketPath, "tools.list", { selector: app.alias, - })) as Array<{ name: string; input_schema?: unknown }>; + })) as { tools: Array<{ name: string; input_schema?: unknown }>; total: number }; expect(listed).toHaveLength(1); + expect(total).toBe(1); expect(listed[0]!.name).toBe("echo"); expect(listed[0]!.input_schema).toEqual({ type: "object", properties: { text: { type: "string" } } }); @@ -316,7 +317,9 @@ describe("tools.list / tools.call: round trip", () => { app.socket.close(); await suspended; - const listed = (await rpcCall(daemon.paths.socketPath, "tools.list", { selector: app.alias })) as unknown[]; + const { tools: listed } = (await rpcCall(daemon.paths.socketPath, "tools.list", { + selector: app.alias, + })) as { tools: unknown[] }; expect(listed).toHaveLength(1); }); @@ -352,6 +355,81 @@ describe("tools.list / tools.call: round trip", () => { app.socket.close(); }); + + test("tools.list sorts by name, filters on name/description, reports total before paging, and slices with limit/offset", async () => { + const { daemon, port } = await startTestDaemon(); + const app = await claimApp(daemon, port); + + // Registered out of alphabetical order on purpose. + await snapshotTools(daemon, app, [ + { name: "zebra", description: "Stripes." }, + { name: "apple", description: "A red fruit." }, + { name: "mango", description: "A tropical fruit." }, + { name: "kiwi", description: "A furry fruit." }, + ]); + + const listAll = (await rpcCall(daemon.paths.socketPath, "tools.list", { + selector: app.alias, + })) as { tools: Array<{ name: string }>; total: number }; + expect(listAll.tools.map((tool) => tool.name)).toEqual(["apple", "kiwi", "mango", "zebra"]); + expect(listAll.total).toBe(4); + + // Filter matches on description, not just name, case-insensitively. + const filtered = (await rpcCall(daemon.paths.socketPath, "tools.list", { + selector: app.alias, + filter: "FRUIT", + })) as { tools: Array<{ name: string }>; total: number }; + expect(filtered.tools.map((tool) => tool.name)).toEqual(["apple", "kiwi", "mango"]); + expect(filtered.total).toBe(3); + + // total reflects the filtered count *before* paging is applied. + const paged = (await rpcCall(daemon.paths.socketPath, "tools.list", { + selector: app.alias, + filter: "fruit", + limit: 1, + offset: 1, + })) as { tools: Array<{ name: string }>; total: number }; + expect(paged.tools.map((tool) => tool.name)).toEqual(["kiwi"]); + expect(paged.total).toBe(3); + + // offset with no limit returns everything from offset on. + const fromOffset = (await rpcCall(daemon.paths.socketPath, "tools.list", { + selector: app.alias, + offset: 2, + })) as { tools: Array<{ name: string }>; total: number }; + expect(fromOffset.tools.map((tool) => tool.name)).toEqual(["mango", "zebra"]); + expect(fromOffset.total).toBe(4); + + app.socket.close(); + }); + + test("tools.list rejects a bad limit/offset/filter as invalid_request", async () => { + const { daemon, port } = await startTestDaemon(); + const app = await claimApp(daemon, port); + await snapshotTools(daemon, app, [{ name: "echo" }]); + + await expect( + rpcCall(daemon.paths.socketPath, "tools.list", { selector: app.alias, limit: 0 }), + ).rejects.toMatchObject({ data: { type: "invalid_request" } }); + + await expect( + rpcCall(daemon.paths.socketPath, "tools.list", { selector: app.alias, limit: -1 }), + ).rejects.toMatchObject({ data: { type: "invalid_request" } }); + + await expect( + rpcCall(daemon.paths.socketPath, "tools.list", { selector: app.alias, offset: -1 }), + ).rejects.toMatchObject({ data: { type: "invalid_request" } }); + + await expect( + rpcCall(daemon.paths.socketPath, "tools.list", { selector: app.alias, filter: 42 }), + ).rejects.toMatchObject({ data: { type: "invalid_request" } }); + + await expect( + rpcCall(daemon.paths.socketPath, "tools.list", { selector: app.alias, filter: "x".repeat(257) }), + ).rejects.toMatchObject({ data: { type: "invalid_request" } }); + + app.socket.close(); + }); }); describe("tools.call: error type preservation", () => { diff --git a/packages/appduct/src/cli/create-cli.ts b/packages/appduct/src/cli/create-cli.ts index bb25523a..a5017d3c 100644 --- a/packages/appduct/src/cli/create-cli.ts +++ b/packages/appduct/src/cli/create-cli.ts @@ -58,7 +58,10 @@ export const createCli = () => { cli .command("tools [selector] [name]", "List a session's tools, or show one tool's full schema.") - .option("--full", "Render full schemas/annotations for every listed tool."); + .option("--full", "Render full schemas/annotations for every listed tool.") + .option("--filter ", "Only tools whose name or description contains this text (case-insensitive).") + .option("--limit ", "Show at most n tools.") + .option("--offset ", "Skip the first n tools of the sorted list."); cli .command("invoke [selector] [tool]", "Call a tool on a session.") diff --git a/packages/appduct/src/cli/result-types.ts b/packages/appduct/src/cli/result-types.ts index 2af9fdc9..35a73fee 100644 --- a/packages/appduct/src/cli/result-types.ts +++ b/packages/appduct/src/cli/result-types.ts @@ -11,6 +11,7 @@ import type { EventNotification, SessionSummary, ToolDescriptor, + ToolsListResult, } from "@appduct/shared"; /** @@ -141,8 +142,18 @@ export type LinkCommandData = { /** `appduct ls`: `sessions.list` passthrough, verbatim (ARCHITECTURE.md §10: "--json passthrough"). */ export type LsCommandData = SessionSummary[]; -/** `appduct tools`: the full list, or a single descriptor when a tool name resolved to a detail lookup. */ -export type ToolsCommandData = ToolDescriptor[] | ToolDescriptor; +/** `appduct tools`'s listing form: the daemon's `tools.list` result, plus the `--filter`/`--limit`/ + * `--offset` inputs that produced it (only the ones actually given — echoed so `--json` and the + * human renderer's "Showing n of total" line can report what was asked for without threading the + * CLI options through separately). */ +export type ToolsListing = ToolsListResult & { + offset?: number; + limit?: number; + filter?: string; +}; + +/** `appduct tools`: a listing, or a single descriptor when a tool name resolved to a detail lookup. */ +export type ToolsCommandData = ToolsListing | ToolDescriptor; /** `appduct invoke`: the tool's raw result payload, printed as-is. */ export type InvokeCommandData = unknown; diff --git a/packages/appduct/src/cli/routes/tools.ts b/packages/appduct/src/cli/routes/tools.ts index 0166137c..572737c7 100644 --- a/packages/appduct/src/cli/routes/tools.ts +++ b/packages/appduct/src/cli/routes/tools.ts @@ -3,7 +3,11 @@ import type { Route } from "../router.js"; import { handleToolsCommand } from "../../commands/tools.js"; -import { splitOptionalSelectorAndTarget } from "../command-options.js"; +import { + parseNonNegativeIntegerOption, + parsePositiveIntegerOption, + splitOptionalSelectorAndTarget, +} from "../command-options.js"; import { commandName } from "../router.js"; import { executeCommand } from "../runner.js"; import { guarded } from "../version-guard.js"; @@ -14,11 +18,17 @@ export const route: Route = async (context) => { context.args, "tools [selector] [name]", ); + const limit = parsePositiveIntegerOption(options.limit, "--limit"); + const offset = parseNonNegativeIntegerOption(options.offset, "--offset"); + const filter = typeof options.filter === "string" ? options.filter : undefined; return executeCommand( commandName(context), guarded(context)(() => - handleToolsCommand({ selector: selector ?? selectorOrTarget, name: target }, { stateDir }), + handleToolsCommand( + { selector: selector ?? selectorOrTarget, name: target, filter, limit, offset }, + { stateDir }, + ), ), context.env, { full: Boolean(options.full) }, diff --git a/packages/appduct/src/client/app-client.ts b/packages/appduct/src/client/app-client.ts index 9aab50e2..717a18d4 100644 --- a/packages/appduct/src/client/app-client.ts +++ b/packages/appduct/src/client/app-client.ts @@ -166,7 +166,10 @@ export const makeAppClient = (stream: DaemonStream, sessionId: tools: async (): Promise => { try { - return await stream.call(RPC_METHODS.toolsList, { selector: sessionId }); + // No `filter`/`limit`/`offset`: this client's public `tools()` contract is "every tool on + // this session", unchanged by `tools.list`'s daemon-side paging (added for the CLI). + const { tools } = await stream.call(RPC_METHODS.toolsList, { selector: sessionId }); + return tools; } catch (error) { throw toAppductError(error); } diff --git a/packages/appduct/src/commands/tools.ts b/packages/appduct/src/commands/tools.ts index 7ab1067a..39dc5460 100644 --- a/packages/appduct/src/commands/tools.ts +++ b/packages/appduct/src/commands/tools.ts @@ -1,6 +1,7 @@ /** - * `appduct tools` (ARCHITECTURE.md §10): `tools [selector] [--full]` lists tools for a session; - * `tools [selector] ` shows one tool's full schema/annotations. + * `appduct tools` (ARCHITECTURE.md §10): `tools [selector] [--full] [--filter ] [--limit + * ] [--offset ]` lists tools for a session; `tools [selector] ` shows one tool's full + * schema/annotations. * * The command table gives both forms a leading optional `[selector]`, which makes a single * positional argument inherently ambiguous (is it the selector, or the tool name in `tools ` @@ -8,17 +9,27 @@ * first try the arg as a tool name in the implicit-selector session's registry; if no such tool * exists there (or the implicit selector doesn't resolve, e.g. `ambiguous_session`), fall back to * treating it as a selector and list that session's tools instead. + * + * `--filter`/`--limit`/`--offset` only ever reach the daemon on a *listing* request: the detail + * path (an explicit ``, or the ambiguous single-arg probe above) always asks for the whole, + * unpaged registry, so a name lookup can never miss a tool that paging would have left off a page. */ import { RPC_METHODS, type ToolDescriptor, type ToolsListResult } from "@appduct/shared"; -import type { CliResult, ToolsCommandData } from "../cli/result-types.js"; +import type { CliResult, ToolsCommandData, ToolsListing } from "../cli/result-types.js"; import { usageError } from "../errors.js"; import { callDaemon, DaemonRpcError, type SpawnFn } from "../rpc/client.js"; export type ToolsCommandOptions = { selector?: string; name?: string; + /** Case-insensitive substring match against name and description. Listing only. */ + filter?: string; + /** Page size. Listing only. */ + limit?: number; + /** Zero-based start index into the sorted, filtered list. Listing only. */ + offset?: number; }; export type ToolsCommandContext = { @@ -26,13 +37,16 @@ export type ToolsCommandContext = { spawn?: SpawnFn; }; +type ListParams = { filter?: string; limit?: number; offset?: number }; + const listTools = ( selector: string | undefined, context: ToolsCommandContext, + params: ListParams = {}, ): Promise => { return callDaemon( RPC_METHODS.toolsList, - { selector }, + { selector, ...params }, { stateDir: context.stateDir, spawn: context.spawn }, ); }; @@ -41,18 +55,40 @@ const findTool = (tools: ToolDescriptor[], name: string): ToolDescriptor | undef return tools.find((tool) => tool.name === name); }; +const hasPagingOptions = (options: ToolsCommandOptions): boolean => { + return options.filter !== undefined || options.limit !== undefined || options.offset !== undefined; +}; + +/** Echoes back only the paging/filter inputs actually given, alongside the daemon's result — the + * human renderer's "Showing n of total" line and `--json` consumers both read this off `data` + * rather than needing the CLI options threaded to them separately. */ +const toListing = (result: ToolsListResult, params: ListParams): ToolsListing => { + return { + ...result, + ...(params.filter !== undefined ? { filter: params.filter } : {}), + ...(params.limit !== undefined ? { limit: params.limit } : {}), + ...(params.offset !== undefined ? { offset: params.offset } : {}), + }; +}; + export const handleToolsCommand = async ( options: ToolsCommandOptions, context: ToolsCommandContext, ): Promise> => { + if (options.name !== undefined && hasPagingOptions(options)) { + throw usageError( + '"--filter", "--limit", and "--offset" only apply to a tools listing, not a single tool lookup.', + ); + } + if (options.selector !== undefined && options.name !== undefined) { - const tools = await listTools(options.selector, context); - const tool = findTool(tools, options.name); + const result = await listTools(options.selector, context); + const tool = findTool(result.tools, options.name); if (!tool) { throw usageError( `Tool "${options.name}" is not registered on session "${options.selector}".`, - { available: tools.map((entry) => entry.name) }, + { available: result.tools.map((entry) => entry.name) }, ); } @@ -72,7 +108,7 @@ export const handleToolsCommand = async ( } if (implicitTools) { - const tool = findTool(implicitTools, options.selector); + const tool = findTool(implicitTools.tools, options.selector); if (tool) { return { ok: true, data: tool }; @@ -81,8 +117,10 @@ export const handleToolsCommand = async ( // Not a tool name on the implicit session (or there is no implicit session): treat the arg as // a selector and list that session's tools instead. - return { ok: true, data: await listTools(options.selector, context) }; + const result = await listTools(options.selector, context, options); + return { ok: true, data: toListing(result, options) }; } - return { ok: true, data: await listTools(undefined, context) }; + const result = await listTools(undefined, context, options); + return { ok: true, data: toListing(result, options) }; }; diff --git a/packages/appduct/src/daemon/daemon.ts b/packages/appduct/src/daemon/daemon.ts index 7bc0e13d..6b2a3b53 100644 --- a/packages/appduct/src/daemon/daemon.ts +++ b/packages/appduct/src/daemon/daemon.ts @@ -32,6 +32,8 @@ import { type ToolsCallResult, type ToolsCancelParams, type ToolsCancelResult, + type ToolsListEntry, + type ToolsListParams, type ToolsListResult, } from "@appduct/shared"; @@ -151,6 +153,43 @@ const asSelectorParams = (params: unknown): { selector?: string } => { return { selector }; }; +/** `tools.list`'s `filter` string cap (ARCHITECTURE.md §5) — generous for a name/description + * substring search, small enough that a malicious/buggy caller can't use it to bloat a request. */ +const MAX_TOOLS_FILTER_LENGTH = 256; + +const asToolsListParams = (params: unknown): ToolsListParams => { + const { selector } = asSelectorParams(params); + const record = asRecordParams(params); + + const filter = record.filter; + + if (filter !== undefined && (typeof filter !== "string" || filter.length > MAX_TOOLS_FILTER_LENGTH)) { + throw new RpcApplicationError( + "invalid_request", + `"filter" must be a string of at most ${MAX_TOOLS_FILTER_LENGTH} characters.`, + ); + } + + const limit = record.limit; + + if (limit !== undefined && (typeof limit !== "number" || !Number.isInteger(limit) || limit <= 0)) { + throw new RpcApplicationError("invalid_request", '"limit" must be a positive integer.'); + } + + const offset = record.offset; + + if (offset !== undefined && (typeof offset !== "number" || !Number.isInteger(offset) || offset < 0)) { + throw new RpcApplicationError("invalid_request", '"offset" must be a non-negative integer.'); + } + + return { + selector, + filter: filter as string | undefined, + limit: limit as number | undefined, + offset: offset as number | undefined, + }; +}; + const asRecordParams = (params: unknown): Record => { if (params === undefined || params === null) { return {}; @@ -540,7 +579,7 @@ export const startDaemon = async (options: DaemonOptions): Promise { - const { selector } = asSelectorParams(params); + const { selector, filter, limit, offset } = asToolsListParams(params); // ARCHITECTURE.md §5: tools.list works for ACTIVE and SUSPENDED sessions alike (the // retained registry survives suspend); only tools.call requires ACTIVE. const resolved = activeSessionManager.resolveForTools(selector); @@ -548,10 +587,32 @@ export const startDaemon = async (options: DaemonOptions): Promise ({ + const entries: ToolsListEntry[] = resolved.registry.list().map((descriptor) => ({ ...descriptor, policy: evaluatePolicy(descriptor, { alias: resolved.alias }, config.policy), })); + + // Sorted by name with a plain code-point comparison — not `localeCompare`, which would + // make listing order depend on the daemon process's locale — so an agent narrowing with + // `--filter`/paging with `--limit`/`--offset` sees the same order every time. + entries.sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)); + + const lowerFilter = filter?.toLowerCase(); + const matching = lowerFilter + ? entries.filter( + (entry) => + entry.name.toLowerCase().includes(lowerFilter) || + entry.description.toLowerCase().includes(lowerFilter), + ) + : entries; + + const total = matching.length; + const page = + offset === undefined && limit === undefined + ? matching + : matching.slice(offset ?? 0, limit === undefined ? undefined : (offset ?? 0) + limit); + + return { tools: page, total }; }, // This handler is the single seam every `tools.call` passes through: policy (ARCHITECTURE.md // §12) is evaluated once the target tool descriptor is known, and one audit record is diff --git a/packages/appduct/src/mcp/daemon-tools.ts b/packages/appduct/src/mcp/daemon-tools.ts index cee4f3c0..a0b7c41b 100644 --- a/packages/appduct/src/mcp/daemon-tools.ts +++ b/packages/appduct/src/mcp/daemon-tools.ts @@ -26,10 +26,8 @@ export const fetchEffectiveTools = async (call: DaemonCall): Promise(RPC_METHODS.toolsList, { selector: session.alias }), - ); + const { tools } = await call(RPC_METHODS.toolsList, { selector: session.alias }); + toolsByAlias.set(session.alias, tools); } catch (error) { // The session can transition (revoke/expire) between `sessions.list` and this per-session // `tools.list` call; treat it as having no tools rather than failing the whole listing. diff --git a/packages/appduct/src/output.ts b/packages/appduct/src/output.ts index 060aab9d..80ed2571 100644 --- a/packages/appduct/src/output.ts +++ b/packages/appduct/src/output.ts @@ -1,5 +1,11 @@ import pc from "picocolors"; -import { formatAgentWebSocketUrl, type EventNotification, type SessionSummary, type ToolDescriptor } from "@appduct/shared"; +import { + formatAgentWebSocketUrl, + renderToolSignature, + type EventNotification, + type SessionSummary, + type ToolDescriptor, +} from "@appduct/shared"; import type { CliError, @@ -17,6 +23,7 @@ import type { LsCommandData, RevokeCommandData, ToolsCommandData, + ToolsListing, } from "./cli/result-types.js"; import { formatJson, type GlobalFlags } from "./cli/global-flags.js"; import { renderQrToTerminal } from "./qr-terminal.js"; @@ -141,17 +148,61 @@ const renderLsData = (colors: ColorPalette, data: LsCommandData, now: Date): str ]; }; -const renderToolSummaryTable = (tools: ToolDescriptor[]): string[] => { - if (tools.length === 0) { - return ["Tools", " No tools registered."]; +/** `renderToolsData` distinguishes the listing form of `ToolsCommandData` (`ToolsListing`, which + * carries `tools`/`total`) from the bare single-tool detail form purely by shape — a `ToolDescriptor` + * never has a `tools` array of its own, so this never misclassifies either one. */ +const isToolsListing = (data: ToolsCommandData): data is ToolsListing => { + return typeof data === "object" && data !== null && Array.isArray((data as ToolsListing).tools); +}; + +/** First line of a tool's description, trimmed and capped — the summary listing shows only this, + * not the full (possibly multi-line, up to `MAX_TOOL_DESCRIPTION_LENGTH`) text; `tools `/ + * `--full` still show it in full. */ +const MAX_LISTED_DESCRIPTION_LENGTH = 120; + +const summarizeDescription = (description: string): string => { + const firstLine = (description.split("\n")[0] ?? "").trim(); + + return firstLine.length > MAX_LISTED_DESCRIPTION_LENGTH + ? `${firstLine.slice(0, MAX_LISTED_DESCRIPTION_LENGTH)}…` + : firstLine; +}; + +/** "No tools registered"/"No tools match" for an empty listing (compact or `--full` — both share + * this line, only the header differs). */ +const renderEmptyToolsLine = (filter: string | undefined): string => { + return filter === undefined ? " No tools registered." : ` No tools match "${filter}".`; +}; + +/** The `Showing n of total tools (offset o). Narrow with --filter or page with --offset + * .` line — only when the page actually left tools out, so a listing that already shows + * everything (including a filtered one with no more matches) stays quiet. */ +const renderTruncationLine = (data: ToolsListing): string[] => { + if (data.tools.length >= data.total) { + return []; } - const nameWidth = Math.max(...tools.map((tool) => tool.name.length), "Name".length); + return [ + "", + `Showing ${data.tools.length} of ${data.total} tools (offset ${data.offset ?? 0}). ` + + "Narrow with --filter or page with --offset .", + ]; +}; + +const renderToolSummaryTable = (colors: ColorPalette, data: ToolsListing): string[] => { + if (data.tools.length === 0) { + return [colors.green("Tools"), renderEmptyToolsLine(data.filter)]; + } return [ - "Tools", - ` ${"Name".padEnd(nameWidth)} Description`, - ...tools.map((tool) => ` ${tool.name.padEnd(nameWidth)} ${tool.description}`), + colors.green("Tools"), + ...data.tools.flatMap((tool) => { + const tag = tool.policy === "allow" ? "" : ` [${tool.policy}]`; + return [` ${renderToolSignature(tool)}${tag}`, ` ${summarizeDescription(tool.description)}`]; + }), + ...renderTruncationLine(data), + "", + "Run `appduct tools ` for a tool's full schema.", ]; }; @@ -159,6 +210,7 @@ const renderToolDetail = (colors: ColorPalette, tool: ToolDescriptor, flags: Glo return renderFields( colors.green(`Tool: ${tool.name}`), [ + ["Signature", renderToolSignature(tool)], ["Description", tool.description], ["Input schema", tool.input_schema], ["Output schema", tool.output_schema], @@ -172,26 +224,31 @@ const renderToolDetail = (colors: ColorPalette, tool: ToolDescriptor, flags: Glo ); }; +const renderToolsFullListing = (colors: ColorPalette, data: ToolsListing, flags: GlobalFlags): string[] => { + if (data.tools.length === 0) { + return [colors.green("Tools"), renderEmptyToolsLine(data.filter)]; + } + + return [ + ...data.tools.flatMap((tool, index) => [ + ...(index > 0 ? [""] : []), + ...renderToolDetail(colors, tool, flags), + ]), + ...renderTruncationLine(data), + ]; +}; + const renderToolsData = ( colors: ColorPalette, data: ToolsCommandData, flags: GlobalFlags, full?: boolean, ): string[] => { - if (!Array.isArray(data)) { + if (!isToolsListing(data)) { return renderToolDetail(colors, data, flags); } - if (full) { - return data.length === 0 - ? ["Tools", " No tools registered."] - : data.flatMap((tool, index) => [ - ...(index > 0 ? [""] : []), - ...renderToolDetail(colors, tool, flags), - ]); - } - - return [colors.green("Tools"), ...renderToolSummaryTable(data).slice(1)]; + return full ? renderToolsFullListing(colors, data, flags) : renderToolSummaryTable(colors, data); }; const renderInvokeData = (colors: ColorPalette, data: InvokeCommandData, flags: GlobalFlags): string[] => { diff --git a/packages/shared/src/__tests__/tool-signature.test.ts b/packages/shared/src/__tests__/tool-signature.test.ts new file mode 100644 index 00000000..a5feeacd --- /dev/null +++ b/packages/shared/src/__tests__/tool-signature.test.ts @@ -0,0 +1,345 @@ +import { describe, expect, test } from "vitest"; + +import { renderToolSignature } from "../domains/tool-signature.js"; + +type Case = [label: string, descriptor: Parameters[0], expected: string]; + +const cases: Case[] = [ + ["no input/output schema at all", { name: "ping" }, "ping(...)"], + [ + "input_schema not rooted at type: object", + { name: "echo", input_schema: { type: "string" } }, + "echo(...)", + ], + [ + "object schema, no properties key, additionalProperties: false -> no-args", + { name: "ping", input_schema: { type: "object", additionalProperties: false } }, + "ping()", + ], + [ + "object schema, no properties key, additionalProperties not false -> unresolved", + { name: "ping", input_schema: { type: "object" } }, + "ping(...)", + ], + [ + "object schema, empty properties object -> explicit no-args", + { name: "ping", input_schema: { type: "object", properties: {} } }, + "ping()", + ], + [ + "required and optional params, declared order preserved", + { + name: "seed_cart", + input_schema: { + type: "object", + properties: { items: { type: "integer" }, sku: { type: "string" } }, + required: ["items"], + }, + }, + "seed_cart(items: int, sku?: string)", + ], + [ + "optional param with a short default", + { + name: "seed_cart", + input_schema: { + type: "object", + properties: { clear: { type: "boolean", default: true } }, + }, + }, + "seed_cart(clear?: bool = true)", + ], + [ + "default omitted when its JSON rendering exceeds 20 characters", + { + name: "configure", + input_schema: { + type: "object", + properties: { tags: { type: "array", items: { type: "string" }, default: ["a", "b", "c", "d", "e"] } }, + }, + }, + "configure(tags?: string[])", + ], + [ + "primitive type names: string/number/boolean/integer/null", + { + name: "types", + input_schema: { + type: "object", + properties: { + a: { type: "string" }, + b: { type: "number" }, + c: { type: "boolean" }, + d: { type: "integer" }, + e: { type: "null" }, + }, + required: ["a", "b", "c", "d", "e"], + }, + }, + "types(a: string, b: number, c: bool, d: int, e: null)", + ], + [ + "type given as an array joins members with ' | '", + { + name: "set_value", + input_schema: { + type: "object", + properties: { value: { type: ["string", "null"] } }, + required: ["value"], + }, + }, + "set_value(value: string | null)", + ], + [ + "enum renders JSON values joined with ' | '", + { + name: "set_flag", + input_schema: { + type: "object", + properties: { name: { enum: ["dark_mode", "new_checkout"] } }, + required: ["name"], + }, + }, + 'set_flag(name: "dark_mode" | "new_checkout")', + ], + [ + "enum longer than 5 values truncates with ' | …'", + { + name: "pick", + input_schema: { + type: "object", + properties: { n: { enum: [1, 2, 3, 4, 5, 6] } }, + required: ["n"], + }, + }, + "pick(n: 1 | 2 | 3 | 4 | 5 | …)", + ], + [ + "enum with exactly 5 values shows no ellipsis", + { + name: "pick", + input_schema: { + type: "object", + properties: { n: { enum: [1, 2, 3, 4, 5] } }, + required: ["n"], + }, + }, + "pick(n: 1 | 2 | 3 | 4 | 5)", + ], + [ + "const renders its JSON value", + { + name: "set_mode", + input_schema: { + type: "object", + properties: { mode: { const: "strict" } }, + required: ["mode"], + }, + }, + 'set_mode(mode: "strict")', + ], + [ + "array of primitives", + { + name: "tag", + input_schema: { + type: "object", + properties: { labels: { type: "array", items: { type: "string" } } }, + }, + }, + "tag(labels?: string[])", + ], + [ + "array with missing items -> unknown[]", + { + name: "tag", + input_schema: { type: "object", properties: { labels: { type: "array" } } }, + }, + "tag(labels?: unknown[])", + ], + [ + "array with a non-object-schema items (hostile) -> unknown[]", + { + name: "tag", + input_schema: { type: "object", properties: { labels: { type: "array", items: 3 } } }, + }, + "tag(labels?: unknown[])", + ], + [ + "object property expands one level", + { + name: "set_address", + input_schema: { + type: "object", + properties: { + address: { + type: "object", + properties: { city: { type: "string" }, zip: { type: "string" } }, + required: ["city"], + }, + }, + required: ["address"], + }, + }, + "set_address(address: { city: string, zip?: string })", + ], + [ + "object nested two levels deep renders the inner object as {...}", + { + name: "set_address", + input_schema: { + type: "object", + properties: { + address: { + type: "object", + properties: { + city: { type: "string" }, + geo: { type: "object", properties: { lat: { type: "number" } } }, + }, + required: ["city"], + }, + }, + required: ["address"], + }, + }, + "set_address(address: { city: string, geo?: {...} })", + ], + [ + "anyOf/oneOf/allOf/not/$ref all render as ...", + { + name: "poly", + input_schema: { + type: "object", + properties: { + a: { anyOf: [{ type: "string" }, { type: "number" }] }, + b: { oneOf: [{ type: "string" }] }, + c: { allOf: [{ type: "string" }] }, + d: { not: { type: "string" } }, + e: { $ref: "#/definitions/Thing" }, + }, + }, + }, + "poly(a?: ..., b?: ..., c?: ..., d?: ..., e?: ...)", + ], + [ + "output schema omitted -> no arrow", + { name: "ping", input_schema: { type: "object", properties: {} } }, + "ping()", + ], + [ + "output schema object root inlines one level", + { + name: "seed_cart", + input_schema: { type: "object", properties: {} }, + output_schema: { + type: "object", + properties: { added: { type: "integer" }, cartId: { type: "string" } }, + required: ["added", "cartId"], + }, + }, + "seed_cart() -> { added: int, cartId: string }", + ], + [ + "output schema non-object root renders as its own type", + { + name: "list_tags", + input_schema: { type: "object", properties: {} }, + output_schema: { type: "array", items: { type: "string" } }, + }, + "list_tags() -> string[]", + ], + [ + "output schema unrecognised (composed) -> -> ...", + { + name: "weird", + input_schema: { type: "object", properties: {} }, + output_schema: { anyOf: [{ type: "string" }] }, + }, + "weird() -> ...", + ], + [ + "output schema that is not an object at all -> -> ...", + { + name: "weird", + input_schema: { type: "object", properties: {} }, + output_schema: null as unknown as Record, + }, + "weird() -> ...", + ], + + // --- hostile inputs: schema internals are never validated elsewhere, so nothing here may throw --- + [ + "hostile: properties: null on the root falls back to the additionalProperties rule", + { name: "ping", input_schema: { type: "object", properties: null } }, + "ping(...)", + ], + [ + "hostile: properties: null with additionalProperties: false -> no-args", + { name: "ping", input_schema: { type: "object", properties: null, additionalProperties: false } }, + "ping()", + ], + [ + "hostile: required is a string, not an array -> nothing is treated as required", + { + name: "greet", + input_schema: { + type: "object", + properties: { name: { type: "string" } }, + required: "name", + }, + }, + "greet(name?: string)", + ], + [ + "hostile: enum is an object, not an array -> falls through to type rendering", + { + name: "pick", + input_schema: { + type: "object", + properties: { n: { type: "string", enum: { not: "an array" } } }, + }, + }, + "pick(n?: string)", + ], + [ + "hostile: input_schema itself is not an object", + { name: "ping", input_schema: "not a schema" as unknown as Record }, + "ping(...)", + ], + [ + "hostile: a property schema that is not an object renders as ...", + { name: "ping", input_schema: { type: "object", properties: { a: 3 } } }, + "ping(a?: ...)", + ], + [ + "hostile: deep nesting beyond one level never expands, however many levels down", + { + name: "deep", + input_schema: { + type: "object", + properties: { + a: { + type: "object", + properties: { + b: { + type: "object", + properties: { c: { type: "object", properties: { d: { type: "string" } } } }, + }, + }, + }, + }, + }, + }, + "deep(a?: { b?: {...} })", + ], +]; + +describe("renderToolSignature", () => { + test.each(cases)("%s", (_label, descriptor, expected) => { + expect(renderToolSignature(descriptor)).toBe(expected); + }); + + test("never throws on a completely empty object", () => { + expect(() => renderToolSignature({ name: "x" })).not.toThrow(); + }); +}); diff --git a/packages/shared/src/domains/rpc.ts b/packages/shared/src/domains/rpc.ts index d4afc155..51d132f4 100644 --- a/packages/shared/src/domains/rpc.ts +++ b/packages/shared/src/domains/rpc.ts @@ -139,7 +139,14 @@ export type SessionsRevokeResult = { ok: true }; // --- tools.list / tools.call --- -export type ToolsListParams = SessionSelectorParams; +export type ToolsListParams = SessionSelectorParams & { + /** Case-insensitive substring match against name and description. */ + filter?: string; + /** Page size; omitted means everything from `offset` on. */ + limit?: number; + /** Zero-based start index into the sorted, filtered list. */ + offset?: number; +}; /** A `tools.list` entry: the tool's descriptor plus the policy decision (ARCHITECTURE.md §12) * that would apply to it right now — resolved daemon-side (it needs `session.alias` and @@ -149,7 +156,17 @@ export type ToolsListEntry = ToolDescriptor & { policy: EffectivePolicyDecision; }; -export type ToolsListResult = ToolsListEntry[]; +/** + * `tools.list`'s result: the registry sorted by `name` (plain code-point order, so it is + * deterministic across locales), `filter`ed, then paged with `limit`/`offset` — `total` is the + * count *after* filtering but *before* paging, so a caller (the CLI) can say how many tools were + * left out of the page it got back. + */ +export type ToolsListResult = { + tools: ToolsListEntry[]; + /** Matching tools before `limit`/`offset` were applied. */ + total: number; +}; export type ToolsCallParams = SessionSelectorParams & { name: string; diff --git a/packages/shared/src/domains/tool-signature.ts b/packages/shared/src/domains/tool-signature.ts new file mode 100644 index 00000000..a06f4d7c --- /dev/null +++ b/packages/shared/src/domains/tool-signature.ts @@ -0,0 +1,221 @@ +/** + * Renders a `ToolDescriptor` into a one-line call signature, e.g. + * `seed_cart(items: int, sku?: string, clear?: bool = true) -> { added: int, cartId: string }`. + * + * This exists so `appduct tools` can list hundreds of tools cheaply for an agent to read: a + * signature plus the description's first line says far more per line than a bare name, without + * the cost of printing every tool's full schema (`--full`/`tools ` remain the source of + * truth for that). + * + * A tool's `input_schema`/`output_schema` are draft-2020-12 JSON Schema fragments whose internals + * are never validated anywhere in this codebase (`tool-descriptor.ts`'s `isToolDescriptor` only + * checks they are JSON objects) — an app can send anything shaped like an object. Every function + * here is therefore purely defensive: an unrecognised or malformed fragment renders as `...` + * (or `{...}`/`(...)`/`unknown[]` in context) rather than throwing. + */ + +import type { ToolDescriptor, ToolSchemaDescriptor } from "./tool-descriptor.js"; + +type JsonSchema = Record; + +const isSchemaObject = (value: unknown): value is JsonSchema => { + return typeof value === "object" && value !== null && !Array.isArray(value); +}; + +/** JSON Schema keywords that describe a composed/indirect schema this renderer does not attempt + * to resolve — always `...`, at any depth. */ +const isUnresolvable = (schema: JsonSchema): boolean => { + return ( + schema.anyOf !== undefined || + schema.oneOf !== undefined || + schema.allOf !== undefined || + schema.not !== undefined || + schema.$ref !== undefined + ); +}; + +const MAX_ENUM_VALUES = 5; +/** `default` only appends to a param entry when its JSON rendering stays this short — a longer + * default belongs in the full schema (`tools `), not a one-line signature. */ +const MAX_DEFAULT_LENGTH = 20; + +const renderEnumValues = (values: unknown[]): string => { + const shown = values.slice(0, MAX_ENUM_VALUES).map((value) => JSON.stringify(value)); + const suffix = values.length > MAX_ENUM_VALUES ? " | …" : ""; + return `${shown.join(" | ")}${suffix}`; +}; + +const primitiveTypeName = (type: string): string | undefined => { + switch (type) { + case "string": + return "string"; + case "number": + return "number"; + case "boolean": + return "bool"; + case "integer": + return "int"; + case "null": + return "null"; + default: + return undefined; + } +}; + +/** + * One property list, shared by the top-level params `(...)` group and a one-level-deep nested + * object's `{...}` group: `undefined` means the schema declares no usable `properties` at all + * (missing, or not itself a JSON object) — the caller decides what that renders as. A present but + * empty `properties` object (an intentional "no fields") renders as `[]`, distinct from that. + */ +const propertyEntries = (schema: JsonSchema, expandObject: boolean): string[] | undefined => { + const properties = schema.properties; + + if (!isSchemaObject(properties)) { + return undefined; + } + + const requiredRaw = schema.required; + const required = Array.isArray(requiredRaw) + ? requiredRaw.filter((entry): entry is string => typeof entry === "string") + : []; + + return Object.keys(properties).map((name) => + renderParamEntry(name, properties[name], required.includes(name), expandObject), + ); +}; + +/** Renders one `(...)`/`{...}` group: an explicit empty property list is `emptyToken`; a schema + * with no `properties` key at all falls back to `emptyToken` when `additionalProperties` is + * `false` (nothing to show, so same as truly empty) or `unresolvedToken` otherwise (there may be + * fields, this renderer just cannot see them). */ +const renderPropertyGroup = ( + schema: JsonSchema, + expandObject: boolean, + emptyToken: string, + wrap: (joined: string) => string, + unresolvedToken: string, +): string => { + const entries = propertyEntries(schema, expandObject); + + if (entries === undefined) { + return schema.additionalProperties === false ? emptyToken : unresolvedToken; + } + + return entries.length === 0 ? emptyToken : wrap(entries.join(", ")); +}; + +/** A `type: "object"` schema encountered while rendering a type (a property's own type, an array's + * `items`, ...) — expanded one level via {@link renderPropertyGroup} when `expandObject` allows + * it, else the deliberately shallow `{...}`. */ +const renderObjectType = (schema: JsonSchema): string => { + return renderPropertyGroup(schema, false, "{}", (joined) => `{ ${joined} }`, "{...}"); +}; + +const renderScalarType = (typeValue: unknown, schema: JsonSchema, expandObject: boolean): string => { + if (typeValue === "array") { + const items = schema.items; + return isSchemaObject(items) ? `${renderType(items, expandObject)}[]` : "unknown[]"; + } + + if (typeValue === "object") { + return expandObject ? renderObjectType(schema) : "{...}"; + } + + if (typeof typeValue === "string") { + const primitive = primitiveTypeName(typeValue); + + if (primitive) { + return primitive; + } + } + + return "..."; +}; + +/** + * The single type renderer every other function here goes through: `enum`/`const` win over + * `type`, a `type` array fans out and joins with ` | `, and `expandObject` gates one level of + * `type: "object"` expansion (`renderObjectType` always passes `false` back down, so nothing + * expands twice). + */ +const renderType = (schema: unknown, expandObject: boolean): string => { + if (!isSchemaObject(schema)) { + return "..."; + } + + if (isUnresolvable(schema)) { + return "..."; + } + + if ("const" in schema) { + const rendered = JSON.stringify(schema.const); + return typeof rendered === "string" ? rendered : "..."; + } + + if (Array.isArray(schema.enum)) { + return renderEnumValues(schema.enum); + } + + const type = schema.type; + + if (Array.isArray(type)) { + return type.map((entry) => renderScalarType(entry, schema, expandObject)).join(" | "); + } + + return renderScalarType(type, schema, expandObject); +}; + +const renderParamEntry = ( + name: string, + propSchema: unknown, + required: boolean, + expandObject: boolean, +): string => { + let entry = `${name}${required ? "" : "?"}: ${renderType(propSchema, expandObject)}`; + + if (isSchemaObject(propSchema) && "default" in propSchema) { + const json = JSON.stringify(propSchema.default); + + if (typeof json === "string" && json.length <= MAX_DEFAULT_LENGTH) { + entry += ` = ${json}`; + } + } + + return entry; +}; + +/** The `(...)` params group. Unlike {@link renderObjectType}, an `input_schema` not rooted at + * `type: "object"` (or absent entirely) is always `(...)` — MCP requires an object-rooted input + * schema (`tool-descriptor.ts`'s `isObjectRootedSchema`), so anything else means this renderer + * cannot describe the call's arguments, not that there are none. */ +const renderParams = (inputSchema: ToolSchemaDescriptor | undefined): string => { + if (!isSchemaObject(inputSchema) || inputSchema.type !== "object") { + return "(...)"; + } + + return renderPropertyGroup(inputSchema, true, "()", (joined) => `(${joined})`, "(...)"); +}; + +/** The ` -> ...` result suffix, empty when the tool declares no `output_schema` at all. An + * object-rooted result is inlined one level (`renderType`'s `type: "object"` branch, same as any + * other property value); anything else renders as its own type, e.g. ` -> string[]`. */ +const renderResult = (outputSchema: ToolSchemaDescriptor | undefined): string => { + if (outputSchema === undefined) { + return ""; + } + + return ` -> ${renderType(outputSchema, true)}`; +}; + +/** + * Renders a tool into its one-line call signature. Pure and total: no input can make this throw, + * and every unrecognised schema shape degrades to `...` (or the contextual `{...}`/`(...)`/ + * `unknown[]`) rather than omitting information or crashing the listing that calls it. + */ +export const renderToolSignature = ( + tool: Pick, +): string => { + const name = typeof tool?.name === "string" ? tool.name : ""; + return `${name}${renderParams(tool?.input_schema)}${renderResult(tool?.output_schema)}`; +}; diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 41797ede..0c9c63cd 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -4,5 +4,6 @@ export * from "./domains/messages.js"; export * from "./domains/rpc.js"; export * from "./domains/session.js"; export * from "./domains/tool-descriptor.js"; +export * from "./domains/tool-signature.js"; export * from "./domains/transport.js"; export * from "./standard-schema.js"; diff --git a/skills/appduct/SKILL.md b/skills/appduct/SKILL.md index ce7278cb..3152566e 100644 --- a/skills/appduct/SKILL.md +++ b/skills/appduct/SKILL.md @@ -14,21 +14,27 @@ auto-spawn it on first use — there is no separate "start the host" step to man ## Agent workflow (CLI) -1. Run **`appduct ls --json`**. `data` is a list of sessions, each with `sessionId`, - `alias`, `state`, `device`, `toolCount`. An empty list means no device has claimed a - session yet — go to **Establish a session** below. +1. Run **`appduct ls`**. It lists sessions, each with a session id, alias, state, + device, and tool count. An empty list means no device has claimed a session yet — go + to **Establish a session** below. 2. Every session-targeting command takes an optional **selector** (a session id or `alias` from step 1) as its first positional argument. **Omit it** when exactly one session is active — the CLI picks it automatically; pass it explicitly when several sessions exist (the CLI errors with `ambiguous_session` and lists the aliases if you don't). -3. **`appduct tools [selector] --json`** — list tools registered in the app. -4. **`appduct tools [selector] --full --json`** — inspect one tool's - input/output schema before calling it. -5. **`appduct invoke [selector] --input '{"key":"value"}' --json`** — - invoke the tool with JSON args. -6. **`appduct events [selector] --json`** — stream session/tool events (NDJSON) if you - need to watch for `session_claimed`, `tools_changed`, or `app_event` without polling. +3. **`appduct tools [selector]`** — list tools registered in the app. Each line is a + call signature (`name(params) -> result`) plus a one-line description, not a full + schema — cheap to read even for an app with hundreds of tools. `...` anywhere in a + signature means the CLI could not summarize that part of the schema; fetch the full + tool (step 4) to see it. On a large app, narrow first with `--filter ` (matches + name or description) and page with `--limit `/`--offset ` if the listing says + tools were left out. +4. **`appduct tools [selector] `** — the tool's full input/output schema + (`--full` is implied for a single tool, no need to pass it). +5. **`appduct invoke [selector] --input '{"key":"value"}'`** — invoke the + tool with JSON args. +6. **`appduct events [selector]`** — stream session/tool events if you need to watch for + `session_claimed`, `tools_changed`, or `app_event` without polling. There is no `--session-id` flag in v2 — use the positional selector instead. @@ -175,8 +181,10 @@ arguments to pass (issue #34). ## Notes -- Use **`--json`** for structured CLI output in agent flows; runtime failures in - `--json` mode are JSON on stderr, not bare text. +- Plain text is the CLI's default output and is meant to be read, not parsed — its exact + wording and layout may change between versions. Add **`--json`** only when a script (not + you) will parse the output, and `--pretty` to indent it for readability. Runtime failures + under `--json` are JSON on stderr, not bare text. - `appduct init`, run once in an app root, records the scheme in `.appduct/config.json` and prints the MCP server entry to paste. Re-running it is always safe (it keeps the recorded scheme; `--force` re-adopts `app.json`'s), it never From 2304f015df652c9830d26f85b645c837b5cbb4f9 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Mon, 21 Sep 2026 08:19:39 +0200 Subject: [PATCH 3/8] fix(cli): render route argument errors as usage errors, read --filter verbatim Parsing positionals and flags in a route body let a usage error escape the runner, so the built CLI crashed with a stack trace on `tools --limit 0`, `invoke` with no tool, or too many positionals. Parse inside the handler instead, before the version check. cac never enforces a placeholder, so `--limit -1` and a bare `--limit` arrived as true and were read as 1; reject them. cac also coerces numeric-looking values to numbers, which silently dropped `--filter 404` and turned `--filter 007` into 7; recover the verbatim text from argv, now carried on RouteContext. --- CHANGELOG.md | 4 ++ .../src/__tests__/command-options.test.ts | 35 +++++++++++++ .../global-flags.integration.test.ts | 26 ++++++++++ packages/appduct/src/__tests__/router.test.ts | 1 + packages/appduct/src/cli/command-options.ts | 50 ++++++++++++++++++- packages/appduct/src/cli/dispatch.ts | 1 + packages/appduct/src/cli/router.ts | 5 ++ packages/appduct/src/cli/routes/events.ts | 40 ++++++++------- packages/appduct/src/cli/routes/invoke.ts | 37 ++++++++------ packages/appduct/src/cli/routes/revoke.ts | 8 ++- packages/appduct/src/cli/routes/tools.ts | 33 +++++++----- 11 files changed, 189 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6626df64..e57b3f99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,10 @@ package versions for a release. for details. - The `appduct` agent skill now defaults its example commands to plain-text output, adding `--json` only where a script (not the agent itself) will parse the result. +- **Fixed:** a bad argument to `tools`, `invoke`, `revoke` or `events` (a missing ``, too + many positionals, `--limit 0`) crashed the CLI with a stack trace instead of printing a usage + error with exit code 64. A numeric flag given without a value (`--limit`, `--limit -1`, + `--since`, `--ttl`, `--timeout`) is now a usage error; it used to be read as `1`. ## 0.10.0 (2026-09-16) diff --git a/packages/appduct/src/__tests__/command-options.test.ts b/packages/appduct/src/__tests__/command-options.test.ts index dc906c60..ce2bc158 100644 --- a/packages/appduct/src/__tests__/command-options.test.ts +++ b/packages/appduct/src/__tests__/command-options.test.ts @@ -4,6 +4,7 @@ import { parseJsonInputOption, parseNonNegativeIntegerOption, parsePositiveIntegerOption, + readTextOption, splitOptionalSelector, splitOptionalSelectorAndTarget, splitSelectorAndRequiredTarget, @@ -42,6 +43,40 @@ describe("parseNonNegativeIntegerOption", () => { }); }); +describe("integer options given without a value", () => { + // `cac` reports `--limit` with no value, or `--limit -1` (it reads `-1` as a flag), as `true`. + test("a boolean is rejected rather than coerced to 1", () => { + expect(() => parsePositiveIntegerOption(true, "--limit")).toThrow(/positive integer/u); + expect(() => parseNonNegativeIntegerOption(true, "--offset")).toThrow(/non-negative integer/u); + }); + + test("a repeated flag (an array) is rejected", () => { + expect(() => parsePositiveIntegerOption([1, 2], "--limit")).toThrow(/positive integer/u); + }); +}); + +describe("readTextOption", () => { + test("passes through undefined", () => { + expect(readTextOption([], undefined, "--filter")).toBeUndefined(); + }); + + test("recovers the verbatim string cac coerced to a number", () => { + expect(readTextOption(["tools", "--filter", "007"], 7, "--filter")).toBe("007"); + expect(readTextOption(["tools", "--filter=1e3"], 1000, "--filter")).toBe("1e3"); + expect(readTextOption(["tools", "--filter", "404"], 404, "--filter")).toBe("404"); + }); + + test("the last occurrence wins and nothing after -- counts", () => { + expect(readTextOption(["--filter", "a", "--filter", "b", "--", "--filter", "c"], ["a", "b"], "--filter")).toBe( + "b", + ); + }); + + test("a flag with no value is a usage error", () => { + expect(() => readTextOption(["tools", "--filter"], true, "--filter")).toThrow(/requires a value/u); + }); +}); + describe("parseJsonInputOption", () => { test("parses a valid JSON object", () => { expect(parseJsonInputOption('{"text":"hi"}')).toEqual({ text: "hi" }); diff --git a/packages/appduct/src/__tests__/global-flags.integration.test.ts b/packages/appduct/src/__tests__/global-flags.integration.test.ts index ccac64ad..8f604b3c 100644 --- a/packages/appduct/src/__tests__/global-flags.integration.test.ts +++ b/packages/appduct/src/__tests__/global-flags.integration.test.ts @@ -52,3 +52,29 @@ describe("global flags (no daemon required)", () => { expect(result.stderr).toContain("Command: cli"); }); }); + +/** + * A route's own argument errors (bad `--limit`, missing ``, too many positionals) must render + * through the runner like any other usage error. Parsed in the route body, they escaped as an + * uncaught rejection and the built CLI crashed with a stack trace. + */ +describe("route argument errors render as usage errors (no daemon required)", () => { + test.each([ + [["tools", "--limit", "0"], /"--limit" must be a positive integer/u], + [["tools", "--limit", "-1"], /"--limit" must be a positive integer/u], + [["tools", "--offset", "abc"], /"--offset" must be a non-negative integer/u], + [["tools", "--filter"], /"--filter" requires a value/u], + [["tools", "a", "b", "c"], /Usage/u], + [["invoke"], /Usage/u], + [["revoke", "a", "b"], /Usage/u], + [["events", "a", "b"], /Usage/u], + [["events", "--since", "-1"], /"--since" must be a non-negative integer/u], + ])("%j", async (argv, message) => { + const result = await runCliWithCapture([...argv, "--json", "--state-dir", "/nonexistent-appduct-state"]); + + expect(result.exitCode).toBe(64); + const parsed = JSON.parse(result.stdout); + expect(parsed.error.type).toBe("usage_error"); + expect(parsed.error.message).toMatch(message); + }); +}); diff --git a/packages/appduct/src/__tests__/router.test.ts b/packages/appduct/src/__tests__/router.test.ts index 05a95938..97fad6a6 100644 --- a/packages/appduct/src/__tests__/router.test.ts +++ b/packages/appduct/src/__tests__/router.test.ts @@ -18,6 +18,7 @@ const makeContext = (args: string[]): { context: RouteContext; stdout: () => str path: [], args, options: {}, + argv: [], env: { flags: { json: true, pretty: false, verbose: true, color: false }, clock: fixedClock, diff --git a/packages/appduct/src/cli/command-options.ts b/packages/appduct/src/cli/command-options.ts index d18e455a..587999b1 100644 --- a/packages/appduct/src/cli/command-options.ts +++ b/packages/appduct/src/cli/command-options.ts @@ -13,7 +13,10 @@ export const parsePositiveIntegerOption = (value: unknown, flagName: string): nu return undefined; } - const parsed = typeof value === "number" ? value : Number(value); + // Only a number or a string is a value: `cac` (run with `run: false`) never enforces a `` + // placeholder, so a flag with no value, or followed by a flag-like token (`--limit -1`), + // arrives as `true`, which `Number()` would otherwise silently turn into `1`. + const parsed = typeof value === "number" ? value : typeof value === "string" ? Number(value) : Number.NaN; if (!Number.isFinite(parsed) || !Number.isInteger(parsed) || parsed <= 0) { throw usageError(`"${flagName}" must be a positive integer.`); @@ -30,7 +33,10 @@ export const parseNonNegativeIntegerOption = (value: unknown, flagName: string): return undefined; } - const parsed = typeof value === "number" ? value : Number(value); + // Only a number or a string is a value: `cac` (run with `run: false`) never enforces a `` + // placeholder, so a flag with no value, or followed by a flag-like token (`--limit -1`), + // arrives as `true`, which `Number()` would otherwise silently turn into `1`. + const parsed = typeof value === "number" ? value : typeof value === "string" ? Number(value) : Number.NaN; if (!Number.isFinite(parsed) || !Number.isInteger(parsed) || parsed < 0) { throw usageError(`"${flagName}" must be a non-negative integer.`); @@ -39,6 +45,46 @@ export const parseNonNegativeIntegerOption = (value: unknown, flagName: string): return parsed; }; +/** + * Reads a free-text flag's value exactly as typed. `cac` coerces any numeric-looking value to a + * number (`--filter 404` arrives as `404`, `--filter 007` as `7`), so the verbatim string is + * recovered from `argv` (`--flag value` or `--flag=value`, the last occurrence wins, nothing after + * `--`), falling back to the parsed value if argv somehow does not carry it. A flag given with no + * value (`cac` reports `true`) is a usage error rather than silently ignored. + */ +export const readTextOption = ( + argv: readonly string[], + value: unknown, + flagName: string, +): string | undefined => { + if (value === undefined) { + return undefined; + } + + if (typeof value === "boolean") { + throw usageError(`"${flagName}" requires a value.`); + } + + let raw: string | undefined; + + for (let index = 0; index < argv.length; index += 1) { + const token = argv[index]!; + + if (token === "--") { + break; + } + + if (token === flagName && index + 1 < argv.length) { + raw = argv[index + 1]; + index += 1; + } else if (token.startsWith(`${flagName}=`)) { + raw = token.slice(flagName.length + 1); + } + } + + return raw ?? String(Array.isArray(value) ? value.at(-1) : value); +}; + /** * Splits the positional args of a command shaped ` [selector] ` (e.g. `invoke * [selector] `, `tools [selector] `): the last positional is always the required diff --git a/packages/appduct/src/cli/dispatch.ts b/packages/appduct/src/cli/dispatch.ts index f052f658..3a035fc6 100644 --- a/packages/appduct/src/cli/dispatch.ts +++ b/packages/appduct/src/cli/dispatch.ts @@ -157,6 +157,7 @@ export const runCli = async (argv: string[], options: RunCliOptions = {}): Promi // consumes it the same way the nested routers consume theirs. args: [matchedCommand, ...parsedArgs], options: parsedOptions, + argv, env, stateDir, versionCheck: { diff --git a/packages/appduct/src/cli/router.ts b/packages/appduct/src/cli/router.ts index c3f95755..da49d346 100644 --- a/packages/appduct/src/cli/router.ts +++ b/packages/appduct/src/cli/router.ts @@ -51,6 +51,11 @@ export type RouteContext = { readonly args: readonly string[]; /** Every parsed flag (global and per-command), as `cac` reports them (camelCased). */ readonly options: Readonly>; + /** The raw argv `runCli` received (no `node`/script prefix). For the rare flag whose value must + * be read verbatim: `cac` coerces every numeric-looking value to a number (`--filter 007` → `7`), + * so a free-text flag recovers its exact string from here (`command-options.ts`'s + * `readTextOption`). */ + readonly argv: readonly string[]; readonly env: CliEnv; /** The resolved state directory (`--state-dir` / `APPDUCT_STATE_DIR` / default). */ readonly stateDir: string; diff --git a/packages/appduct/src/cli/routes/events.ts b/packages/appduct/src/cli/routes/events.ts index f9fadd55..b792bb4a 100644 --- a/packages/appduct/src/cli/routes/events.ts +++ b/packages/appduct/src/cli/routes/events.ts @@ -14,33 +14,37 @@ import { guarded } from "../version-guard.js"; export const route: Route = async (context) => { const { options, stateDir, env } = context; - const { selector } = splitOptionalSelector(context.args, "events [selector]"); - const since = parseNonNegativeIntegerOption(options.since, "--since"); const follow = Boolean(options.follow); return executeHostedCommand( commandName(context), - guarded(context)(() => { - // Deferred into the wrapped handler (rather than thrown directly in the route body, - // matching the codebase's existing lax convention for that) so `executeHostedCommand`'s - // own try/catch renders it as a normal usage_error instead of an uncaught rejection. + // All argument parsing happens inside the handler (not the route body) so + // `executeHostedCommand`'s own try/catch renders a bad argument as a normal usage_error + // instead of an uncaught rejection, and before the version check so a typo never waits on + // the daemon. + () => { + const { selector } = splitOptionalSelector(context.args, "events [selector]"); + const since = parseNonNegativeIntegerOption(options.since, "--since"); + if (since !== undefined && follow) { throw usageError('"--since" is a one-shot pull and cannot be combined with "--follow".'); } - return handleEventsCommand( - { selector, since }, - { - stateDir, - onEvent: (event: EventNotification) => { - env.stdout.write(`${renderEventLine(event, env.flags)}\n`); - }, - onCursor: (cursor) => { - env.stdout.write(`${renderEventsCursorLine(cursor, env.flags)}\n`); + return guarded(context)(() => + handleEventsCommand( + { selector, since }, + { + stateDir, + onEvent: (event: EventNotification) => { + env.stdout.write(`${renderEventLine(event, env.flags)}\n`); + }, + onCursor: (cursor) => { + env.stdout.write(`${renderEventsCursorLine(cursor, env.flags)}\n`); + }, }, - }, - ); - }), + ), + )(); + }, env, { kind: "interactive", diff --git a/packages/appduct/src/cli/routes/invoke.ts b/packages/appduct/src/cli/routes/invoke.ts index bb79ee64..068312e0 100644 --- a/packages/appduct/src/cli/routes/invoke.ts +++ b/packages/appduct/src/cli/routes/invoke.ts @@ -14,10 +14,6 @@ import { guarded } from "../version-guard.js"; export const route: Route = async (context) => { const { options, stateDir } = context; - const { selector, target: tool } = splitSelectorAndRequiredTarget( - context.args, - "invoke [selector] --input ''", - ); // SIGINT cancels the in-flight tools.call rather than leaving it running unowned in the app // (issue #9) — the listener is torn down once the command settles either way. @@ -28,18 +24,27 @@ export const route: Route = async (context) => { try { return await executeCommand( commandName(context), - guarded(context)(() => - handleInvokeCommand( - { - selector, - tool, - args: parseJsonInputOption(typeof options.input === "string" ? options.input : undefined), - timeoutMs: parsePositiveIntegerOption(options.timeout, "--timeout"), - }, - { stateDir }, - cancelController.signal, - ), - ), + // Positionals are split inside the handler so a missing `` renders through the runner + // as a usage error instead of escaping the route as an uncaught rejection. + () => { + const { selector, target: tool } = splitSelectorAndRequiredTarget( + context.args, + "invoke [selector] --input ''", + ); + + return guarded(context)(() => + handleInvokeCommand( + { + selector, + tool, + args: parseJsonInputOption(typeof options.input === "string" ? options.input : undefined), + timeoutMs: parsePositiveIntegerOption(options.timeout, "--timeout"), + }, + { stateDir }, + cancelController.signal, + ), + )(); + }, context.env, ); } finally { diff --git a/packages/appduct/src/cli/routes/revoke.ts b/packages/appduct/src/cli/routes/revoke.ts index 1850f84b..4719d4b6 100644 --- a/packages/appduct/src/cli/routes/revoke.ts +++ b/packages/appduct/src/cli/routes/revoke.ts @@ -10,11 +10,15 @@ import { guarded } from "../version-guard.js"; export const route: Route = async (context) => { const { stateDir } = context; - const { selector } = splitOptionalSelector(context.args, "revoke [selector]"); return executeCommand( commandName(context), - guarded(context)(() => handleRevokeCommand({ selector }, { stateDir })), + // Parsed inside the handler so a usage error renders through the runner instead of escaping + // the route as an uncaught rejection. + () => { + const { selector } = splitOptionalSelector(context.args, "revoke [selector]"); + return guarded(context)(() => handleRevokeCommand({ selector }, { stateDir }))(); + }, context.env, ); }; diff --git a/packages/appduct/src/cli/routes/tools.ts b/packages/appduct/src/cli/routes/tools.ts index 572737c7..9d964608 100644 --- a/packages/appduct/src/cli/routes/tools.ts +++ b/packages/appduct/src/cli/routes/tools.ts @@ -6,6 +6,7 @@ import { handleToolsCommand } from "../../commands/tools.js"; import { parseNonNegativeIntegerOption, parsePositiveIntegerOption, + readTextOption, splitOptionalSelectorAndTarget, } from "../command-options.js"; import { commandName } from "../router.js"; @@ -14,22 +15,28 @@ import { guarded } from "../version-guard.js"; export const route: Route = async (context) => { const { options, stateDir } = context; - const { selector, target, selectorOrTarget } = splitOptionalSelectorAndTarget( - context.args, - "tools [selector] [name]", - ); - const limit = parsePositiveIntegerOption(options.limit, "--limit"); - const offset = parseNonNegativeIntegerOption(options.offset, "--offset"); - const filter = typeof options.filter === "string" ? options.filter : undefined; return executeCommand( commandName(context), - guarded(context)(() => - handleToolsCommand( - { selector: selector ?? selectorOrTarget, name: target, filter, limit, offset }, - { stateDir }, - ), - ), + // Argument parsing runs inside the handler so a usage error renders through the runner (exit + // 64, `--json` envelope) instead of escaping the route as an uncaught rejection, and before + // the version check so a typo never waits on (or restarts) the daemon. + () => { + const { selector, target, selectorOrTarget } = splitOptionalSelectorAndTarget( + context.args, + "tools [selector] [name]", + ); + const limit = parsePositiveIntegerOption(options.limit, "--limit"); + const offset = parseNonNegativeIntegerOption(options.offset, "--offset"); + const filter = readTextOption(context.argv, options.filter, "--filter"); + + return guarded(context)(() => + handleToolsCommand( + { selector: selector ?? selectorOrTarget, name: target, filter, limit, offset }, + { stateDir }, + ), + )(); + }, context.env, { full: Boolean(options.full) }, ); From 4d9d6ca8697e6d14fa1834ccf2e2ca804dd170e9 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Mon, 21 Sep 2026 08:19:46 +0200 Subject: [PATCH 4/8] fix(cli): tools listing edge cases past the end, name probe, and descriptions An --offset past the end printed "No tools registered" for a non-empty registry; it now says which offset was empty and how many tools match. The single-arg form that resolves to a tool name silently ignored --filter/--limit/--offset; it now fails the same way an explicit does. The listed description summary now breaks on any line terminator, drops control characters, and never cuts through a surrogate pair. --- .../src/__tests__/cli-v2.integration.test.ts | 17 +++++++++++ packages/appduct/src/commands/tools.ts | 16 +++++++++-- packages/appduct/src/output.ts | 28 +++++++++++++------ 3 files changed, 50 insertions(+), 11 deletions(-) diff --git a/packages/appduct/src/__tests__/cli-v2.integration.test.ts b/packages/appduct/src/__tests__/cli-v2.integration.test.ts index 9e0f74f6..c0a08c83 100644 --- a/packages/appduct/src/__tests__/cli-v2.integration.test.ts +++ b/packages/appduct/src/__tests__/cli-v2.integration.test.ts @@ -397,6 +397,23 @@ describe("appduct CLI v2: end-to-end command table", () => { expect(detail.ok).toBe(true); expect((detail.data as { name: string }).name).toBe("tool_29"); + // The single-arg form that resolves to a tool name follows the same rule as ` `. + const probeUnderPaging = await runCliJson(["tools", "tool_29", "--limit", "1"], stateDir); + expect(probeUnderPaging.ok).toBe(false); + expect(probeUnderPaging.error?.type).toBe("usage_error"); + + // A numeric-looking filter is matched as text, verbatim (cac alone would turn "07" into 7). + const numericFilter = await runCliJson(["tools", alias, "--filter", "07"], stateDir); + expect(numericFilter.ok).toBe(true); + const numericFilterData = numericFilter.data as { tools: Array<{ name: string }>; total: number; filter: string }; + expect(numericFilterData.tools.map((tool) => tool.name)).toEqual(["tool_07"]); + expect(numericFilterData.filter).toBe("07"); + + // An offset past the end is an empty page of a non-empty registry, not "No tools registered". + const pastEnd = await runCliHuman(["tools", alias, "--offset", "100"], stateDir); + expect(pastEnd.stdout).toContain("No tools at offset 100; 30 matching tools in total."); + expect(pastEnd.stdout).not.toContain("No tools registered"); + // Human output: a signature line per tool, and the "Showing" line once the page truncates. const human = await runCliHuman(["tools", alias, "--limit", "5"], stateDir); expect(human.stdout).toContain("tool_00(value: string)"); diff --git a/packages/appduct/src/commands/tools.ts b/packages/appduct/src/commands/tools.ts index 39dc5460..da1133a8 100644 --- a/packages/appduct/src/commands/tools.ts +++ b/packages/appduct/src/commands/tools.ts @@ -71,14 +71,18 @@ const toListing = (result: ToolsListResult, params: ListParams): ToolsListing => }; }; +const listingOnlyError = () => { + return usageError( + '"--filter", "--limit", and "--offset" only apply to a tools listing, not a single tool lookup.', + ); +}; + export const handleToolsCommand = async ( options: ToolsCommandOptions, context: ToolsCommandContext, ): Promise> => { if (options.name !== undefined && hasPagingOptions(options)) { - throw usageError( - '"--filter", "--limit", and "--offset" only apply to a tools listing, not a single tool lookup.', - ); + throw listingOnlyError(); } if (options.selector !== undefined && options.name !== undefined) { @@ -111,6 +115,12 @@ export const handleToolsCommand = async ( const tool = findTool(implicitTools.tools, options.selector); if (tool) { + // The same rule as an explicit ` `: silently dropping the listing flags + // here would make `tools --limit 5` behave differently from `tools `. + if (hasPagingOptions(options)) { + throw listingOnlyError(); + } + return { ok: true, data: tool }; } } diff --git a/packages/appduct/src/output.ts b/packages/appduct/src/output.ts index 80ed2571..33377132 100644 --- a/packages/appduct/src/output.ts +++ b/packages/appduct/src/output.ts @@ -161,17 +161,29 @@ const isToolsListing = (data: ToolsCommandData): data is ToolsListing => { const MAX_LISTED_DESCRIPTION_LENGTH = 120; const summarizeDescription = (description: string): string => { - const firstLine = (description.split("\n")[0] ?? "").trim(); - - return firstLine.length > MAX_LISTED_DESCRIPTION_LENGTH - ? `${firstLine.slice(0, MAX_LISTED_DESCRIPTION_LENGTH)}…` + // Any line break ends the first line (`\r` alone included), and remaining control characters + // are dropped: the description is app-supplied text printed straight to a terminal. + const firstLine = (description.split(/\r\n|[\n\r\u2028\u2029]/u)[0] ?? "") + .replace(/\p{Cc}/gu, "") + .trim(); + // Cut by code point, never through the middle of a surrogate pair. + const codePoints = Array.from(firstLine); + + return codePoints.length > MAX_LISTED_DESCRIPTION_LENGTH + ? `${codePoints.slice(0, MAX_LISTED_DESCRIPTION_LENGTH).join("")}…` : firstLine; }; /** "No tools registered"/"No tools match" for an empty listing (compact or `--full` — both share * this line, only the header differs). */ -const renderEmptyToolsLine = (filter: string | undefined): string => { - return filter === undefined ? " No tools registered." : ` No tools match "${filter}".`; +const renderEmptyToolsLine = (data: ToolsListing): string => { + if (data.total > 0) { + // An empty page of a non-empty result: `--offset` ran past the end. Saying "No tools + // registered" here would send an agent looking for a registry problem that is not there. + return ` No tools at offset ${data.offset ?? 0}; ${data.total} matching tool${data.total === 1 ? "" : "s"} in total.`; + } + + return data.filter === undefined ? " No tools registered." : ` No tools match ${JSON.stringify(data.filter)}.`; }; /** The `Showing n of total tools (offset o). Narrow with --filter or page with --offset @@ -191,7 +203,7 @@ const renderTruncationLine = (data: ToolsListing): string[] => { const renderToolSummaryTable = (colors: ColorPalette, data: ToolsListing): string[] => { if (data.tools.length === 0) { - return [colors.green("Tools"), renderEmptyToolsLine(data.filter)]; + return [colors.green("Tools"), renderEmptyToolsLine(data)]; } return [ @@ -226,7 +238,7 @@ const renderToolDetail = (colors: ColorPalette, tool: ToolDescriptor, flags: Glo const renderToolsFullListing = (colors: ColorPalette, data: ToolsListing, flags: GlobalFlags): string[] => { if (data.tools.length === 0) { - return [colors.green("Tools"), renderEmptyToolsLine(data.filter)]; + return [colors.green("Tools"), renderEmptyToolsLine(data)]; } return [ From e1e3b3f9b11e423acc59fb18a242e19098c7594b Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Mon, 21 Sep 2026 08:19:46 +0200 Subject: [PATCH 5/8] fix(shared): keep renderToolSignature total and one line on hostile schemas Nested array items recursed without bound (a cyclic items overflowed the stack), a BigInt const or default threw from JSON.stringify, and a throwing getter escaped. Cap array depth, stringify defensively, and fall back to name(...) on anything else. Enum and const literals are cut at 40 characters, and a property name that is not identifier-like is JSON-quoted so a newline or escape sequence cannot break or hijack the line. --- .../src/__tests__/tool-signature.test.ts | 56 ++++++++++++++ packages/shared/src/domains/tool-signature.ts | 75 ++++++++++++++++--- 2 files changed, 119 insertions(+), 12 deletions(-) diff --git a/packages/shared/src/__tests__/tool-signature.test.ts b/packages/shared/src/__tests__/tool-signature.test.ts index a5feeacd..1325f935 100644 --- a/packages/shared/src/__tests__/tool-signature.test.ts +++ b/packages/shared/src/__tests__/tool-signature.test.ts @@ -332,6 +332,38 @@ const cases: Case[] = [ }, "deep(a?: { b?: {...} })", ], + [ + "hostile: arrays nested past the depth cap stop at ...[]", + { + name: "grid", + input_schema: { + type: "object", + properties: { + a: { + type: "array", + items: { + type: "array", + items: { + type: "array", + items: { type: "array", items: { type: "array", items: { type: "array", items: { type: "string" } } } }, + }, + }, + }, + }, + }, + }, + "grid(a?: ...[][][][][])", + ], + [ + "hostile: a huge enum value is cut, not printed whole", + { name: "e", input_schema: { type: "object", properties: { a: { enum: ["x".repeat(5000)] } } } }, + `e(a?: "${"x".repeat(39)}…)`, + ], + [ + "hostile: a property name with a line break or escape is quoted, never raw", + { name: "n", input_schema: { type: "object", properties: { "a\nb": { type: "string" } } } }, + 'n("a\\nb\\u001b[31m"?: string)', + ], ]; describe("renderToolSignature", () => { @@ -342,4 +374,28 @@ describe("renderToolSignature", () => { test("never throws on a completely empty object", () => { expect(() => renderToolSignature({ name: "x" })).not.toThrow(); }); + + // In-process inputs JSON can't carry, but "pure and total" is the contract. + test("never throws on a cyclic items, a BigInt const/default, or a throwing getter", () => { + const cyclic: Record = { type: "array" }; + cyclic.items = cyclic; + expect(renderToolSignature({ name: "c", input_schema: { type: "object", properties: { a: cyclic } } })).toBe( + "c(a?: ...[][][][][])", + ); + + expect( + renderToolSignature({ + name: "b", + input_schema: { type: "object", properties: { a: { const: 1n }, b: { type: "integer", default: 1n } } }, + }), + ).toBe("b(a?: ..., b?: int)"); + + const throwing = { + type: "object", + get properties(): never { + throw new Error("boom"); + }, + }; + expect(renderToolSignature({ name: "t", input_schema: throwing })).toBe("t(...)"); + }); }); diff --git a/packages/shared/src/domains/tool-signature.ts b/packages/shared/src/domains/tool-signature.ts index a06f4d7c..42dfd881 100644 --- a/packages/shared/src/domains/tool-signature.ts +++ b/packages/shared/src/domains/tool-signature.ts @@ -39,8 +39,43 @@ const MAX_ENUM_VALUES = 5; * default belongs in the full schema (`tools `), not a one-line signature. */ const MAX_DEFAULT_LENGTH = 20; +/** An `enum`/`const` value longer than this (as JSON) is cut with `…` — one 5 KB enum string must + * not turn a one-line signature into a page. */ +const MAX_LITERAL_LENGTH = 40; +/** Nested `type: "array"` `items` deeper than this render as `...[]`. Objects already stop after + * one level; without this cap, arrays of arrays (or a cyclic `items`, possible for an in-process + * caller) would recurse until the stack overflows. */ +const MAX_ARRAY_DEPTH = 4; + +/** `JSON.stringify` that never throws (a `BigInt`, a cycle) — `undefined` when it can't render. */ +const tryStringify = (value: unknown): string | undefined => { + try { + const json = JSON.stringify(value); + return typeof json === "string" ? json : undefined; + } catch { + return undefined; + } +}; + +const renderLiteral = (value: unknown): string => { + const json = tryStringify(value); + + if (json === undefined) { + return "..."; + } + + const codePoints = Array.from(json); + return codePoints.length > MAX_LITERAL_LENGTH ? `${codePoints.slice(0, MAX_LITERAL_LENGTH).join("")}…` : json; +}; + +/** A property name as written in the signature: bare when it is identifier-like, else quoted as + * JSON, so a hostile key (a newline, an escape sequence) can never break the line it sits on. */ +const renderPropertyName = (name: string): string => { + return /^[\p{L}\p{N}_$-]+$/u.test(name) ? name : JSON.stringify(name); +}; + const renderEnumValues = (values: unknown[]): string => { - const shown = values.slice(0, MAX_ENUM_VALUES).map((value) => JSON.stringify(value)); + const shown = values.slice(0, MAX_ENUM_VALUES).map(renderLiteral); const suffix = values.length > MAX_ENUM_VALUES ? " | …" : ""; return `${shown.join(" | ")}${suffix}`; }; @@ -112,10 +147,20 @@ const renderObjectType = (schema: JsonSchema): string => { return renderPropertyGroup(schema, false, "{}", (joined) => `{ ${joined} }`, "{...}"); }; -const renderScalarType = (typeValue: unknown, schema: JsonSchema, expandObject: boolean): string => { +const renderScalarType = ( + typeValue: unknown, + schema: JsonSchema, + expandObject: boolean, + arrayDepth: number, +): string => { if (typeValue === "array") { const items = schema.items; - return isSchemaObject(items) ? `${renderType(items, expandObject)}[]` : "unknown[]"; + + if (!isSchemaObject(items)) { + return "unknown[]"; + } + + return arrayDepth >= MAX_ARRAY_DEPTH ? "...[]" : `${renderType(items, expandObject, arrayDepth + 1)}[]`; } if (typeValue === "object") { @@ -139,7 +184,7 @@ const renderScalarType = (typeValue: unknown, schema: JsonSchema, expandObject: * `type: "object"` expansion (`renderObjectType` always passes `false` back down, so nothing * expands twice). */ -const renderType = (schema: unknown, expandObject: boolean): string => { +const renderType = (schema: unknown, expandObject: boolean, arrayDepth = 0): string => { if (!isSchemaObject(schema)) { return "..."; } @@ -149,8 +194,7 @@ const renderType = (schema: unknown, expandObject: boolean): string => { } if ("const" in schema) { - const rendered = JSON.stringify(schema.const); - return typeof rendered === "string" ? rendered : "..."; + return renderLiteral(schema.const); } if (Array.isArray(schema.enum)) { @@ -160,10 +204,10 @@ const renderType = (schema: unknown, expandObject: boolean): string => { const type = schema.type; if (Array.isArray(type)) { - return type.map((entry) => renderScalarType(entry, schema, expandObject)).join(" | "); + return type.map((entry) => renderScalarType(entry, schema, expandObject, arrayDepth)).join(" | "); } - return renderScalarType(type, schema, expandObject); + return renderScalarType(type, schema, expandObject, arrayDepth); }; const renderParamEntry = ( @@ -172,12 +216,12 @@ const renderParamEntry = ( required: boolean, expandObject: boolean, ): string => { - let entry = `${name}${required ? "" : "?"}: ${renderType(propSchema, expandObject)}`; + let entry = `${renderPropertyName(name)}${required ? "" : "?"}: ${renderType(propSchema, expandObject)}`; if (isSchemaObject(propSchema) && "default" in propSchema) { - const json = JSON.stringify(propSchema.default); + const json = tryStringify(propSchema.default); - if (typeof json === "string" && json.length <= MAX_DEFAULT_LENGTH) { + if (json !== undefined && json.length <= MAX_DEFAULT_LENGTH) { entry += ` = ${json}`; } } @@ -217,5 +261,12 @@ export const renderToolSignature = ( tool: Pick, ): string => { const name = typeof tool?.name === "string" ? tool.name : ""; - return `${name}${renderParams(tool?.input_schema)}${renderResult(tool?.output_schema)}`; + + try { + return `${name}${renderParams(tool?.input_schema)}${renderResult(tool?.output_schema)}`; + } catch { + // Unreachable for JSON off the wire; an in-process caller can still hand over an object whose + // getters throw. The listing that calls this must not die for one tool. + return `${name}(...)`; + } }; From 3df593aed6c98eb776aebdd69faa630773841d60 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Mon, 21 Sep 2026 08:19:46 +0200 Subject: [PATCH 6/8] fix(client): accept a bare-array tools.list result from an older daemon appduct/client runs no daemon version check, so a newer client against an already-running older daemon got undefined from tools(). Accept both shapes. --- .../appduct/src/__tests__/app-client.test.ts | 32 +++++++++++++++++++ packages/appduct/src/client/app-client.ts | 8 +++-- 2 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 packages/appduct/src/__tests__/app-client.test.ts diff --git a/packages/appduct/src/__tests__/app-client.test.ts b/packages/appduct/src/__tests__/app-client.test.ts new file mode 100644 index 00000000..213eb8f3 --- /dev/null +++ b/packages/appduct/src/__tests__/app-client.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, test } from "vitest"; + +import { makeAppClient } from "../client/app-client.js"; +import type { DaemonStream } from "../rpc/client.js"; + +const toolEntry = { + name: "ping", + description: "Ping.", + input_schema: { type: "object" }, + policy: "allow", +}; + +const streamAnswering = (result: unknown): DaemonStream => { + return { + call: async () => result as T, + onNotification: () => () => {}, + onClose: () => () => {}, + close: () => {}, + }; +}; + +describe("AppClient.tools()", () => { + test("unwraps the `{ tools, total }` tools.list result", async () => { + const client = makeAppClient(streamAnswering({ tools: [toolEntry], total: 1 }), "s1"); + expect(await client.tools()).toEqual([toolEntry]); + }); + + test("still accepts a bare array from a daemon that predates `{ tools, total }`", async () => { + const client = makeAppClient(streamAnswering([toolEntry]), "s1"); + expect(await client.tools()).toEqual([toolEntry]); + }); +}); diff --git a/packages/appduct/src/client/app-client.ts b/packages/appduct/src/client/app-client.ts index 717a18d4..6a23af42 100644 --- a/packages/appduct/src/client/app-client.ts +++ b/packages/appduct/src/client/app-client.ts @@ -168,8 +168,12 @@ export const makeAppClient = (stream: DaemonStream, sessionId: try { // No `filter`/`limit`/`offset`: this client's public `tools()` contract is "every tool on // this session", unchanged by `tools.list`'s daemon-side paging (added for the CLI). - const { tools } = await stream.call(RPC_METHODS.toolsList, { selector: sessionId }); - return tools; + const result = await stream.call(RPC_METHODS.toolsList, { + selector: sessionId, + }); + // Unlike the CLI, this client runs no daemon version check, so it can meet a daemon from + // before `tools.list` returned `{ tools, total }` — one that still answers a bare array. + return Array.isArray(result) ? result : result.tools; } catch (error) { throw toAppductError(error); } From 8d0b611d0c797569a8664d00aa9ea856ca9b6993 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Mon, 21 Sep 2026 08:25:58 +0200 Subject: [PATCH 7/8] fix(shared): render a tool with no input_schema as a no-argument call The SDKs omit input_schema for a tool that takes no input, and the MCP server already maps that to an empty object schema. Printing (...) sent agents to fetch a full schema that does not exist. --- packages/appduct/src/__tests__/output.test.ts | 4 ++-- .../shared/src/__tests__/tool-signature.test.ts | 2 +- packages/shared/src/domains/tool-signature.ts | 14 ++++++++++---- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/appduct/src/__tests__/output.test.ts b/packages/appduct/src/__tests__/output.test.ts index 73695448..081ebde2 100644 --- a/packages/appduct/src/__tests__/output.test.ts +++ b/packages/appduct/src/__tests__/output.test.ts @@ -156,14 +156,14 @@ describe("output rendering", () => { expect(descriptionLine!.endsWith("…")).toBe(true); }); - test("a tool with no input/output schema at all still gets a signature", () => { + test("a tool with no input/output schema at all renders as a no-argument call", () => { const rendered = renderResult( { ok: true, data: { tools: [{ name: "ping", description: "Health check.", policy: "allow" }], total: 1 } }, { command: "tools", flags: flags() }, ).stdout ?? ""; - expect(rendered).toContain(" ping(...)"); + expect(rendered).toContain(" ping()"); }); test("--full listing renders full detail blocks and still shows the Showing line, with no trailing hint", () => { diff --git a/packages/shared/src/__tests__/tool-signature.test.ts b/packages/shared/src/__tests__/tool-signature.test.ts index 1325f935..7e3ec8c4 100644 --- a/packages/shared/src/__tests__/tool-signature.test.ts +++ b/packages/shared/src/__tests__/tool-signature.test.ts @@ -5,7 +5,7 @@ import { renderToolSignature } from "../domains/tool-signature.js"; type Case = [label: string, descriptor: Parameters[0], expected: string]; const cases: Case[] = [ - ["no input/output schema at all", { name: "ping" }, "ping(...)"], + ["no input/output schema at all -> no-args, like the MCP mapping", { name: "ping" }, "ping()"], [ "input_schema not rooted at type: object", { name: "echo", input_schema: { type: "string" } }, diff --git a/packages/shared/src/domains/tool-signature.ts b/packages/shared/src/domains/tool-signature.ts index 42dfd881..e63cec13 100644 --- a/packages/shared/src/domains/tool-signature.ts +++ b/packages/shared/src/domains/tool-signature.ts @@ -229,11 +229,17 @@ const renderParamEntry = ( return entry; }; -/** The `(...)` params group. Unlike {@link renderObjectType}, an `input_schema` not rooted at - * `type: "object"` (or absent entirely) is always `(...)` — MCP requires an object-rooted input - * schema (`tool-descriptor.ts`'s `isObjectRootedSchema`), so anything else means this renderer - * cannot describe the call's arguments, not that there are none. */ +/** The `(...)` params group. An absent `input_schema` is `()`: the SDKs omit it for a tool that + * takes no input, and the MCP server maps it to an empty object schema (`tool-mapping.ts`). Unlike + * {@link renderObjectType}, a present `input_schema` not rooted at `type: "object"` is always + * `(...)` — MCP requires an object-rooted input schema (`tool-descriptor.ts`'s + * `isObjectRootedSchema`), so anything else means this renderer cannot describe the call's + * arguments, not that there are none. */ const renderParams = (inputSchema: ToolSchemaDescriptor | undefined): string => { + if (inputSchema === undefined) { + return "()"; + } + if (!isSchemaObject(inputSchema) || inputSchema.type !== "object") { return "(...)"; } From 1c54fbdc11a3d8c699e7d3c32621d0dd0b0d1eab Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Mon, 21 Sep 2026 10:27:04 +0200 Subject: [PATCH 8/8] test(appduct): adapt to ephemeral daemon ports and the in-memory MCP daemon Read the bound wss port from daemon status now that test state dirs ask for wssPort 0, and answer tools.list from the fake daemon with the sorted { tools, total } result. --- packages/appduct/src/__tests__/cli-v2.integration.test.ts | 4 +++- packages/appduct/src/__tests__/mcp-daemon-fake.ts | 8 +++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/appduct/src/__tests__/cli-v2.integration.test.ts b/packages/appduct/src/__tests__/cli-v2.integration.test.ts index c0a08c83..02a4da23 100644 --- a/packages/appduct/src/__tests__/cli-v2.integration.test.ts +++ b/packages/appduct/src/__tests__/cli-v2.integration.test.ts @@ -324,11 +324,13 @@ describe("appduct CLI v2: end-to-end command table", () => { "tools --filter/--limit/--offset page a large registry, a name lookup still resolves under paging, and human output is a signature listing", async () => { const stateDir = await makeTempStateDir(); - const port = JSON.parse(await readFile(path.join(stateDir, "config.json"), "utf8")).wssPort as number; const status = await runCliJson(["daemon", "status"], stateDir); expect(status.ok).toBe(true); daemonPids.push((status.data as { daemon: { pid: number } }).daemon.pid); + // `wssPort: 0` in the state dir, so the bound port is only known from the daemon itself. + const port = (status.data as { daemon: { wss_port: number } }).daemon.wss_port; + expect(port).toBeGreaterThan(0); const linkResult = await runCliJson(["link", "--ttl", "60"], stateDir); const linkData = linkResult.data as { deepLink: string }; diff --git a/packages/appduct/src/__tests__/mcp-daemon-fake.ts b/packages/appduct/src/__tests__/mcp-daemon-fake.ts index 7dadc6d1..87d60de2 100644 --- a/packages/appduct/src/__tests__/mcp-daemon-fake.ts +++ b/packages/appduct/src/__tests__/mcp-daemon-fake.ts @@ -156,7 +156,13 @@ export const createFakeDaemon = (): FakeDaemon => { throw toolError("unknown_session", `No session matches "${selector}".`); } - return entries.map((entry) => ({ ...entry })) as TResult; + // The daemon's `{ tools, total }` shape, sorted by name as the daemon sorts its registry. + // The server asks for the unpaged, unfiltered listing, so `total` is the whole registry. + const tools = entries + .map((entry) => ({ ...entry })) + .sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)); + + return { tools, total: tools.length } as TResult; } if (method === RPC_METHODS.toolsCall) {