-
Notifications
You must be signed in to change notification settings - Fork 7
Fix 'Press any key to continue' to accept any key, not just Enter #445
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Create local input.WaitForAnyKey() and input.WaitForAnyKeyMessage() functions - Use raw terminal mode like existing AskForConfirm() pattern - Replace all calls to tui.WaitForAnyKey() and tui.WaitForAnyKeyMessage() - Fixes issue where only Enter key worked instead of any key - Tested with space key and letter keys - both work immediately Co-Authored-By: Dhilan Fye <dfye@agentuity.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
WalkthroughDependency updates in go.mod: toolchain bumped to Go 1.25.1, multiple direct and indirect modules upgraded (OpenTelemetry, gRPC, protobuf, golang.org/x/*, testify, go-common), some indirects removed/added (e.g., remove filippo.io/edwards25519, add github.com/cenkalti/backoff/v5). No source files or public APIs changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
internal/input/input.go (2)
25-31: Cooked‑mode fallback should consume the whole line to avoid stray buffered input.Reading a single byte in cooked mode leaves the rest (including newline) in stdin, which can bleed into the next prompt.
Apply this diff:
- oldState, err := term.MakeRaw(int(os.Stdin.Fd())) + oldState, err := term.MakeRaw(int(os.Stdin.Fd())) if err != nil { fmt.Print(message) - buf := make([]byte, 1) - os.Stdin.Read(buf) + // cooked mode: read until newline so nothing leaks into subsequent prompts + br := bufio.NewReader(os.Stdin) + _, _ = br.ReadBytes('\n') return }Add import:
import ( "context" "fmt" + "bufio" "os"
39-43: Arrow/function keys emit multi‑byte sequences; consider draining residual bytes.Reading only one byte can leave extra bytes (e.g., “ESC [ A”) in the buffer, which downstream prompts might observe. If portability allows later, we can non‑blocking‑read a few more bytes after the first ESC to drain common sequences. Not a blocker for this PR.
Please sanity‑test by pressing arrow keys at the prompt, then immediately opening a selection prompt (e.g., a tui.Select) to confirm there’s no spurious input.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cmd/agent.go(2 hunks)cmd/cloud.go(2 hunks)cmd/mcp.go(2 hunks)cmd/project.go(2 hunks)internal/errsystem/console.go(2 hunks)internal/input/input.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-22T18:27:10.386Z
Learnt from: jhaynie
PR: agentuity/cli#313
File: cmd/cloud.go:852-853
Timestamp: 2025-05-22T18:27:10.386Z
Learning: In the agentuity CLI codebase, there's a specific error handling pattern: errsystem is used exclusively for unexpected system errors, while expected error conditions use tui.ShowWarning() and direct os.Exit(1) calls.
Applied to files:
internal/errsystem/console.go
🧬 Code graph analysis (5)
cmd/agent.go (1)
internal/input/input.go (1)
WaitForAnyKey(15-17)
cmd/cloud.go (1)
internal/input/input.go (1)
WaitForAnyKey(15-17)
cmd/mcp.go (1)
internal/input/input.go (1)
WaitForAnyKeyMessage(20-60)
internal/errsystem/console.go (1)
internal/input/input.go (1)
WaitForAnyKeyMessage(20-60)
cmd/project.go (1)
internal/input/input.go (1)
WaitForAnyKeyMessage(20-60)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build and Test (blacksmith-4vcpu-ubuntu-2204)
- GitHub Check: Test CLI Upgrade Path (blacksmith-4vcpu-ubuntu-2204)
- GitHub Check: Test CLI Upgrade Path (windows-latest)
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
cmd/cloud.go (1)
23-23: Switch to internal/input looks good.Import and call-site replacement correctly route to the new raw‑mode implementation.
Also applies to: 97-97
cmd/mcp.go (1)
12-12: Good migration to input.WaitForAnyKeyMessage().The prompt now benefits from raw‑mode reads; no other behavior changes.
Also applies to: 123-123
cmd/project.go (1)
17-17: Project flow now uses raw‑mode keypress; LGTM.Import and call-site change are correct and TTY‑safe.
Also applies to: 424-424
cmd/agent.go (1)
20-20: Agent warnings pause now honors any key; looks good.Minimal, correct change.
Also applies to: 586-586
- Explicitly call term.Restore() before os.Exit(1) in signal handlers - Prevents terminal from being left in raw mode when user presses Ctrl+C - Addresses CodeRabbit review feedback about proper cleanup Co-Authored-By: Dhilan Fye <dfye@agentuity.com>
jhaynie
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of making this change in this project we should instead fix the bug in the tui package of go-common
|
Thanks for the feedback @jhaynie! You're absolutely right - fixing this in the go-common tui package would be the better architectural approach since it would benefit all projects using go-common. I can see two options:
Would you prefer I:
Let me know your preference and I'll proceed accordingly. |
- Updated go-common dependency from v1.0.72 to v1.0.91 (contains merged fix) - Removed internal/input package (CLI-specific workaround) - Reverted all imports back to using tui.WaitForAnyKey() and tui.WaitForAnyKeyMessage() - Verified fix works correctly with any key press (not just Enter) - Cleaned up go.mod dependencies Co-Authored-By: Dhilan Fye <dfye@agentuity.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
go.mod (1)
30-62: Minor tidy: rungo mod tidyto drop any unused or misclassified direct deps.E.g.,
go-git/go-git/v5is listed without// indirectin a secondary block; tidy will normalize.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test CLI Upgrade Path (windows-latest)
- GitHub Check: Analyze (go)
🔇 Additional comments (7)
go.mod (7)
132-139: grpc/protobuf/genproto set updated: run tests touching RPC code to catch wire incompatibilities early.Pseudo‑versions for genproto are fine; ensure generated stubs match these versions.
23-23: Testify bump to v1.11.1: run unit tests to catch any matcher/require API shifts.Low risk, but confirm no deprecation fallout.
36-36: Backoff major version bump (v4 → v5): confirm no lingering v4 imports
Automated search for 'cenkalti/backoff/v4' produced no output; manually verify all Go imports use 'github.com/cenkalti/backoff/v5' and go.mod requires v5 — e.g. run locally: rg -nP --type go 'cenkalti/backoff(/v4)?'.
59-61: No term.MakeRaw/term.Restore found — original raw‑mode concern not applicable.rg showed no calls to term.MakeRaw, term.Restore, or signal.Notify; only isatty.IsTerminal in internal/errsystem/console.go (calls tui.WaitForAnyKeyMessage, ~lines 146–152). If tui.WaitForAnyKeyMessage itself uses x/term.MakeRaw then apply TTY gating and ensure term.Restore via defer + signal handlers; otherwise ignore this comment.
Likely an incorrect or invalid review comment.
120-129: OTel upgrade OK — no removed/renamed APIs found.Found only internal/dev/server.go:644 using otel.Tracer(..., trace.WithInstrumentationAttributes(...)); both otel.Tracer and trace.WithInstrumentationAttributes are present in the opentelemetry-go API docs. (pkg.go.dev)
3-3: Invalidgodirective: patch versions aren’t allowed; add atoolchainline instead.
go.modonly accepts MAJOR.MINOR in thegodirective. Usego 1.25and specify the patch viatoolchain go1.25.1.Apply this diff:
-go 1.25.1 +go 1.25 +toolchain go1.25.1Likely an incorrect or invalid review comment.
7-7: Confirm AskForConfirm API compatibility with github.com/agentuity/go-common v1.0.91go.mod pins github.com/agentuity/go-common v1.0.91; callsites found: internal/bundler/upgrade.go and internal/bundler/bundler.go — verify AskForConfirm's signature/return type in the go-common v1.0.91 tag matches these usages (or update the callsites/tests).
- Added: [AGENT-684] Check if zsh is installed before adding autocomplete in the CLI (#450) - Added: [AGENT-628] Unit tests (#441) - Added: feat: automatically add AGENTUITY_SDK_KEY and AGENTUITY_PROJECT_KEY to .env file when running dev command (#442) - Changed: Dont sort releases by commit msg (#447) - Changed: [AGENT-628] prevent local development env files from syncing to production (#440) - Fixed: Fix npm workspaces (#451) - Fixed: Fix 'Press any key to continue' to accept any key, not just Enter (#445) Co-Authored-By: unknown <>
- Added: [AGENT-684] Check if zsh is installed before adding autocomplete in the CLI (#450) - Added: [AGENT-628] Unit tests (#441) - Added: feat: automatically add AGENTUITY_SDK_KEY and AGENTUITY_PROJECT_KEY to .env file when running dev command (#442) - Changed: Dont sort releases by commit msg (#447) - Changed: [AGENT-628] prevent local development env files from syncing to production (#440) - Fixed: Fix npm workspaces (#451) - Fixed: Fix 'Press any key to continue' to accept any key, not just Enter (#445) Co-Authored-By: unknown <> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Fix 'Press any key to continue' to accept any key, not just Enter
Summary
The
agentuity project importcommand was showing "Press any key to continue..." prompts that only responded to the Enter key, not any key as advertised. This was caused by the underlyingtui.WaitForAnyKey()andtui.WaitForAnyKeyMessage()functions using regularos.Stdin.Read(), which requires Enter to flush the input buffer.Solution:
internal/inputpackage withWaitForAnyKey()andWaitForAnyKeyMessage()functionsterm.MakeRaw()) for immediate keypress detection, following the same pattern as the existingAskForConfirm()functionTesting: Created standalone test program that successfully detected space bar and letter keys without requiring Enter.
Review & Testing Checklist for Human
agentuity project importand verify the "Press any key to continue..." prompt responds to various keys (letters, numbers, space, arrow keys) without needing EnterNotes
AskForConfirm()pattern from go-common packageSummary by CodeRabbit
Chores
Security
Performance
Bug Fixes