fix: Fix stdin byte-theft at the menu→line-prompt seam; correct mislabeled dirty-state warning in wt create#30
Conversation
Orphaned stdin read-ahead pump goroutine steals the next typed line when a menu is followed by PromptWithDefault/ConfirmYesNo, causing the "Worktree name" prompt in wt create to hang or swallow input. Extend the existing MenuSession shared-reader mechanism to session- aware PromptWithDefault/ConfirmYesNo and thread one session through wt create. Also correct the dirty-state warning to say "current worktree" instead of "main repo" since the check runs against the process CWD.
There was a problem hiding this comment.
Pull request overview
This PR fixes an interactive stdin “byte theft” bug when an arrow-key menu is immediately followed by a cooked-mode line prompt (notably wt create’s dirty-state menu → worktree-name prompt), by extending MenuSession’s single-shared-reader contract to line prompts and threading one session through wt create. It also corrects a misleading dirty-state warning message to refer to the “current worktree”.
Changes:
- Add session-aware
MenuSession.PromptWithDefault/MenuSession.ConfirmYesNothat read via the sharedblockingByteReaderpump (plus shared prompt-format constants and a shared yes/no parser). - Thread a single
MenuSessionthroughwt create’s full interactive flow (menus + line prompts) and update the dirty-state warning copy. - Add targeted regression/unit tests and update the CLI contract memory docs to capture the extended invariant.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/internal/worktree/menu.go | Adds session-aware line prompts + shared parsing/formatting helpers; adds blockingByteReader.readLine() to assemble cooked lines through the pump. |
| src/internal/worktree/menu_session_test.go | Adds regression + unit tests for the menu→line-prompt seam and readLine() behavior. |
| src/cmd/wt/testutil_test.go | Introduces runWtStdin to drive non-TTY fallback interactions deterministically from integration tests. |
| src/cmd/wt/create.go | Threads a single MenuSession through all interactive stdin consumers; fixes dirty-state warning text. |
| src/cmd/wt/create_test.go | Adds an integration test asserting the corrected dirty-state warning copy. |
| docs/memory/wt-cli/menu-navigation-contract.md | Documents the new menu→line-prompt seam contract and wt create session-threading. |
| docs/memory/wt-cli/index.md | Updates the index description for the amended menu navigation/reader-sharing contract. |
| fab/changes/260708-wryx-menu-prompt-byte-theft/plan.md | Adds the generated plan capturing requirements, tasks, and acceptance criteria for the change. |
| fab/changes/260708-wryx-menu-prompt-byte-theft/intake.md | Adds intake write-up explaining the bug, rationale, and scope. |
| fab/changes/260708-wryx-menu-prompt-byte-theft/.status.yaml | Tracks fab pipeline status/metrics for the change. |
| fab/changes/260708-wryx-menu-prompt-byte-theft/.history.jsonl | Records fab stage transition history for the change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // bufio.Reader over the same fd). Feed the full typed line concurrently; the | ||
| // menu's orphan is parked first and steals the leading byte(s), so the | ||
| // prompt reader never assembles the intact "my-name". | ||
| promptReader := newBlockingByteReader(stream) | ||
| feedAsync(stream, "my-name\n", false) |
There was a problem hiding this comment.
Fixed — made the theft deterministic: push the line's first byte and drain it back through the menu's orphaned reader (both hard sync points) before the prompt reader is created, then feed the remainder ("y-name\n"). The prompt reader can now never assemble the intact "my-name", so the sleep/two-goroutine race is gone. Verified with -race -count=50. (e9be7d9)
| > Amended by `260705-3wr8-menu-viewport-scrolling` (terminal-height-aware scrolling viewport). | ||
| > Amended by `260708-wryx-menu-prompt-byte-theft` (the single-reader `MenuSession` contract extended to the menu→line-prompt seam — session-aware `PromptWithDefault`/`ConfirmYesNo`, and `wt create` joining the session-threading pattern). | ||
|
|
||
| This file documents the contract that `ShowMenu` honors after the arrow-key navigation change. Future changes touching `src/internal/worktree/menu.go` should preserve these invariants unless an explicit spec amendment supersedes them. The change affects every interactive prompt invoked by `wt` (most prominently the `wt open` / `wt delete` worktree pickers) — but only via the shared `ShowMenu` entry point. No call site under `src/cmd/wt/` was edited. |
There was a problem hiding this comment.
Fixed — reworded so the "no src/cmd/wt/ call site edited" claim is scoped to the original arrow-key change, then explicitly notes that the 260708-wryx amendment deliberately edited wt create's RunE to thread one MenuSession through its interactive flow. No longer self-contradictory. (e9be7d9)
Meta
wryxexcludes
fab/,docs/· generated by fab-kit v2.13.6Pipeline: intake ✓ → apply ✓ → review ✓ → hydrate ✓ → ship → review-pr
Summary
An orphaned stdin read-ahead pump goroutine steals the user's next typed line when a menu is immediately followed by a line prompt, causing the "Worktree name" prompt in
wt createto hang or swallow input. This extends the existingMenuSessionshared-reader mechanism to session-awarePromptWithDefault/ConfirmYesNoand threads one session throughwt create's interactive flow. It also corrects a mislabeled dirty-state warning that says "main repo" when the check actually runs against the current worktree.Changes
src/internal/worktree/menu.goMenuSessionthroughwt create's interactive flow (src/cmd/wt/create.go)