feat(cli): add post-exec status, fix interactive flag, show stderr#35
feat(cli): add post-exec status, fix interactive flag, show stderr#35pablopunk wants to merge 2 commits into
Conversation
WalkthroughThe CLI adds shared status helpers for install, uninstall, link, import/export, and hook operations. Interactive mode now reports success, failure, dry-run, and skipped states, tracks non-dry-run failures, and always prints completion output. Direct mode receives consistent status output and unconditional completion output. Installer failures now print trimmed subprocess stderr regardless of verbose mode, and interactive TTY detection preserves terminal input. Tests cover status formatting, failure output, and interactive behavior. Sequence Diagram(s)sequenceDiagram
participant CLI as main()
participant Installer as installComponent/uninstallComponent
participant Subprocess
participant Output as stdout/stderr
CLI->>Installer: execute component operation
Installer->>Subprocess: run command
Subprocess-->>Installer: return result and stderr
Installer->>Output: print stderr when the command fails
Installer-->>CLI: return operation result
CLI->>Output: print operation status
CLI->>Output: print failure count when needed
CLI->>Output: print Done.
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/feedback.test.ts (1)
176-233: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInteractive feedback tests simulate output rather than exercising the actual code path.
These tests call
process.stdout.write/process.stderr.writedirectly to verify the format, but don't invokemain()with mocked dependencies. They pass even if the interactive mode implementation diverges from the simulated strings. Consider mockingrunInteractive,installComponent, etc. and callingmain()to verify end-to-end behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/feedback.test.ts` around lines 176 - 233, Replace the simulated stdout/stderr writes in the “interactive mode feedback patterns (Change 1)” tests with end-to-end tests that invoke main() using mocked runInteractive, installComponent, and other required dependencies. Assert the actual output produced by main(), including Done. formatting, failure count behavior, colors, and output streams, so the tests detect divergences in the implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/index.ts`:
- Around line 324-326: In the direct-mode results handling loop, avoid adding
the same component multiple times when several links fail. Replace the
per-result push logic in the `for (const r of results)` block with a single
`results.some(r => r.failed && !r.dryRun)` check, pushing `name` only when at
least one qualifying failure exists, matching the interactive-mode behavior.
---
Nitpick comments:
In `@tests/feedback.test.ts`:
- Around line 176-233: Replace the simulated stdout/stderr writes in the
“interactive mode feedback patterns (Change 1)” tests with end-to-end tests that
invoke main() using mocked runInteractive, installComponent, and other required
dependencies. Assert the actual output produced by main(), including Done.
formatting, failure count behavior, colors, and output streams, so the tests
detect divergences in the implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 23ed5d20-dcef-4b62-8b7a-f498aae8b19e
📒 Files selected for processing (7)
.task-artifacts/002-plan.md.task-artifacts/004-implement.md.task-artifacts/005-implement.mdsrc/index.tssrc/installer.tstests/feedback.test.tstests/installer.test.ts
| for (const r of results) { | ||
| if (r.failed && !r.dryRun) failures.push(name); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Direct mode link handling inflates failure count.
failures.push(name) is called once per failed LinkResult, so a component with multiple failed links gets counted multiple times. Interactive mode (lines 173–174) correctly uses results.some() to push only once.
🐛 Proposed fix: use `some()` like interactive mode
if (comp.hasLinks) {
const results = createLinks(name, comp.link, process.cwd(), options);
- for (const r of results) {
- if (r.failed && !r.dryRun) failures.push(name);
- }
+ const anyFailed = results.some((r) => r.failed && !r.dryRun);
+ if (anyFailed) failures.push(name);
printLinkStatus(name, results);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const r of results) { | |
| if (r.failed && !r.dryRun) failures.push(name); | |
| } | |
| const anyFailed = results.some((r) => r.failed && !r.dryRun); | |
| if (anyFailed) failures.push(name); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/index.ts` around lines 324 - 326, In the direct-mode results handling
loop, avoid adding the same component multiple times when several links fail.
Replace the per-result push logic in the `for (const r of results)` block with a
single `results.some(r => r.failed && !r.dryRun)` check, pushing `name` only
when at least one qualifying failure exists, matching the interactive-mode
behavior.
1 participant
Summary
Issue #34 reported that
dotcommands produce no output on success in non-verbose mode, leaving users unsure whether operations completed.Changes
✓ name,✗ name,~ name,- name) for every operation, plus a✓ Done.summary line. Failures are counted and reported to stderr.--verboseguard on error output ininstallComponent()anduninstallComponent(). If a subprocess fails, its stderr is now always printed, matching the behavior already present in hooks.options.interactiveflag: The TUI mode was incorrectly settinginteractive: falsefor subprocesses (the flag wasisTty && args.mode === "direct"). Changed toisTtyso that interactive installers (brew, password prompts, etc.) receive terminal input when launched from the TUI.tests/feedback.test.tswith 10 unit tests covering all visual states of the newprintResultStatusandprintLinkStatusfunctions.These changes make
dot's output behavior consistent between interactive and direct modes, and ensure users always see what happened after running a command.Summary by CodeRabbit
New Features
Bug Fixes
Tests