fix: wt create — reclaim terminal foreground after init phase#20
Merged
Conversation
… menu) wt create (and wt init) lost terminal foreground during the init phase: an interactive child in an init script (e.g. zsh -i in a direnv-hook check) grabs the controlling terminal's foreground group and exits without restoring it, orphaning wt to the background so the Open-phase menu's TTY write triggers SIGTTOU (zsh: suspended (tty output)). Fix: wt now captures the terminal foreground before the init child and reclaims it (tcsetpgrp back to its own pgrp, wrapped in signal.Ignore(SIGTTOU)) after the child returns on all exit paths, gated on term.IsTerminal (no-op when not a TTY / --non-interactive / CI), no-op on Windows. Preserves the SIGINT-during-init Option B contract (Setpgid: true unchanged). Includes a self-skipping Unix PTY integration test verified to catch the regression. No CLI surface change; ioctls via golang.org/x/sys/unix (promoted indirect to direct, no new module).
There was a problem hiding this comment.
Pull request overview
Fixes a job-control correctness issue where wt create/wt init could be left in a background process group after the init child exits, causing subsequent terminal operations (notably the Open/menu raw-mode transition) to trigger SIGTTOU and stop the process. The PR introduces TTY foreground capture/reclaim around the init phase (guarded by TTY detection), provides Unix/Windows helper implementations, and adds an end-to-end PTY-based regression test.
Changes:
- Add Unix/Windows build-tagged helpers to
tcgetpgrp/tcsetpgrpthe controlling terminal foreground process group, ignoringSIGTTOUduring the reclaim. - Apply foreground capture/reclaim around the init child in both
wt createandwt init, including explicit reclaims on success/failure paths. - Add a Unix-only, self-skipping PTY integration test that reproduces the “foreground stranded →
SIGTTOUstop” regression.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/go.mod | Promotes golang.org/x/sys to a direct dependency for Unix ioctl helpers/tests. |
| src/cmd/wt/tty_unix.go | Implements terminalForeground/reclaimTerminalForeground via x/sys/unix ioctls with SIGTTOU ignored during reclaim. |
| src/cmd/wt/tty_windows.go | Windows no-op stubs for foreground bookkeeping (no tcsetpgrp model). |
| src/cmd/wt/create.go | Captures/restores terminal foreground around the init child; reclaims before failure banner and before Open/menu. |
| src/cmd/wt/init.go | Applies the same capture/reclaim behavior to standalone wt init around cmd.Run(). |
| src/cmd/wt/tty_pty_test.go | Adds an end-to-end PTY regression test using helper binaries to reproduce and detect WIFSTOPPED behavior. |
| fab/changes/260602-z4p7-wt-reclaim-tty-foreground-after-init/plan.md | Change plan/requirements documentation for the fix and test strategy. |
| fab/changes/260602-z4p7-wt-reclaim-tty-foreground-after-init/intake.md | Intake/root-cause writeup documenting the SIGTTOU stop scenario and chosen approach. |
| fab/changes/260602-z4p7-wt-reclaim-tty-foreground-after-init/.status.yaml | FAB status metadata for the change lifecycle. |
| fab/changes/260602-z4p7-wt-reclaim-tty-foreground-after-init/.history.jsonl | FAB history log for stage transitions/commands. |
| docs/memory/wt-cli/init-failure-contract.md | Documents the new “reclaim terminal foreground after init” contract alongside SIGINT Option B invariants. |
| docs/memory/wt-cli/create-output-phases.md | Notes the pre-Open foreground reclaim ordering guarantee in the output-phases contract. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+24
to
+26
| 1. `PhaseSeparator("Git")` — emitted at `create.go:264` immediately before the deferred summary block (`Created worktree:` / `Path:` / `Branch:`), joining the existing deferred-summary emission under the rollback handler. | ||
| 2. The **Init** separator — emitted by the init runner, NOT by `create.go` (see next requirement). | ||
| 3. `PhaseSeparator("Open")` — emitted at `create.go:319` immediately before the open phase. | ||
| 3. `PhaseSeparator("Open")` — emitted immediately before the open phase. As of `260602-z4p7`, when the init phase ran, the Open separator + menu render are now preceded by an **unconditional foreground reclaim** (`reclaimTerminalForeground(ttyFd, wtPgid)` at `create.go:361–363`, gated on the same `term.IsTerminal` check as capture) so the Open phase can never SIGTTOU on a shared-TTY init child that stranded terminal foreground. The separator output contract itself is unchanged — this is a job-control ordering guarantee, not an output change. See `init-failure-contract.md` "Terminal foreground is reclaimed after the init child". |
Owner
Author
There was a problem hiding this comment.
Fixed — replaced the stale create.go line numbers (264/282, now 265/326 after this PR) with descriptive references ("before the Git/Open separator", "before the init-phase signal.Reset") so the contract no longer drifts with line shifts. (1be5d87)
| // wt is either still alive (not stopped) or has exited cleanly. | ||
| // We poll for up to a few seconds: any STOPPED observation => bug (exit 20); | ||
| // a clean exit or a still-running-but-not-stopped wt => fix (exit 0). | ||
| deadline := time.Now().Add(8 * time.Second) |
Owner
Author
There was a problem hiding this comment.
Fixed — bumped the launcher success-poll window from 8s to 15s (matching integration_sigint_unix_test.go) and documented why it must exceed wt's time-to-Open-phase, so a slow runner can't elapse the deadline before wt reaches term.MakeRaw and report a vacuous pass. (1be5d87)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Meta
Pipeline: intake ✓ → apply ✓ → review ✓ → hydrate ✓ → ship → review-pr
Impact: +454/−1 code (excluding
fab/,docs/) · +1087/−4 totalSummary
wt createacts as a job-control parent for the init child (Setpgid: true) while sharing its controlling terminal, but never reclaimed terminal foreground after the child returned. If the init script (or any descendant — e.g. azsh -idirenv-hook check) grabbed the terminal foreground and exited without restoring it,wtwas left in the background, so the next TTY write (the Open-phase menu render) tripped SIGTTOU and suspended the process (zsh: suspended (tty output)), stranding a half-finished worktree. This fix makeswtcapture-before and reclaim-after terminal foreground around the init child on every exit path, restoring reliable interactive behavior (Constitution VI).Changes
wt create) — capturetcgetpgrpbefore the init child,tcsetpgrpback towt's own pgrp after on all exit paths (success/pre-Open, init-failure/pre-banner, SIGINT-abort), wrapped insignal.Ignore(SIGTTOU).tcsetpgrphelpers — newtty_unix.go(x/sys/unixioctlsTIOCGPGRP/TIOCSPGRP) andtty_windows.go(no-op stubs), mirroring the existingsignal_unix.go/signal_windows.gopattern.--non-interactive, piped, CI); no error, no warning.Setpgid: true, the tight reinstall window, anddefer rb.Execute()are unchanged; the reclaim only touches the terminal's foreground, never the child's pgrp.wt init— standalone interactivewt initis no longer left suspended after the child strands foreground.openptyonx/sys/unix; verified to catch the regression (exit 20 /WIFSTOPPEDwithout the fix, exit 0 with it).