refactor: convert Tier 2-3 commands to return-based output and consola#394
refactor: convert Tier 2-3 commands to return-based output and consola#394
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Init
Other
Bug Fixes 🐛Init
Other
Internal Changes 🔧Init
Other
Other
🤖 This preview updates automatically when you update the PR. |
Codecov Results 📊✅ 104 passed | Total: 104 | Pass Rate: 100% | Execution Time: 0ms 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ✅ Patch coverage is 99.66%. Project has 901 uncovered lines. Files with missing lines (3)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 95.50% 95.50% —%
==========================================
Files 142 142 —
Lines 20021 20019 -2
Branches 0 0 —
==========================================
+ Hits 19121 19118 -3
- Misses 900 901 +1
- Partials 0 0 —Generated by Codecov Action |
8cd4b67 to
a9e837a
Compare
a9e837a to
8dcba68
Compare
8dcba68 to
f7f8b0c
Compare
BYK
left a comment
There was a problem hiding this comment.
Addressing review comments
f7f8b0c to
e861376
Compare
Convert remaining non-streaming commands away from direct stdout/stderr writes, continuing the output convergence started in #382. Return-based output (OutputConfig<T>): - project/view, log/view: pure formatter functions, return { data, hint } - org/list, trace/list: return structured data with human formatters - Add jsonTransform to OutputConfig for custom JSON envelopes (trace/list) - Widen buildListCommand to accept OutputConfig alongside "json" Consola logging: - auth/login, logout, status: logger.withTag("auth.*"), log.info/success/warn - cli/feedback, setup, fix, upgrade: logger.withTag("cli.*") - api.ts: remove stderr: Writer param from 4 helper functions Infrastructure: - OutputConfig<T> gains jsonTransform property for envelope wrapping - ListCommandFunction type allows unknown returns All tests updated to spy on process.stderr.write (consola target) instead of mock context stdout/stderr. 26 files changed across src and test.
e861376 to
2e690a8
Compare
…ace' test
The test asserted result.not.toContain('[') on raw formatLogRow()
output, but ANSI escape codes contain '[' characters. This caused a
false failure. The adjacent 'handles missing severity' test correctly
used stripAnsi() — this aligns the two tests.
- org/list: show both truncation notice and tip hint when results are truncated (was either-or due to else-if) - cli/setup: rename local quiet-aware `log` to `emit` and module-level `cmdLog` to `log` to match the naming convention used by all other commands (avoids shadowing confusion) - trace/list: keep empty-page next-page command inline in hint text instead of splitting it across main output and de-emphasized footer
|
All three BugBot LOW findings addressed in 1e6cfd4:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Restore hasMore check in formatTraceListHuman so the primary message says 'No traces on this page.' when more pages exist, consistent with the hint that suggests the next-page command. Previously, the formatter always said 'No traces found.' which contradicted the hint.
…wn rendering (#398) ## Summary Migrate 5 commands from imperative `log.info`/`log.success`/`log.warn` output to the structured `CommandOutput<T>` return pattern with dedicated markdown formatters. This is a continuation of the output convergence work from PR #394. ### Commands migrated | Command | Data Type | Key Change | |---------|-----------|------------| | `auth status` | `AuthStatusData` | Multi-section display (source, user, token, defaults, verification) → `mdKvTable` + `colorTag` | | `auth logout` | `LogoutResult` | Not-authenticated / env-token cases now throw typed errors instead of `log.warn` | | `cli feedback` | `FeedbackResult` | Telemetry-disabled case throws `ConfigError` with suggestion | | `cli fix` | `FixResult` / `FixIssue` | 20+ diagnostic log calls → structured issues array. Uses `OutputError` for failures (renders data then exits 1) | | `cli upgrade` | `UpgradeResult` | Discriminated `ResolveResult` union. Progress messages kept on stderr; final result returned as data | ### Pattern Every migrated command follows the same structure: ```ts // 1. Define data type export type FooResult = { ... }; // 2. Add output config to buildCommand output: { json: true, human: formatFooResult }, // 3. Return data from func() return { data: result }; ``` Human formatters in `src/lib/formatters/human.ts` use the shared markdown helpers (`mdKvTable`, `colorTag`, `safeCodeSpan`, `renderMarkdown`) for consistent terminal rendering. All commands now automatically support `--json` and `--fields`. ### What's NOT in this PR - **`auth login`** — Has an interactive OAuth path that needs raw `stdout`/`stderr` Writers. Deferred to `buildStreamingCommand()`. - **List commands** (`issue list`, `project list`, etc.) — Need streaming infrastructure. - **`api` command** — Intentionally raw (direct API proxy). - **Progress `log.info` calls** in `cli/upgrade` — These are transient status messages on stderr, which is the correct place for them. ### Commands now on return-based `CommandOutput` (19 total) `api`, `auth/refresh`, `auth/whoami`, `auth/status`, `auth/logout`, `auth/token`, `cli/feedback`, `cli/fix`, `cli/upgrade`, `issue/explain`, `issue/plan`, `issue/view`, `event/view`, `log/view`, `trace/view`, `org/view`, `project/view`, `project/create`, `team/create` ### Test results 917 pass, 0 fail, 30 test files. --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…410) ## Summary Migrate `log/list` non-streaming paths from direct stdout writes to the return-based `CommandOutput<T>` pattern used by all other list commands. ### Changes - **New `LogListResult` type**: `{ logs, hint?, traceId? }` — data structure for non-follow log operations - **`executeSingleFetch`**: Returns `LogListResult` instead of writing to stdout - **`executeTraceSingleFetch`**: Returns `LogListResult` (replaces `displayTraceLogs` call) - **`formatLogListHuman`**: Renders logs as formatted table with trace-ID column awareness - **`jsonTransformLogList`**: Applies per-element field filtering for `--fields` flag - **Output config**: Changed from `"json"` to full `OutputConfig<LogListResult>` - **`func()` return type**: `CommandOutput<LogListResult> | void` — non-follow returns data, follow returns void ### Not changed - `executeFollowMode`, `FollowConfig`, `writeLogs` — streaming infrastructure stays as-is (will be migrated with `buildStreamingCommand`) ### Part of Phase 6b of the output convergence effort. All list commands now return data structures for non-streaming paths: - ✅ org/list, trace/list (PR #394) - ✅ team/list, repo/list, project/list, issue/list (PR #404) - ✅ **log/list non-follow (this PR)** - 🔜 log/list --follow, auth/login (Phase 6c: `buildStreamingCommand`)
Summary
Continues the output convergence from #382, converting Tier 2-3 commands away from direct
stdout.write/stderr.writeto either return-basedOutputConfig<T>or consola logging.Changes
Return-based output (
OutputConfig<T>with{ data, hint? }returns):project/view,log/view— pure formatter functions, structured return valuesorg/list,trace/list— return structured data with human formattersjsonTransformproperty onOutputConfig<T>for custom JSON envelopes (used bytrace/listfor{ data, hasMore, nextCursor? }shape)buildListCommandwidened to acceptOutputConfigalongside"json"Consola logging (replaces
stdout.write/stderr.write):auth/login,auth/logout,auth/status—logger.withTag("auth.*")cli/feedback,cli/setup,cli/fix,cli/upgrade—logger.withTag("cli.*")api.ts— removedstderr: Writerparameter from 4 helper functionsTests: All updated to spy on
process.stderr.write(consola output target) instead of mock contextstdout/stderr. 26 files changed.Not converted (intentional)
auth/token— designed for pipe capture (sentry auth token | pbcopy)interactive-login.ts— deeply coupled Writer threading, deferred to separate PRissue/list,project/list,log/list) — next PR