cli: --app-info probe + --format json envelope

[cliffhall]

Closes #1574

App-probing and machine-readable output for the CLI, re-implemented informed by PR #1510 (33fac3f) and adapted to the v2/main OAuth-wired cli.ts.

What changed

• --method tools/call --tool-name <t> --app-info — prints one JSON line describing the tool's MCP App posture (hasApp, resourceUri, csp, permissions, domain, …) without invoking the tool; exits 0 when it has an app, 2 (no_app) when it doesn't.
• --method tools/list --app-info — one NDJSON line per tool over a single connection.
• --format json — a single stdout envelope ({ result } + { appInfo } sibling key for App tools), no banner, so any method pipes into jq.
• Exit-code refinements: a missing tool exits 5 (tool_not_found) — distinct from the no-app 2; a tools/call returning isError:true now exits 5 (tool_is_error) after printing its payload.

Implementation

• collectAppInfo() uses core extractAppInfo() plus a tolerated resources/read that reports failures in a resourceError field instead of throwing.
• A single emitResult() owns --format/--app-info output and the exit-code mapping; runMethod returns a MethodOutcome discriminated union so the NDJSON path signals "already emitted".

Tests & docs

• app-info.test.ts, format-json.test.ts (end-to-end via the bundled mcp_app_demo fixture), and emit-result.test.ts (direct unit coverage of the isError→exit-5 and resourceError branches via exported emitResult/collectAppInfo).
• Updated the tools.test "nonexistent tool" case to the new exit-5 tool_not_found contract.
• clients/cli/README.md documents the flags, NDJSON shape, and exit semantics.
• Per-file coverage ≥90 on all four dimensions (cli.ts 100% stmts/funcs/lines); full npm run ci passes.

Stacking

Second of the Wave 3 CLI stack. Base is v2/1573-cli-programmatic-ergonomics (#1573), not v2/main — review/merge #1573 first. The final target is v2/main.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 2m 4s —— View job

---

Review: --app-info probe + --format json envelope

Read the full diff against origin/v2/1573-cli-programmatic-ergonomics, plus error-handler.ts, core/mcp/apps.ts, and awaitable-log.ts. This is a clean, well-documented change with strong test coverage. The MethodOutcome discriminated union and single emitResult are a tidy way to keep the NDJSON path and the format/exit-code logic from tangling. A few observations, all minor — nothing blocking.

Findings

1. tools/list --app-info aborts the whole listing if any tool has a malformed UI URI — clients/cli/src/cli.ts:190-197
Inside the NDJSON loop, collectAppInfo → extractAppInfo(tool) throws on a malformed _meta.ui.resourceUri (documented behavior of getAppResourceUri/extractAppInfo in core/mcp/apps.ts). That throw is outside any per-tool try/catch, so a single bad tool kills the entire listing with a generic exit-1 rather than emitting the good lines. The collectAppInfo try/catch only wraps readResource, not the initial extractAppInfo. For a list probe, per-tool tolerance (emit {"toolName":…,"error":…} and continue) would be more robust and matches the "one line per tool" contract. The single tools/call --app-info path throwing is fine — but the list path arguably shouldn't be all-or-nothing. Fix this →

2. --format json is silently ignored for tools/list --app-info — clients/cli/src/cli.ts:190-198
When both flags are set on tools/list, the NDJSON branch runs first and emits raw AppInfo objects (JSON.stringify(info)), never wrapping them under an appInfo key the way the single-tool path does (emitResult, line 374). It's already machine-readable, so this is defensible, but the interaction isn't documented — a reader who expects --format json to change the shape gets NDJSON regardless. Worth either a one-line note in README.md/the --format help text, or explicitly rejecting the combination.

3. Dead helper: expectJsonError — clients/cli/__tests__/helpers/assertions.ts:49
Removing it from the tools.test.ts import (correct, given the new exit-5 contract) leaves expectJsonError defined but no longer referenced anywhere. Since it's an export it won't trip no-unused-vars, but it's now dead. Drop it, or keep it intentionally if you expect a future isError-returning test to want it.

Behavior change worth calling out (intended, but flag for consumers)

A tools/call returning isError:true now exits 5 (tool_is_error) where it previously exited 0 with the payload on stdout. This is the documented intent (&& chains shouldn't proceed on a failed call) and the payload is still printed, but any existing script that ran a tool expecting exit-0-with-isError will now see a non-zero exit. Documented in the README — good — just the highest-impact semantic change in the PR.

Things done well

• Exit-code taxonomy is coherent: tool_not_found (5) vs no_app (2) vs tool_is_error (5) are genuinely distinguishable by the code field even though two share exit 5.
• collectAppInfo tolerating a resources/read failure into resourceError instead of throwing is the right call — "tool claims an app but the resource is unreadable" is a real, useful probe result.
• Not collecting app-info on a plain text-mode tools/call avoids penalizing normal calls for a malformed _meta.ui; the guard (args.appInfo || args.format === "json") is exactly right.
• Trailing "\n" added to all awaitableLog outputs (was missing before) is a nice cleanup for line-oriented piping.
• Test coverage is thorough: the emitResult/collectAppInfo direct-unit tests cover precisely the two branches the stdio fixture can't reach (isError→5, resourceError), and the non-Error rejection stringify path is covered too.

Overall: LGTM. The findings are polish, not correctness blockers.
· branch v2/1574-cli-app-info-format-json

[cliffhall]

Thanks — applied 2 of 3 findings and documented the third. Details:

1. tools/list --app-info all-or-nothing on a malformed UI URI — applied (38f494f). Good catch. Rather than a loop-local try/catch, I moved the tolerance into collectAppInfo itself: the initial extractAppInfo(tool) is now guarded, so a malformed _meta.ui.resourceUri folds into {hasApp:false, toolName, resourceError} exactly like a resources/read failure. The NDJSON loop is naturally per-tool tolerant now — one bad tool can't abort the listing. Guarding inside the helper (vs. the loop) also keeps the branch unit-testable; added an emit-result.test.ts case driving a http://… (non-ui://) resourceUri and asserting hasApp:false + resourceError with readResource never reached. Side effect: tools/call --app-info on a malformed tool now exits 2 (no_app) with a resourceError field instead of a generic exit-1 throw — which I think is strictly better (the probe reports why it found no usable app).

2. --format json silently ignored for tools/list --app-info — documented. Added an explicit note to the README (and the code comment): tools/list --app-info always emits NDJSON regardless of --format; --format json only reshapes the single-result paths into the {result[, appInfo]} envelope. I opted to document rather than reject the combination — the NDJSON is already machine-readable, and rejecting a harmless flag combo felt more surprising than clarifying it.

3. Dead expectJsonError helper — removed. It had no remaining callers after the exit-5 tool_not_found change, so I dropped it from assertions.ts.

isError→5 behavior-change callout: acknowledged, no action — it's the documented intent and the README calls it out.

npm run ci is green. (This branch also carries the parent's #1651 comment fix via a merge from v2/1573-....)

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 1m 46s —— View job

---

Re-review: --app-info probe + --format json envelope

Re-read cli.ts, the new/changed tests, README.md, plus core/mcp/apps.ts and error-handler.ts. This pass focuses on the three follow-up commits since the last review. All three prior findings are correctly resolved, and the fix is clean. ✅

Verification of the applied fixes

1. tools/list --app-info all-or-nothing on a malformed URI — fixed correctly. Moving the tolerance into collectAppInfo (guarding the initial extractAppInfo(tool) at cli.ts:433-441) is the better call than a loop-local try/catch — the NDJSON loop (cli.ts:195-202) is now inherently per-tool tolerant, and the branch is unit-testable in isolation. The new emit-result.test.ts:111 case drives a non-ui:// resourceUri and asserts {hasApp:false, resourceError} with readResource never reached — exactly the branch that was previously unreachable. Confirmed against core/mcp/apps.ts: getAppResourceUri/extractAppInfo throw only from the tool-side URI parse, so the second extractAppInfo(tool, read.result) at cli.ts:445 (post-success) can't throw — no residual gap.

The documented side effect — tools/call --app-info on a malformed tool now exits 2 (no_app) with a resourceError field instead of a generic exit-1 — is a genuine improvement: the probe reports why it found no usable app.

2. --format json ignored for tools/list --app-info — documented clearly. The README note and the inline comment (cli.ts:190-194) both spell out that the list-probe shape is fixed NDJSON and --format json only reshapes the single-result paths. Documenting rather than rejecting a harmless combo is the right call. Agreed.

3. Dead expectJsonError — removed. Gone from both assertions.ts and the tools.test.ts import; the "nonexistent tool" case is rewritten to the exit-5 tool_not_found contract.

Minor observations (non-blocking, no action required)

• Exit 0 for an app tool whose UI resource is unreadable. In emitResult (cli.ts:372-386) the no-app throw is gated solely on info.hasApp, so a tools/call --app-info on a tool that advertises a valid ui:// resource but whose resources/read fails returns {hasApp:true, resourceError} and exits 0. A caller branching on exit code alone can't distinguish a fully-successful probe from a partial one — it has to inspect resourceError. This is consistent with the "tolerate, don't abort" design (and documented), so it's defensible; just worth being aware that exit-0 no longer implies a complete probe.
• result = { ...appInfo } (cli.ts:238) is effectively dead data — emitResult uses the sibling appInfo, not result, on the --app-info path. It's only there to satisfy the MethodOutcome "result" variant, so it's harmless; a result: {} would read as more obviously-a-placeholder, but not worth churning.

Coverage note

The emit-result.test.ts/app-info.test.ts split is well-judged: the stdio-fixture e2e tests cover the happy paths and exit codes, and the direct-unit tests cover precisely the branches the fixture can't reach (isError→5, resourceError from a rejected read, non-Error rejection stringify, and now the malformed-URI fold). One branch not directly asserted end-to-end is emitResult with hasApp:true && resourceError → exit 0, but it's fully covered by the info.hasApp branch logic that is tested.

---

Verdict: LGTM. The three findings are resolved, the approach chosen for #1 is better than what I suggested, and the two remaining observations are optional polish. CI (run 29165815634) is still in progress at the time of this review — worth a glance before merge, though the author reports npm run ci green locally. Reminder per the stack note: base is v2/1573-cli-programmatic-ergonomics (#1573), so merge that first.
· branch v2/1574-cli-app-info-format-json

[cliffhall]

Thanks for the re-review. On the two optional observations: I'm leaving both as-is. The exit-0-with-resourceError case is the intended "tolerate, don't abort" behavior (a caller that needs probe completeness inspects resourceError), and result = { ...appInfo } on the --app-info path is just the MethodOutcome "result" placeholder — emitResult reads the sibling appInfo, so it's inert. Not worth the churn. No further changes on this PR.