Skip to content

feat: improve TUI spinner and fix publish workflow#21

Merged
utkarsh232005 merged 1 commit into
KDM-cli:mainfrom
utkarsh232005:feat/tui-spinner
May 14, 2026
Merged

feat: improve TUI spinner and fix publish workflow#21
utkarsh232005 merged 1 commit into
KDM-cli:mainfrom
utkarsh232005:feat/tui-spinner

Conversation

@utkarsh232005
Copy link
Copy Markdown
Member

@utkarsh232005 utkarsh232005 commented May 14, 2026

This PR introduces several improvements to the TUI spinner and hardens the publish workflow.

Key changes:

  • Restricted the 'publish' workflow to only run on the 'main' branch to prevent tag conflicts.
  • Improved error handling and status reporting in the root command and tests.
  • Fixed an issue where the publish workflow failed due to an existing tag by ensuring a consistent state.
  • Enhanced test coverage for spinner lifecycle events.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed CLI connection check to properly report connection failures and exit with appropriate status codes (1 for failure, 0 for success) for improved error handling
  • Chores

    • Updated publish workflow configuration to execute exclusively on the main branch
    • Enhanced test suite with improved spinner mock implementations and additional lifecycle assertions for comprehensive command validation

Review Change Stack

Copilot AI review requested due to automatic review settings May 14, 2026 11:24
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 14, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Root command now properly exits with code 1 on connection-check failures (code 0 on success). Spinner test mocks are updated with fail method support. Show command tests enhanced to assert spinner lifecycle behavior across error paths. Publish workflow restricted to main branch.

Changes

Error Handling and Exit Codes

Layer / File(s) Summary
Root command error tracking and exit codes
src/commands/root.ts
hadError flag tracks connection-check failures; catch block sets flag and fails spinner; finally block exits with code 1 on error, code 0 on success, while preserving help output.
Spinner interface alignment in test mocks
src/__tests__/health.test.ts, src/__tests__/logs.test.ts, src/__tests__/show.test.ts
Health and logs test mocks extended with fail: vi.fn().mockReturnThis() for chaining; show test imports createSpinner to enable spinner lifecycle assertions.
Show command spinner lifecycle assertions
src/__tests__/show.test.ts
Test assertions added for showRunners Kubernetes-failure path (spinner creation, start, warn with "Some runners could not be fetched"); showPods Kubernetes-failure path asserts spinner fail with fetch-failure message.

Publish Workflow Guard

Layer / File(s) Summary
Publish job main-branch guard
.github/workflows/publish.yml
Job-level if: github.ref == 'refs/heads/main' condition ensures npm publish and release creation run only on main.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • KDM-cli/kdm-cli#17: The main PR's updates to src/commands/root.ts (tracking connection-check errors and exiting with code 1) build directly on the spinner-based connection check flow introduced in PR #17, and the related test mock/assertion changes align with PR #17's new spinner fail/warn interface.

Poem

🎯 Exit codes now truthfully tell,
When spinners fail, when runners excel.
Main branch guards publish's way,
Tests assert the happy and dismay. 🎪

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the two main objectives: TUI spinner improvements (test mocks updated with fail method, enhanced assertions) and publish workflow fix (restricted to main branch). It is concise and directly reflects the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves CLI status behavior around connection failures and spinner lifecycle validation, while constraining the publish workflow to releases from main.

Changes:

  • Root command now exits with a non-zero status when connection checks throw.
  • Spinner mocks/tests now cover fail/warn lifecycle paths.
  • Publish workflow is restricted to the main branch.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/commands/root.ts Tracks connection-check errors and exits with status 1 on failure.
src/__tests__/show.test.ts Adds assertions for spinner lifecycle behavior in show command error paths.
src/__tests__/logs.test.ts Adds fail to the spinner mock used by logs command tests.
src/__tests__/health.test.ts Adds fail to the spinner mock used by health command tests.
.github/workflows/publish.yml Limits the publish job to refs/heads/main.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@utkarsh232005 utkarsh232005 merged commit 3c8001e into KDM-cli:main May 14, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants