Skip to content

fix(opencode): handle cancel in upgrade process and customize spinner#26150

Open
mortalYoung wants to merge 2 commits intoanomalyco:devfrom
mortalYoung:fix/upgrade
Open

fix(opencode): handle cancel in upgrade process and customize spinner#26150
mortalYoung wants to merge 2 commits intoanomalyco:devfrom
mortalYoung:fix/upgrade

Conversation

@mortalYoung
Copy link
Copy Markdown

@mortalYoung mortalYoung commented May 7, 2026

Issue for this PR

Closes #26154

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

The upgrade command had two issues:

  1. Cancel not handled: When the user pressed Ctrl+C or dismissed the confirmation prompt, prompts.isCancel() was not checked. The cancelled value was falsy but only a bare !install check was used — this works incidentally but prompts.isCancel() should be checked explicitly to correctly handle the cancel signal returned by @clack/prompts.

  2. Default spinner frames: The default spinner frames use characters in the range U+25D0–U+25D3 (◐◓◑◒), which are classified as "Neutral" width in the Unicode standard — meaning each font decides their rendered size independently. Many monospace fonts (Menlo, SF Mono, Fira Code, etc.) render these glyphs inconsistently: some frames appear slightly wider or taller than others, causing visible jitter in the animation. The Braille dot characters (⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏) are classified as "Narrow" width and render at a consistent single column width across fonts, producing a stable animation.

How did you verify your code works?

Tested locally by running bun dev upgrade, pressing Ctrl+C at the confirmation prompt and confirming it exits cleanly with "Done". Also verified the spinner renders correctly during the upgrade process.

Screenshots / recordings

N/A — CLI-only change, no UI.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actions github-actions Bot added needs:compliance This means the issue will auto-close after 2 hours. needs:issue labels May 7, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions github-actions Bot removed needs:issue needs:compliance This means the issue will auto-close after 2 hours. labels May 7, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Thanks for updating your PR! It now meets our contributing guidelines. 👍

Copy link
Copy Markdown

@kowanietz kowanietz left a comment

Choose a reason for hiding this comment

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

I also experience the spinner jitter. Might be worth looking into centralizing it


prompts.log.info(`From ${InstallationVersion} → ${target}`)
const spinner = prompts.spinner()
const spinner = prompts.spinner({ frames: ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"] })
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This fixes the spinner jitter for upgrade, but I noticed other CLI commands still call prompts.spinner() directly. Would it make sense to move this spinner config into the existing helper for consistency?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ok. I defined createSpinner function and put it into a new file named prompt.ts as a result of I can't find an appropriate existing helper file.

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.

upgrade spinner flush

2 participants