Skip to content

Conversation

@thebristolsound
Copy link
Contributor

@thebristolsound thebristolsound commented Sep 1, 2025

Fix Windows prompt cursor positioning issue with ANSI escape sequences

Overview

This PR addresses a Windows-specific bug where ANSI escape sequences in the Goose CLI prompt cause incorrect cursor positioning and visual artifacts. This is a follow-up to the draft PR #3989 originally started by another contributor, with a refined implementation focused on simplicity and reliability.

Problem Description

On Windows, the Goose CLI prompt uses ANSI escape sequences for styling (cyan and bold colors):

let prompt = format!("{} ", console::style("( O)>").cyan().bold());

However, rustyline on Windows has a known issue with ANSI escape sequence width calculation that causes:

  • Cursor positioning to be offset from the actual text
  • Visual artifacts when editing commands
  • A poor user experience with input handling

Root Cause

This is a known issue in the rustyline library where ANSI escape sequences are not properly accounted for in width calculations on Windows terminals. The issue affects how rustyline positions the cursor relative to the displayed text.

Related Issues:

Solution

This PR implements a Windows-specific workaround with a simple, direct approach:

Platform-Specific Prompt Generation: The get_input_prompt_string() function uses a compile-time cfg! check to determine the operating system:

  • Windows: Returns plain text prompt "( O)> " without ANSI escape sequences
  • Other platforms: Returns styled prompt with ANSI colors (preserving existing behavior)

Code Changes

  • Simple Implementation: Uses cfg!(target_os = "windows") directly in the prompt generation function to conditionally return styled or plain text
  • Comprehensive Comments: Added detailed explanation of the Windows-specific workaround and references to upstream issues
  • Platform-Specific Testing: Updated unit tests to use conditional compilation (#[cfg]) to test only the behavior that runs on each platform, avoiding CI/CD environment issues

Testing

The test_get_input_prompt_string test uses conditional compilation to validate platform-specific behavior:

  • On Windows builds: Tests that the prompt is plain text without ANSI codes
  • On non-Windows builds: Tests that the prompt contains ANSI styling
  • Both: Verify basic structure and formatting

This approach ensures tests are reliable across different environments and CI/CD systems.

Future Considerations

This workaround should be removed once the repository updates to a version of rustyline that includes the fix from kkawakam/rustyline#890 and the fix has been verified to resolve the cursor positioning issues on Windows.

Credit

This work builds upon the draft PR #3989 originally created by another contributor. The implementation has been simplified for reliability and maintainability. #3989


Before screenshot:
image

After (fixed):
image

Screenshots taken in Windows 11 Terminal Preview, PowerShell 7

@thebristolsound thebristolsound force-pushed the bugfix/fix-windows-cursor-shift branch from 084d9bc to 2e6ee5e Compare September 1, 2025 02:28
@thebristolsound thebristolsound changed the title fix: Fix Windows prompt cursor positioning issue with ANSI escape sequences fix: Windows prompt cursor positioning issue with ANSI escape sequences Sep 1, 2025
@thebristolsound
Copy link
Contributor Author

Bump

Copy link
Collaborator

@michaelneale michaelneale left a comment

Choose a reason for hiding this comment

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

looks good to me, can't test on windows myself but downside risk is low. And I learned that you can use "not" in platform macros, very cool! thanks @thebristolsound

@thebristolsound
Copy link
Contributor Author

Thanks! Taking a look at these test failures, they were passing locally but something looks a bit off.

@thebristolsound thebristolsound force-pushed the bugfix/fix-windows-cursor-shift branch 2 times, most recently from cdf5e7c to 4b47fd8 Compare September 11, 2025 03:58
@thebristolsound thebristolsound requested a review from a team as a code owner September 11, 2025 03:58
Copilot AI review requested due to automatic review settings September 11, 2025 04:01
@thebristolsound thebristolsound force-pushed the bugfix/fix-windows-cursor-shift branch from 4b47fd8 to e18f9cd Compare September 11, 2025 04:01
Copy link
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 fixes a Windows-specific bug where ANSI escape sequences in the Goose CLI prompt cause incorrect cursor positioning and visual artifacts in terminals. The solution implements platform-specific prompt generation to use plain text on Windows while preserving styled prompts on other platforms.

  • Extracted prompt generation into a reusable get_input_prompt_string() function with conditional compilation
  • Added Windows-specific workaround to use plain text prompts without ANSI escape sequences
  • Included comprehensive unit tests to verify platform-specific behavior and prompt consistency

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@thebristolsound thebristolsound force-pushed the bugfix/fix-windows-cursor-shift branch from e679c9c to c53456a Compare September 11, 2025 04:08
Copy link
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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@michaelneale
Copy link
Collaborator

thanks @thebristolsound you will need to also follow instructions for the DCO Check: https://github.com/block/goose/pull/4464/checks?check_run_id=50117009096

@thebristolsound thebristolsound force-pushed the bugfix/fix-windows-cursor-shift branch 4 times, most recently from a695b3f to 3a10238 Compare September 13, 2025 20:55
Copy link
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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@thebristolsound thebristolsound force-pushed the bugfix/fix-windows-cursor-shift branch from 8a1ac80 to d3ccb4a Compare September 13, 2025 20:59
@thebristolsound thebristolsound force-pushed the bugfix/fix-windows-cursor-shift branch 3 times, most recently from f390049 to 0465c08 Compare September 13, 2025 22:39
Matt Donovan and others added 3 commits September 13, 2025 15:39
Signed-off-by: Matt Donovan <[email protected]>

Update crates/goose-cli/src/session/input.rs

Co-authored-by: Copilot <[email protected]>
Signed-off-by: Matt Donovan <[email protected]>

Update input.rs

Signed-off-by: Matt Donovan <[email protected]>
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Matt Donovan <[email protected]>

Remove useless unit test

Signed-off-by: Matt Donovan <[email protected]>
…(terminal vs plaintext)

Signed-off-by: Matt Donovan <[email protected]>
@thebristolsound thebristolsound force-pushed the bugfix/fix-windows-cursor-shift branch 2 times, most recently from c7e4381 to ff2bf74 Compare September 13, 2025 22:48
Copy link
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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@thebristolsound thebristolsound force-pushed the bugfix/fix-windows-cursor-shift branch from ff2bf74 to 68f0c6b Compare September 13, 2025 22:55
Copy link
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

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


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@thebristolsound thebristolsound force-pushed the bugfix/fix-windows-cursor-shift branch from 68f0c6b to adfee2d Compare September 13, 2025 23:02
@thebristolsound
Copy link
Contributor Author

Okay, sorry for the notification spam on this one, but I think we're all green in the pipeline now. I've re-requested review as I made some minor adjustments to the unit tests in this PR. Let me know if anything else comes up or you want additional changes. cc @michaelneale

@thebristolsound
Copy link
Contributor Author

bump. Any chance we can get this merged in?

@DOsinga
Copy link
Collaborator

DOsinga commented Sep 30, 2025

yes!

@DOsinga DOsinga merged commit af40917 into block:main Sep 30, 2025
10 checks passed
zanesq added a commit that referenced this pull request Sep 30, 2025
…-unification

* 'main' of github.com:block/goose: (24 commits)
  feat(cli): add `path` & `limit` to `session list` command (#4878)
  Allow better concurrent access (#4896)
  fix: Windows prompt cursor positioning issue with ANSI escape sequences (#4464)
  Fix: LiteLLM API key field not showing in UI configuration (#4105)
  fix: path is duplicated on tool calls causing them to fail (#4658) (#4859)
  add new prompt to get all available tutorials (#4802)
  Add filtering for agentVisible: false messages on streaming providers (#4847)
  alexhancock/mcp-crate-cleanup (#4885)
  docs: rename sub-recipe to subrecipe (#4886)
  docs: new multi-model section with autopilot topic (#4864)
  make agent manager singleton (#4880)
  Cli web auth token (#4456)
  fix(token_counter): fix panic with GitHub Copilot (#4632)
  Revert "Internal MCP Crate Cleanup (#4800)" (#4883)
  remove 2 redundant comments and one that lies (#4866)
  Internal MCP Crate Cleanup (#4800)
  Fix #4612: Return non-zero exit code when CLI session ends with error (#4621)
  Dead code cleanup (#4873)
  fix: restoring test data and correcting name (#4875)
  Add .goosehints file to enforce lowercase branding in documentation (#4870)
  ...
matt-wirth added a commit to LiquidMetal-AI/goose that referenced this pull request Sep 30, 2025
* remove only-pr-labels (block#4842)

Signed-off-by: Angela Ning <[email protected]>

* Docs: Add link to Plug & Play video for Reddit MCP (block#4852)

* Fix: Token count UI doesn't re-render if it's open. (block#4822)

* Update databricks flash model (block#4836)

* Session manager (block#4648)

Co-authored-by: Douwe Osinga <[email protected]>

* Add Hacktoberfest Guides (block#4830)

Co-authored-by: taniashiba <[email protected]>

* docs: goose x Hacktoberfest 2025 Blog (block#4855)

Co-authored-by: Tania Chakraborty <[email protected]>
Co-authored-by: Angie Jones <[email protected]>

* fix: delete some flaky and not-useful tests (block#4861)

* can tell the system what shell it is using (block#4807)

* new subrecipe blog post banner (block#4862)

* docs: remove recipe generator link from next to extension search (block#4858)

* lowercase g in goose (block#4832)

* doc: file parameter recipe update (block#4594)

* Fiie input recipe ref doc (block#4869)

* Add .goosehints file to enforce lowercase branding in documentation (block#4870)

Co-authored-by: Angie Jones <[email protected]>

* fix: restoring test data and correcting name (block#4875)

* Dead code cleanup (block#4873)

Co-authored-by: Douwe Osinga <[email protected]>
Co-authored-by: Michael Neale <[email protected]>

* Fix block#4612: Return non-zero exit code when CLI session ends with error (block#4621)

Signed-off-by: jalateras <[email protected]>

* Internal MCP Crate Cleanup (block#4800)

* remove 2 redundant comments and one that lies (block#4866)

Co-authored-by: Douwe Osinga <[email protected]>

* Revert "Internal MCP Crate Cleanup (block#4800)" (block#4883)

* fix(token_counter): fix panic with GitHub Copilot (block#4632)

Signed-off-by: sings-to-bees-on-wednesdays <[email protected]>

main was broken. this seems important

* Cli web auth token (block#4456)

Signed-off-by: Evanfeenstra <[email protected]>

* make agent manager singleton (block#4880)

* docs: new multi-model section with autopilot topic (block#4864)

* docs: rename sub-recipe to subrecipe (block#4886)

* alexhancock/mcp-crate-cleanup (block#4885)

* Temporary workaround for mcp server

* Add filtering for agentVisible: false messages on streaming providers (block#4847)

* add new prompt to get all available tutorials (block#4802)

Signed-off-by: AdemolaAri <[email protected]>

* Fix for auth in extension, fix for stale keychain

* fix: path is duplicated on tool calls causing them to fail (block#4658) (block#4859)

Signed-off-by: demetrio108 <[email protected]>

* Fix: LiteLLM API key field not showing in UI configuration (block#4105)

Signed-off-by: jalateras <[email protected]>
Co-authored-by: Ebony Louis <[email protected]>
Co-authored-by: Claude <[email protected]>
Co-authored-by: Jack Amadeo <[email protected]>

* fix: Windows prompt cursor positioning issue with ANSI escape sequences (block#4464)

Signed-off-by: Matt Donovan <[email protected]>
Co-authored-by: Matt Donovan <[email protected]>

* Allow better concurrent access (block#4896)

Co-authored-by: Douwe Osinga <[email protected]>

* feat(cli): add `path` & `limit` to `session list` command (block#4878)

* Added CMD+T keyboard shortcut that takes you to the Home tab (block#4541)

Signed-off-by: Guillaume Simard <[email protected]>

---------

Signed-off-by: Angela Ning <[email protected]>
Signed-off-by: jalateras <[email protected]>
Signed-off-by: Evanfeenstra <[email protected]>
Signed-off-by: AdemolaAri <[email protected]>
Signed-off-by: demetrio108 <[email protected]>
Signed-off-by: Matt Donovan <[email protected]>
Signed-off-by: Guillaume Simard <[email protected]>
Co-authored-by: Angela Ning <[email protected]>
Co-authored-by: Emma Youndtsmith <[email protected]>
Co-authored-by: David Katz <[email protected]>
Co-authored-by: Douwe Osinga <[email protected]>
Co-authored-by: Douwe Osinga <[email protected]>
Co-authored-by: Ebony Louis <[email protected]>
Co-authored-by: taniashiba <[email protected]>
Co-authored-by: taniandjerry <[email protected]>
Co-authored-by: Tania Chakraborty <[email protected]>
Co-authored-by: Angie Jones <[email protected]>
Co-authored-by: Jack Amadeo <[email protected]>
Co-authored-by: Michael Neale <[email protected]>
Co-authored-by: w. ian douglas <[email protected]>
Co-authored-by: Alex Hancock <[email protected]>
Co-authored-by: Jarrod Sibbison <[email protected]>
Co-authored-by: Rizel Scarlett <[email protected]>
Co-authored-by: Jim Alateras <[email protected]>
Co-authored-by: sings-to-bees-on-wednesdays <[email protected]>
Co-authored-by: Evan Feenstra <[email protected]>
Co-authored-by: Yingjie He <[email protected]>
Co-authored-by: dianed-square <[email protected]>
Co-authored-by: Ademola Arigbabuwo <[email protected]>
Co-authored-by: Demetrio ⚡️ <[email protected]>
Co-authored-by: Claude <[email protected]>
Co-authored-by: Jack Amadeo <[email protected]>
Co-authored-by: Matt Donovan <[email protected]>
Co-authored-by: Matt Donovan <[email protected]>
Co-authored-by: Robert Mcgregor <[email protected]>
Co-authored-by: Guillaume Simard <[email protected]>
wpfleger96 added a commit to wpfleger96/goose that referenced this pull request Oct 1, 2025
* main: (206 commits)
  Tiny: fix github casing  (block#4903)
  remove anyOf from create_task tool (block#4897)
  chore(deps): bump tracing-subscriber from 0.3.19 to 0.3.20 (block#4442)
  fix optional recipe schema zod validation (block#4900)
  Added CMD+T keyboard shortcut that takes you to the Home tab (block#4541)
  feat(cli): add `path` & `limit` to `session list` command (block#4878)
  Allow better concurrent access (block#4896)
  fix: Windows prompt cursor positioning issue with ANSI escape sequences (block#4464)
  Fix: LiteLLM API key field not showing in UI configuration (block#4105)
  fix: path is duplicated on tool calls causing them to fail (block#4658) (block#4859)
  add new prompt to get all available tutorials (block#4802)
  Add filtering for agentVisible: false messages on streaming providers (block#4847)
  alexhancock/mcp-crate-cleanup (block#4885)
  docs: rename sub-recipe to subrecipe (block#4886)
  docs: new multi-model section with autopilot topic (block#4864)
  make agent manager singleton (block#4880)
  Cli web auth token (block#4456)
  fix(token_counter): fix panic with GitHub Copilot (block#4632)
  Revert "Internal MCP Crate Cleanup (block#4800)" (block#4883)
  remove 2 redundant comments and one that lies (block#4866)
  ...
HikaruEgashira pushed a commit to HikaruEgashira/goose that referenced this pull request Oct 3, 2025
…es (block#4464)

Signed-off-by: Matt Donovan <[email protected]>
Co-authored-by: Matt Donovan <[email protected]>
Signed-off-by: HikaruEgashira <[email protected]>
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.

4 participants