Skip to content

fix(drop-stage): land the drop on intent main so haiku next doesn't hang - #377

Merged
jwaldrip merged 2 commits into
mainfrom
fix/drop-stage-hang-lands-on-main
May 28, 2026
Merged

jwaldrip merged 2 commits into
mainfrom
fix/drop-stage-hang-lands-on-main

Conversation

@jwaldrip

Copy link
Copy Markdown
Contributor

What

haiku next hangs after an optional stage is dropped. Root cause is a branch-location bug, and the second half fixes how the statusline reads during/after a drop.

Why it hung

The keep-or-drop offer parks the checkout on the optional stage's own branch (the cursor's post-action branch switch in haiku_run_next). haiku_drop_stage committed the intent.stages edit to whatever branch was checked out — that doomed stage branch.

But every future stage branch forks from intent main (git branch <stage> <main>, git-worktree.ts), and the per-tick sync only flows main → stage. A dropped stage never completes, so it never merges up. Intent main never saw the drop:

Tick Checkout intent.stages there findCurrentStage
A dropped-stage branch drop applied → next stage
(switch) forks next-stage branch from main drop NOT present → dropped stage
B dropped-stage branch drop applied → next stage

elaborate_loop(dropped)elaborate_loop(next), A/B/A/B forever. The inter-tick deadlock detector caught the churn and swapped in loop_halted — that halt is the "hang."

The fix

haiku_drop_stage now switches back to intent main, lands the drop there (the fork source for every future stage branch), and reaps the orphan stage branch so it can't reassert the stale plan through a downstream sync. Guard 3 already proves the stage never started, so its branch holds no work to preserve. No-op in filesystem mode.

Statusline

  • During the offer: renders keep / drop? (gated) instead of a generic elaborate, so the strip shows the engine is waiting on a decision rather than working.
  • After the drop: the dropped stage simply falls out of the pipeline (it's gone from resolveIntentStages) — no tombstone. This was also broken before the fix, for the same root cause (the statusline read the same stale main intent.stages).

Tests

  • New drop-stage-lands-on-main.test.mjs — red against the old behavior (drop must leave the checkout on intent main), green with the fix. Asserts the drop lands on main, the branch is reaped, and the cursor advances to the next stage instead of flipping back to the dropped one.
  • Full suite: 2070 passed, 0 failed across 242 files.

🤖 Generated with Claude Code

…t hang

The keep-or-drop offer parks the checkout on the optional stage's own
branch (the cursor's post-action branch switch). haiku_drop_stage committed
the intent.stages edit to whatever branch was checked out — i.e. that
doomed stage branch. But every future stage branch forks from intent main
(git branch <stage> <main>) and the per-tick sync only flows main -> stage,
and a dropped stage never completes (so it never merges up). Intent main
therefore never saw the drop: the next stage forked from a main that still
listed the dropped stage, findCurrentStage flip-flopped dropped <-> next
every tick, and the deadlock detector halted the loop — the "haiku next
hangs after a drop" report.

Fix: switch back to intent main, land the drop there, and reap the orphan
stage branch so it can't reassert the stale plan through a downstream sync.
Guard 3 already proves the stage never started, so its branch holds no work
to preserve. No-op in filesystem mode.

Statusline: render the optional stage's pending keep-or-drop offer as
"keep / drop?" (gated) instead of a generic "elaborate", so the strip shows
the engine is waiting on a decision. After a drop the dropped stage simply
falls out of the pipeline (resolveIntentStages) — no tombstone.

Adds drop-stage-lands-on-main.test.mjs: red against the old behavior
("drop must leave the checkout on intent main"), green with the fix —
asserts the drop lands on main, the branch is reaped, and the cursor
advances instead of flipping back to the dropped stage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @jwaldrip's task in 2m 11s —— View job


PR Review

  • Gather context and read changed files
  • Review haiku_drop_stage.ts — core fix
  • Review statusline/state.ts — statusline fix
  • Review drop-stage-lands-on-main.test.mjs — test coverage
  • Post final assessment

Summary

This is a correct and well-bounded fix. The root cause diagnosis in the PR description matches the code exactly, and the implementation is surgical.

The core bug was a commit-location mismatch: haiku_drop_stage committed the intent.stages mutation to whatever branch was currently checked out (the optional stage's own branch), but every future stage branch forks from intent main. The per-tick sync is unidirectional (main → stage), and a dropped stage never completes so it never merges up — intent main permanently held the stale plan, forked branches inherited it, and findCurrentStage oscillated indefinitely until the deadlock detector halted the loop.

The fix is correct. ensureOnStageBranch(slug, undefined) reliably resolves to haiku/${slug}/main (the code path where stage is falsy sets targetBranch = intentMain) and deleteStageBranch is safely best-effort and guards against stage === "main". gitCommitState has its own isGitRepo() guard so the unconditional call is safe in filesystem mode.


One minor issue

statusline/state.ts — unnecessary type cast

Inside case "elaborate_loop":, TypeScript's switch-case narrowing already tells the compiler that action has the shape { kind: "elaborate_loop"; optional_offer?: boolean; … } — the field is declared directly on that union variant in cursor.ts:166. The explicit cast (action as { optional_offer?: boolean }) is redundant:

// current
if ((action as { optional_offer?: boolean }).optional_offer === true) {

// cleaner — the union narrowing already covers it
if (action.optional_offer === true) {

Fix this →

This doesn't affect behavior, but it's the right way to read the type.


Test coverage

The test is thorough and exactly mirrors the failure scenario: checkout parked on the doomed stage branch, call the tool, assert the three post-conditions (checkout on main, stage gone from main's intent.stages, orphan branch reaped). The regression proof is tight.

One observation: process.env.CLAUDE_PLUGIN_ROOT is set at module scope (line 33) rather than inside the test. That's fine for an isolated test file, but worth knowing if this module is ever imported by a test runner that batches multiple test files into one process — the env mutation would persist. Not a bug here, just worth flagging for future authors using the same pattern.


Verdict: approve. The fix is correct, the test is a genuine regression proof, and the statusline change is a useful improvement. The one non-blocking nit is the redundant cast in statusline/state.ts.

- biome check packages/haiku/src now clean (import ordering on the new
  git-worktree import block fixed the failing Biome Lint job)
- statusline: drop the redundant `as { optional_offer?: boolean }` cast —
  the switch-case already narrows `action` to the elaborate_loop variant,
  which declares optional_offer (cursor.ts:166)
- format the regression test to biome style

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jwaldrip

Copy link
Copy Markdown
Contributor Author

Addressed the review in f35e809:

  • Biome Lint (CI failure): the failing job was the import ordering on the new git-worktree import block in haiku_drop_stage.ts. npx biome check packages/haiku/src is clean now (252 files).
  • Redundant cast: dropped — action.optional_offer reads straight off the narrowed elaborate_loop variant.
  • CLAUDE_PLUGIN_ROOT at module scope (non-blocking): noted. Kept it module-scoped to match the existing pattern in optional-stages.test.mjs; the test runner spawns one process per file (scripts/test-single.mjs), so the env mutation doesn't leak across files.

@claude

claude Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @jwaldrip's task in 1m 45s —— View job


Claude finished reviewing PR #377 — re-review after f35e809

  • Read full PR diff
  • Review haiku_drop_stage.ts — core fix (including f35e809 format changes)
  • Review statusline/state.ts — redundant cast removed
  • Review drop-stage-lands-on-main.test.mjs — test coverage
  • Post final assessment

Re-review after f35e809

Both issues from the first pass are resolved. The PR is clean.

Redundant cast — removed. cursor.ts:166 declares optional_offer?: boolean directly on the elaborate_loop variant, so the switch-case narrowing already makes it accessible. action.optional_offer is the right read. ✓

Biome import ordering — fixed. The git-worktree import block in haiku_drop_stage.ts is now sorted to pass npx biome check. ✓

Core fix — still correct. Tracing the full path: ensureOnStageBranch(slug, undefined) computes stageBranch = "" (falsy stage), then targetBranch = intentMain (git-worktree.ts:2694-2695). So passing undefined reliably switches to haiku/${slug}/main. deleteStageBranch guards against stage === "main" and never throws (git-worktree.ts:4832-4833). gitCommitState has always been called outside isGitRepo() in the existing codebase — it handles the no-git case internally. The pre-switch read of intentFm (used to compute nextStages) is safe: guard 3 proves the design branch carries no engine-authored content not present on main, so the derived nextStages is identical to what setFrontmatterField would produce reading from either branch.

Test — tight regression proof. The three assertions directly mirror the three failure modes described in the PR: (1) checkout lands on intent main after the drop, (2) the drop appears in main's intent.md, (3) the orphan branch is reaped. The sanity checks before the call confirm the pre-condition (parked on the doomed branch, cursor sees design). CLAUDE_PLUGIN_ROOT at module scope matches the existing pattern in optional-stages.test.mjs and is safe given the one-process-per-file test runner.

No new findings. Ready to merge.

@jwaldrip
jwaldrip merged commit 51f695c into main May 28, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant