Skip to content

docs: Add GitHub squash merge analogy to wt merge - #42

Merged
max-sixty merged 3 commits into
mainfrom
claude/improve-wt-merge-docs-01UR5KE9CRzxZEWYjVMVjnww
Nov 24, 2025
Merged

docs: Add GitHub squash merge analogy to wt merge#42
max-sixty merged 3 commits into
mainfrom
claude/improve-wt-merge-docs-01UR5KE9CRzxZEWYjVMVjnww

Conversation

@max-sixty

Copy link
Copy Markdown
Owner

Update both the help text and README to clarify that wt merge's squash
behavior is like GitHub's "Squash and merge" button. This helps users
quickly understand what the command does by drawing on their existing
familiarity with GitHub's merge workflows.

Changes:

  • Updated Squash section in wt merge --help
  • Updated corresponding README section
  • Updated snapshot test

Update both the help text and README to clarify that wt merge's squash
behavior is like GitHub's "Squash and merge" button. This helps users
quickly understand what the command does by drawing on their existing
familiarity with GitHub's merge workflows.

Changes:
- Updated Squash section in wt merge --help
- Updated corresponding README section
- Updated snapshot test
Move "with LLM-generated commit message" to the end of the sentence for
better flow and clarity. The GitHub analogy now sits in the middle,
providing context before the implementation detail.

Before: "...squashed into one with LLM message (like GitHub's...)"
After: "...squashed into one (like GitHub's...) with LLM-generated commit message"

This follows the pattern: [action] → [analogy] → [detail]
Use consistent terminology throughout the codebase and documentation:
- "with LLM message" → "with LLM commit message"
- "with LLM-generated commit message" → "with LLM commit message"
- "with LLM-generated messages" → "with LLM commit messages"

This makes the terminology clearer and more consistent across all help
text, documentation, and code comments.

Files updated:
- src/cli.rs: Merge and step command help text
- README.md: All references to LLM message generation
- tests/: Updated comments for consistency
- Snapshot tests updated
@max-sixty
max-sixty merged commit 3726913 into main Nov 24, 2025
14 checks passed
@max-sixty
max-sixty deleted the claude/improve-wt-merge-docs-01UR5KE9CRzxZEWYjVMVjnww branch November 24, 2025 17:22
max-sixty added a commit that referenced this pull request Jun 28, 2026
## The bug

In the `wt switch --prs` interactive picker, removing a worktree row
with `alt-x` made the streamed PR/MR rows vanish from the list until the
user pressed `alt-r` to refresh. The worktree/branch rows survived; only
the `--prs` rows disappeared.

## Root cause

The `alt-x` removal rework (`d66bc6af5`) replaced the old `reload(remove
{})` with a synchronous `resync_pool` that rebuilds skim's item pool
from the picker's `shared_items` Vec. But `shared_items` only ever held
the skeleton (worktree/branch) rows: `on_skeleton` populates it, while
the `--prs` thread streams its rows straight to skim's item channel
(`prs::fetch_and_stream` → `tx.send`) and never recorded them in
`shared_items`. So when `resync_pool` rebuilt the pool from
`shared_items`, the PR rows were dropped. They only reappeared when
`alt-r` re-ran the whole collect + `--prs` pipeline. (The old `reload`
path had the same blind spot; it matters more now that `alt-x` is the
sole removal path.)

## The fix

The `--prs` thread now appends its PR/MR rows into `shared_items` as
well as streaming them to skim — the same way it already extends
`shortcut_table`. With `shared_items` holding the full row set (header +
worktree/branch + PR/MR rows), `resync_pool` preserves the PR rows on an
`alt-x` removal for free.

The append is guarded by a per-spawn epoch counter
(`PipelineFactory::prs_epoch`, handed to each spawn's `--prs` thread via
`PrsShared`). An `alt-r` refresh spawns a fresh `--prs` thread while the
prior spawn's forge call may still be in flight; without the guard, that
stale call (whose skim channel is already dropped) would re-add
now-duplicate rows to the list a newer spawn rebuilt. The epoch is read
under the `shared_items` lock so the check pairs with the next spawn's
`on_skeleton` overwrite, which holds the same lock. The append is
ordered before `tx.send` so the rows reach `shared_items` no later than
they reach skim's pool (favoring a sub-microsecond transient-duplicate
window over re-dropping rows, were the order reversed).

The no-flash cursor behavior from `d66bc6af5` is unchanged: the rebuilt
list is just longer, so the cursor holds its index and the row that
slides into the removed slot lands under it.

## Testing

New PTY regression test
`test_switch_picker_prs_rows_survive_alt_x_removal` drives the drop path
(a clean, integrated worktree) in `--prs` mode and asserts the `#42` PR
row survives the removal. The mock answers `gh pr list --state` (the
`--prs` fetch) with PR #42 but `gh pr list --head <branch>` (the
per-worktree CI fetch) with an empty list, so `#42` appears only as a
`--prs` row, never folded into a worktree row's CI cell. Confirmed: the
test fails without the fix and passes with it. Full pre-merge gate green
(4257 tests).

> _This was written by Claude Code on behalf of max_

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
max-sixty pushed a commit that referenced this pull request Jul 25, 2026
#3532)

## Problem

`test_switch_picker_prs_shows_loading_marker` intermittently times out
on `test (windows)` — it flaked on #3531 (a docs-only change), which is
what prompted this fix.

The test asserts a *transient* frame: while a mocked, delayed `gh pr
list` is in flight, the picker header shows `↳ Loading open PRs…` and
the `#42` row has not yet streamed in. The mock held that loading state
for a fixed time (originally 3s). On a loaded Windows runner, the PTY
boot sequence — `boot_picker_pty`'s skim-ready wait plus its initial
`wait_for_stable`, together with the async worktree-column churn (`·`
placeholders resolving) — can consume that whole window. By the time the
stabilization helper first polls for `Loading open PRs`, the mocked
fetch has already returned and the rows have streamed in, so the marker
is gone and the wait times out at `STABILIZE_TIMEOUT` (30s).

## Fix

Per review feedback
([comment](#3532 (comment))),
tie the marker's lifetime to the picker's, with **no timing number at
all** — the file's "poll, don't pick a time" rule applied to the
producer side.

`mock-stub` gains a `hold_until_parent_exit` response mode: the mocked
`gh pr list` holds until `wt` exits instead of sleeping a fixed
`delay_ms`. Detection needs no platform primitive (`getppid` /
`OpenProcess`):

- While `wt` lives, its `--prs` fetch thread runs the mock with a piped
stdout and blocks reading that pipe to EOF, so as long as the mock
neither writes a full response nor exits, the loading marker stays.
- When the picker aborts, `wt` detaches the fetch thread
(`drop(prs_handle)`) and exits, which closes the read end of the mock's
stdout pipe. The mock's next write then fails with `BrokenPipe` (Rust
ignores `SIGPIPE`, so the broken write surfaces as an `Err` rather than
killing the process), and the mock exits.

Polling for that write error is a **causal** parent-death signal —
release is tied to `wt` exiting, not to a timer — and it behaves
identically on Unix and Windows, so there's no platform-specific FFI to
get right. The one-byte probes written into stdout are never observed
(this mode's contract is that the parent aborts without parsing the
response, and the fetch thread drains the pipe until process exit). A
60s poll cap bounds a stuck orphan if the pipe somehow never breaks.

This removes the race the earlier 30s bump only widened: the marker is
present for the entire window the poll could observe it, on any boot
latency, and no orphaned sleeper is left behind — the mock exits the
moment the picker does.

## Testing

`cargo test --test integration --features shell-integration-tests
test_switch_picker_prs_shows_loading_marker` passes in ~1.3s, confirming
the capture-and-abort path stays fast and the mock releases promptly on
`wt` exit. The sibling `--prs` tests
(`test_switch_picker_prs_{github,gitlab}_list`,
`..._rows_survive_alt_x_removal`) still pass through the refactored
`mock_forge_env` helper. Reasoning is documented inline at the mock-stub
call site and in `wait_for_parent_exit`.

---------

Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
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.

2 participants