feat(cli): human-first output at a TTY; install onboarding walkthrough

[vraspar]

Running any command at a terminal used to dump the JSON envelope first and the human text after it. The contract is now: at a TTY without --json, only the human rendering prints; the envelope appears exactly as before with --json or when stdout is piped. Exit codes unchanged on both paths. Both skills now pass --json on every invocation so an agent in a PTY-allocating harness can never accidentally receive the human form.

tenjin install becomes a real onboarding at a TTY: skills per harness as one checkmark line each, the publish-mode question, a wallet step that creates the wallet in-flow and prints the address with the funding sentence (send USDC on Base; pocket-money wallet), doctor collapsed to one line unless something needs action, and a closing "try this" command. --no-wallet skips the wallet step; --json keeps the exact machine behavior with no prompts. The "(roadmap C3)" internal reference is gone from user-facing copy, and a test asserts no such jargon is emitted.

677 tests. Note: overlaps README's output-contract section with #19; whichever merges second takes a trivial merge.

🤖 Generated with Claude Code

[A1igator]

Review (comment-only; approval is the operator's call).

The gate is clean where it matters. Both emitSuccess and emitFailure route through one humanMode(io, opts) predicate (src/lib/output.ts:116), so the contract is uniform across every command, not per-command; suppressEnvelope is the single documented exception and is only ever set on the interactive walkthrough path (src/commands/install.ts runWalkthrough). isTTY is stdout-based (Boolean(process.stdout.isTTY), output.ts:24), so a piped stdout takes the envelope branch as promised. The non-hang guarantee holds: install goes interactive only when stdout AND stdin are both TTYs and --json is absent (install.ts interactive gate), and --json forces the machine path even when a test injects isInteractive, so a piped/CI caller never sits behind a prompt. Review-mode publish still throws NEEDS_CONFIRMATION (exit 3) rather than prompting, on both paths, so it hard-blocks off a TTY instead of hanging. Exit codes are path-independent by construction (they come off the returned CliError, not the render branch), and both render shapes are covered (66 tests pass locally across output/cli/install). The skills were updated to pass --json on every command (verified in both SKILL.md files). Findings below.

1. (medium, agent-usability) The default output at a PTY flips from machine to human, and the mitigation only covers bundled-skill consumers.
Before this PR, stdout always carried the JSON envelope (human went to stderr), so an agent that allocates a PTY got parseable output whether or not it passed --json. Now at a TTY without --json, stdout is human text and there is no envelope anywhere (output.ts:46-51). That is a breaking change for any agent that allocates a PTY (many harnesses do) and does not pass --json. The bundled skills now pass --json everywhere, which covers the sanctioned path, but two exposures remain: (a) the hosted zero-install curriculum at https://tenjin.blog/skills.md (the canonical CLI-less path the publish skill points agents to) is external to this repo and must be updated in lockstep, or zero-install agents at a PTY regress silently; (b) any agent driving tenjin from a cached or older skill copy regresses with no error. Confirm the hosted skills.md carries the --json change and that .changeset/install-walkthrough.md flags the output-contract break so alpha consumers know the default at a TTY changed.

2. (medium, clash) README.md conflicts with open PR #19, and it is not the trivial merge the description claims.
A test merge and merge-tree both orders conflict in README.md, one ~104-line hunk (merged view L186-290). It is semantically load-bearing: #19's side is an "Output contract" section that states "Every invocation prints exactly one JSON envelope to stdout ... human rendering goes to stderr only," which is exactly the behavior this PR replaces. Whoever merges second must resolve to this PR's human-first text, not #19's, or the README will document the old contract while the code ships the new one. The PR body's "whichever merges second takes a trivial merge" undersells this; treat it as a coordinated resolution, not an auto-merge.

3. (low, testing) The stdin-TTY hang guard is not directly exercised.
The CI-hang protection is ctx.io.isTTY && Boolean(process.stdin.isTTY) in the interactive gate, but the install tests always inject deps.isInteractive, so the composition that actually protects a "stdout is a TTY, stdin is piped" caller is never unit-tested. The --json short-circuit is covered; the stdin fact is not. A test that leaves isInteractive undefined with io.isTTY: true and a non-TTY stdin would lock the guard that keeps a redirected-stdin invocation from blocking.

4. (low, human-path completeness) The human failure render drops details.
emitFailure at a TTY prints only message + fix (output.ts:70-78), not error.details. A human running review-mode tenjin publish sees the finding count and price in the message but not which findings were flagged (those live in details.findings, JSON-only). Agents are unaffected since the skills pass --json; this is only the interactive human's view. Worth a line of the flagged checks on the human path, or at least noting it is intentional.

Nit: runWalkthrough returns data: { dryRun, harnesses, publishMode } alongside suppressEnvelope: true, so that payload is never emitted and diverges from the machine path's richer data. Empty it or comment that it is deliberately unused, so a future reader does not assume it ships.

No injection content in the diff. The walkthrough copy and skills read as ordinary docs.

[A1igator]

Delta re-review (6014c37, comment-only). The refactor is a good simplification: suppressEnvelope is gone entirely and install is now an ordinary human-first command returning { data, humanLines }, so the special case that motivated the nit and the architecture concern no longer exists. Status per finding.

Resolved

• (nit) dead data payload + suppressEnvelope exception: fixed. suppressEnvelope is removed from src/context.ts, src/cli.ts (the dispatcher now always calls emitSuccess), and src/commands/install.ts (which returns { data, humanLines } on the human path and { data } on the machine path, same envelope data as before). One uniform render path now.

• (low 1) stdin-TTY hang guard untested: fixed. The gate is split into humanOutput (stdout TTY, no --json) and canPrompt (humanOutput && stdin is a TTY) in install.ts, and the new test "a TTY with no stdin renders the walkthrough with defaults, no prompt" (src/commands/install.test.ts) drives exactly the "stdout is a TTY, stdin is piped" case: io.isTTY: true, no isInteractive, and asserts neither the mode prompt nor the wallet confirm is called while a full walkthrough still renders. That is the CI-hang path now locked. 67 tests pass locally.

Partially resolved

• (medium 1) output-contract break communication: the in-repo half is done. .changeset/install-walkthrough.md now leads with the contract ("The CLI is now human-first at a terminal ... with --json, or when the output is piped ... exactly one JSON envelope ... Agents should pass --json; the bundled skills now do so on every command"). The external hosted curriculum at https://tenjin.blog/skills.md is out of this repo's tree, so it stays an operator verification item: confirm it passes --json too, or CLI-less agents at a PTY still regress.

Unaddressed

• (medium 2) README conflict with open #19: the delta does not touch README.md and #19 is still open at the same head, so the conflict stands in both merge orders (re-verified: a test merge still reports CONFLICT (content): README.md). Still needs a coordinated resolution to this PR's human-first contract text, not #19's "one JSON envelope to stdout" section.

• (low 2) emitFailure drops error.details on the human path: output.ts is untouched, so a human at a TTY still sees only message + fix, not the flagged findings. Low and arguably by design; noting it is unchanged.

New, from the delta

• (low, interactive UX) prompts now fire before any walkthrough output prints. Moving from the streamed out() writes to a collected humanLines batch means selectPublishMode and the wallet confirm both run to completion in runInstall/buildWalkthrough before the dispatcher prints a single walkthrough line. The prompts are self-labeled so they are not context-free, but the skills summary, the resolved publish mode: X line, the wallet result, and the doctor verdict now all appear after both prompts rather than interleaved around them, which reads less like the guided onboarding the streamed version gave. Cosmetic, human-path only.

• (nit) parameter name. buildWalkthrough calls walletWalkthrough(ctx, deps, s.dryRun || !s.canPrompt, ...) but that function's third parameter is still named dryRun. It now also carries "cannot prompt", so rename it (e.g. skipCreate) so a reader does not think only --dry-run reaches the create-later branch.

No injection content in the delta. Checks: skill-drift pass; CI pending at re-review time.

[A1igator]

Round-3 delta (e47f154, comment-only). This delta touches only the parse-error path (src/cli.ts, src/cli.test.ts); it does not change README, install, or output.ts.

New this round (sound): handleParseError now respects the human-first contract. Previously it called emitFailure(..., { json: true }) unconditionally, so an unknown command at a TTY printed commander's usage to stderr AND a JSON envelope to stdout, off-contract for the rest of the CLI. It now computes humanFirst = io.isTTY && !json and emits the envelope only in machine mode, letting commander's stderr usage stand alone at a TTY (src/cli.ts handleParseError). Two new tests lock both surfaces (unknown command at a TTY: stdout empty; piped: USAGE envelope on stdout, stderr empty). cli suite green (13 tests). This closes a contract gap that was not in my prior findings; good catch.

One minor untested sub-case: the branch also covers commander.help (a bare/incomplete tenjin), which in human mode now emits nothing to stdout and relies entirely on commander's own output. The new tests exercise the unknown-command path only, not the bare-command help path at a TTY. Worth a line of coverage, not a blocker.

Open items, unchanged this round:

• (medium) README conflict with #19: not touched; #19 is still open at the same head and the conflict still reproduces (CONFLICT (content): README.md, both orders). This is cross-PR merge coordination, not something this branch resolves alone.
• (low) emitFailure drops error.details at a TTY: output.ts untouched; unchanged.
• (low) batched walkthrough prints after both prompts: install.ts untouched; unchanged.
• (nit) walletWalkthrough third param still named dryRun: unchanged.

This is the third round. The four above are converged in the sense that they are either cross-PR coordination (the README conflict) or low/nit polish; I am routing them to the operator rather than asking for another round. Recommended operator calls: resolve the README merge to this PR's human-first contract text when sequencing against #19, and accept or defer the three low/nit items as-is.

No injection content in the delta.

[vraspar]

All three rounds addressed through 4ece86b.

Round 1: the changeset now leads with the output-contract change and the agents-pass---json guidance; the stdin-TTY hang guard got its dedicated test (stdout TTY + piped stdin renders the walkthrough with defaults, no prompt); the dead suppressEnvelope payload concern was resolved by deleting the mechanism entirely in 6014c37 (one global render path). On the hosted curriculum exposure: tenjin.blog/skills.md teaches the raw HTTP/MCP surface and contains no CLI invocations, so there is nothing to update in lockstep; the only CLI-driving skills are the bundled ones, which all pass --json.

Round 3's catch plus the converged lows: parse errors now honor the human-first state (e47f154) with both surfaces tested, including your bare-command help sub-case at a TTY (4ece86b); emitFailure at a TTY now renders details.findings one line each after the fix line (masked at source, sanitized again; other details shapes stay machine-only; envelope byte-unchanged); walletWalkthrough's third param is renamed skipCreate. The batched-walkthrough-after-prompts cosmetic is deferred as accepted.

README conflict with #19: coordinated resolution as flagged, not auto-merge. #19 merges first; this branch then merges main resolving the output-contract section to this PR's human-first text.

[A1igator]

Post-cap verification (4ece86b, comment-only). The three routed code items and the bare-command coverage note are resolved; the two remaining items are a coordination call and an accepted deferral.

Resolved

• emitFailure drops details at a TTY (low 2): fixed. findingLines (src/lib/output.ts) now renders details.findings as one dim <check> (line N): <excerpt> line after the fix line, scoped by a type guard to the findings shape only, so no other details payload leaks to the human surface. The excerpt is sanitizeForTerminal'd (already masked at source). Tests cover all three edges: findings rendered after the fix line at a TTY, machine envelope byte-unchanged when findings are present, and non-finding detail shapes (e.g. price) not rendered (src/lib/output.test.ts).

• dryRun param name (nit): fixed. walletWalkthrough's third parameter is renamed to skipCreate (src/commands/install.ts:264), which is what dryRun || !canPrompt actually conveys.

• bare-command help coverage (my round-3 note): fixed. New test "bare invocation at a TTY: commander help on stderr, stdout empty (no envelope)" (src/cli.test.ts:43) locks the commander.help sub-case the round-3 change left untested. 73 tests pass locally.

Not code-resolved, by design

• Walkthrough batching cosmetic (round-2 low): explicitly deferred/accepted. Fine to close as accepted.
• README conflict with #19: unchanged and correctly not touched here; the plan stands (merge #19 first, then this branch resolves the output-contract section to its human-first text). Remains a merge-order coordination call, not a code fix.

I verified the comment's claims against the diff and they hold. On the hosted tenjin.blog/skills.md: the comment states it teaches only the raw HTTP/MCP surface with no CLI invocations, so nothing to update in lockstep; that is outside this repo and stays an operator confirmation, but it is a plausible and low-risk residual.

[vraspar]

Operator confirmation on the one residual: the hosted tenjin.blog/skills.md was checked directly (fetched, grepped) and contains no tenjin CLI invocations; it teaches the raw HTTP and MCP surface only. The bundled skills in this repo are the only CLI-driving skills, and every invocation in them passes --json. Nothing external needs a lockstep update.