refactor: migrate non-streaming commands to CommandOutput with markdown rendering#398
Merged
refactor: migrate non-streaming commands to CommandOutput with markdown rendering#398
Conversation
Convert auth/status from imperative log.info/success/error calls to the
structured CommandOutput<T> return pattern with a dedicated markdown
formatter.
Changes:
- Define AuthStatusData type for structured status information
- Add formatAuthStatus() human formatter using mdKvTable, colorTag, and
renderMarkdown for rich terminal output
- Command func() now returns { data } instead of writing to logger
- Verification errors captured in data rather than thrown
- JSON output includes all status fields in a consistent shape
- Tests updated to assert on stdout (rendered output) instead of stderr
auth/logout: - Returns LogoutResult data with loggedOut and configPath fields - Not-authenticated and env-token-active cases now throw typed errors (AuthError) instead of log.warn, matching auth/status pattern - Human formatter renders success message with markdown cli/feedback: - Returns FeedbackResult data with sent and message fields - Telemetry-disabled case throws ConfigError with suggestion instead of log.warn, so the central error handler formats it consistently - Human formatter shows ✓ or ⚠ based on send success Both commands now support --json and --fields via the output config.
Convert cli/fix from 20+ imperative log calls to structured FixResult data returned via CommandOutput. Changes: - Define FixIssue and FixResult types for structured diagnostic output - All handler functions (ownership, permissions, schema) now return arrays of FixIssue objects instead of logging directly - Add formatFixResult() human formatter with per-category sections, status markers (✓/✗/•), and instructions blocks - Use OutputError for failure cases (renders data then exits code 1) - --json and --fields flags now supported automatically - Tests updated to assert on rendered stdout instead of consola stderr
Convert cli/upgrade final-result messages to structured UpgradeResult
data returned via CommandOutput, while keeping transient progress
messages as log.info to stderr.
Changes:
- Define UpgradeResult type with action, versions, channel, method, and
warnings fields
- resolveTargetVersion returns discriminated union ResolveResult instead
of string|null, enabling early-return paths to carry structured data
- migrateToStandaloneForNightly returns warnings[] instead of logging
- formatUpgradeResult() human formatter with ✓/⚠ markers and exhaustive
action switch
- All code paths return { data: UpgradeResult }
- --json and --fields flags now supported automatically
Contributor
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Init
Issue List
Other
Bug Fixes 🐛Init
Other
Internal Changes 🔧Init
Other
Other
🤖 This preview updates automatically when you update the PR. |
Contributor
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 98.13%. Project has 917 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 95.50% 95.49% -0.01%
==========================================
Files 142 142 —
Lines 20037 20343 +306
Branches 0 0 —
==========================================
+ Hits 19136 19426 +290
- Misses 901 917 +16
- Partials 0 0 —Generated by Codecov Action |
![cursor[bot]](https://avatars.gh.lumenfield.work/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.gh.lumenfield.work/in/12637?s=60&v=4){kind=small}
A no-op logout when not authenticated is more user-friendly than an
error. Matches the previous behavior where exit code was 0.
- Not-authenticated: returns { loggedOut: false, message } (exit 0)
- Env-token-active: still throws AuthError (can't proceed)
- Formatter handles both loggedOut:true and loggedOut:false cases
…ead code 1. cli/fix.ts: repairOwnership now returns RepairOutcome[] aligned by index with the input issues array, fixing the bug where mixed success/failure results were misaligned via separate fixed[]/failed[] arrays. 2. cli/upgrade.ts + human.ts: Remove unreachable 'channel-only' action variant from UpgradeResult.action union and formatter — no code path produces it. Can be re-added when the feature is implemented.
…/schema handlers Permission handler: - repairPermissions now returns RepairOutcome[] aligned by index - Dir-first ordering preserved via index maps (original issue indices tracked through reorder) - Remove dead collectResults function Schema handler: - Keep original fixIssues from getSchemaIssues instead of building new objects from repair strings - Mark all issues based on overall repair success/failure (schema repair runs independently of detection, so 1:1 mapping isn't possible)
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.
Move RepairOutcome type above the JSDoc so it's no longer wedged between the doc comment and the function. Update @returns to describe the new RepairOutcome[] return shape.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrate 5 commands from imperative
log.info/log.success/log.warnoutput to the structuredCommandOutput<T>return pattern with dedicated markdown formatters. This is a continuation of the output convergence work from PR #394.Commands migrated
auth statusAuthStatusDatamdKvTable+colorTagauth logoutLogoutResultlog.warncli feedbackFeedbackResultConfigErrorwith suggestioncli fixFixResult/FixIssueOutputErrorfor failures (renders data then exits 1)cli upgradeUpgradeResultResolveResultunion. Progress messages kept on stderr; final result returned as dataPattern
Every migrated command follows the same structure:
Human formatters in
src/lib/formatters/human.tsuse the shared markdown helpers (mdKvTable,colorTag,safeCodeSpan,renderMarkdown) for consistent terminal rendering. All commands now automatically support--jsonand--fields.What's NOT in this PR
auth login— Has an interactive OAuth path that needs rawstdout/stderrWriters. Deferred tobuildStreamingCommand().issue list,project list, etc.) — Need streaming infrastructure.apicommand — Intentionally raw (direct API proxy).log.infocalls incli/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/createTest results
917 pass, 0 fail, 30 test files.