Skip to content

fix(server): settle cancelled worktree setup before rollback - #12176

Merged
juliusmarminge merged 1 commit into
mainfrom
mobile-worktree-setup/cancellation
Sep 17, 2026
Merged

juliusmarminge merged 1 commit into
mainfrom
mobile-worktree-setup/cancellation

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 17, 2026

Copy link
Copy Markdown
Member

Cancelling a worktree bootstrap interrupted its failure handler before it could publish the cancelled outcome or roll back the thread. Clients could keep showing setup as running after the task stopped.

Keep the bootstrap interruptible while letting its settlement handler finish recording the outcome, closing the setup terminal, removing the worktree, and rolling back the thread. A focused router test verifies the cancelled receipt, deleted thread, failed original dispatch, and absence of an agent turn; the existing synchronous and asynchronous handoff cases still pass.

Validation: all three focused setup router cases pass, server typecheck passes, and targeted lint has no errors. This changes server settlement only; screenshots are not applicable.

Implemented with GPT-6 in Codex.

Summary by CodeRabbit

  • Bug Fixes
    • Improved cancellation handling during synchronous worktree setup.
    • Cancelled setup now reports a clear “Worktree setup cancelled.” error and cleans up the temporary bootstrap thread.
    • Prevented the bootstrap turn from starting after setup is cancelled.
    • Ensured cancellation cleanup and rollback complete reliably before the operation ends.

@juliusmarminge
juliusmarminge added this pull request to stack #12178 September 17, 2026 01:32
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 17, 2026
@juliusmarminge
juliusmarminge marked this pull request as ready for review September 17, 2026 01:33
@juliusmarminge juliusmarminge changed the title mobile worktree setup/cancellation fix(server): settle cancelled worktree setup before rollback Sep 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.6 KiB 13.6 KiB +18 B (+0.1%) 15.1 KiB
Codex Thread snapshot wire 7.1 KiB 7.1 KiB +2 B (+0.0%) 7.3 KiB
Codex Live turn WebSocket wire 6.6 KiB 6.6 KiB +16 B (+0.2%) 7.8 KiB
Codex Live turn WebSocket decoded 57.1 KiB 57.1 KiB 0 B (0.0%) 66.4 KiB
Codex Live turn messages 10 10 0 (0.0%) 21
Claude Total thread wire 13.6 KiB 13.6 KiB −4 B (−0.0%) 15.1 KiB
Claude Thread snapshot wire 7.1 KiB 7.1 KiB +1 B (+0.0%) 7.3 KiB
Claude Live turn WebSocket wire 6.5 KiB 6.5 KiB −5 B (−0.1%) 7.8 KiB
Claude Live turn WebSocket decoded 57.8 KiB 57.8 KiB 0 B (0.0%) 66.4 KiB
Claude Live turn messages 9 9 0 (0.0%) 21

Baseline: 962bf62 · PR result: 51c0812 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 113.9 KiB
  • Claude decoded thread snapshot: 114.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@macroscopeapp

This comment has been minimized.

@macroscopeapp

macroscopeapp Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 51c0812

Macroscope's review found this PR approvable — This narrowly fixes cancelled worktree setup by allowing cancellation to reach the existing rollback path and ensuring the cancellation outcome and cleanup finish atomically. A focused server test verifies the thread is deleted, the cancelled activity is recorded, and no turn starts.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: c08b7252-22ed-40fc-ba7f-59407d2b8671

📥 Commits

Reviewing files that changed from the base of the PR and between 962bf62 and 51c0812.

📒 Files selected for processing (2)
  • apps/server/src/server.test.ts
  • apps/server/src/ws.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Worktree setup cancellation now interrupts the bootstrap program, protects cleanup and rollback, deletes the bootstrap thread, reports cancellation, and prevents turn dispatch. Tests cover the cancellation result and related dispatch metadata.

Changes

Worktree setup cancellation

Layer / File(s) Summary
Cancellation flow and validation
apps/server/src/ws.ts, apps/server/src/server.test.ts
The bootstrap program is interruptible during setup and uninterruptible during cancellation cleanup and rollback. Tests verify the cancelled outcome, deleted bootstrap thread, failed dispatch metadata, and absent turn dispatch.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant CancellationRPC
  participant BootstrapProgram
  participant TurnDispatch
  CancellationRPC->>BootstrapProgram: interrupt running setup
  BootstrapProgram->>BootstrapProgram: complete cleanup and rollback
  BootstrapProgram->>CancellationRPC: report cancellation and deleted thread
  BootstrapProgram-->>TurnDispatch: fail dispatch without starting a turn
Loading

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to 51c08

Cancelled worktree setup completes its cleanup and reports the cancelled result without starting an agent turn. No current merge-blocking risk was identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: settling cancelled worktree setup before rollback.
Description check ✅ Passed The description explains what changed, why it was needed, the test coverage, and that UI changes do not apply. It does not use the template headings or checklist, but it contains the required informat…
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mobile-worktree-setup/cancellation

Comment @coderabbitai help to get the list of available commands.

@juliusmarminge
juliusmarminge merged commit 3bee4c2 into main Sep 17, 2026
28 checks passed
@juliusmarminge
juliusmarminge deleted the mobile-worktree-setup/cancellation branch September 17, 2026 01:42
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 17, 2026
## What's Changed
* fix(server): settle cancelled worktree setup before rollback by @juliusmarminge in pingdotgg/t3code#12176
* feat(mobile): port worktree setup progress and agent handoff by @juliusmarminge in pingdotgg/t3code#12177
* fix(server): flush checkpoint objects and refs before publishing them by @Mnigos in pingdotgg/t3code#10944
* fix(server): keep ready checkpoints when a later placeholder arrives by @Adolanium in pingdotgg/t3code#8432
* fix(server): keep VCS waits from blocking turn completion by @Vrtak-CZ in pingdotgg/t3code#11970
* fix(web): keep header spacing stable when sidebar drawer opens by @flamboh in pingdotgg/t3code#12162
* fix(web): fall back when pull request avatars fail by @tastelessjolt in pingdotgg/t3code#11728
* feat(web): enable rich text composer by default by @juliusmarminge in pingdotgg/t3code#12160
* feat(web): make keybindings searchable from settings search by @maria-rcks in pingdotgg/t3code#12175
* fix(web): preserve thread reading positions by @maria-rcks in pingdotgg/t3code#12144
* fix(diff): collapse files by default by @maria-rcks in pingdotgg/t3code#12190
* fix(web): folder links from chat open the file tree instead of a broken preview by @pc-style in pingdotgg/t3code#10909
* feat(web): command palette search matches thread IDs by @saphid in pingdotgg/t3code#11185
* fix(web): align notification icons with titles by @maria-rcks in pingdotgg/t3code#12202
* fix(skills): support unicode currency symbols as skill aliases by @WilgotM in pingdotgg/t3code#12098
* feat(settings): add automatic storage cleanup per machine and project by @maria-rcks in pingdotgg/t3code#11598
* feat(web): command palette finds the pull requests and usage pages by @flamboh in pingdotgg/t3code#12211
* feat(web): start new threads with multiple models in separate worktrees by @maria-rcks in pingdotgg/t3code#12179

## New Contributors
* @Adolanium made their first contribution in pingdotgg/t3code#8432
* @Vrtak-CZ made their first contribution in pingdotgg/t3code#11970
* @pc-style made their first contribution in pingdotgg/t3code#10909

**Full Changelog**: pingdotgg/t3code@v0.0.43-nightly.20260917.1837...v0.0.43-nightly.20260917.1851

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.43-nightly.20260917.1851
aorwall added a commit to aorwall/t3code that referenced this pull request Sep 18, 2026
Merges `pingdotgg/t3code` `6d1d549441` into the fork, from base
`0bf2d6b010` — 50 commits.

- **Landed:** 410 files against 407 in the upstream range; the gap of 3
is `docs/fork/gaps.md`, `inventory.json` and `upstream-merge-log.md`.
Everything in the range landed.
- **Fork delta:** 777 files.
- **Verification:** all 9 `verify.mjs` checks pass, tests green in all
15 packages.
- **Unsupported methods:** ADD 0, DROP 0 —
`packages/contracts/src/rpc.ts` and `auth.ts` are untouched. Upstream
added no WebSocket method in this range.

## The one that mattered

Upstream's pingdotgg#12015 moved the **entire body of the thread route** out of
`apps/web/src/routes/_chat.$environmentId.$threadId.tsx` and into a new
upstream file, `apps/web/src/components/ThreadRouteView.tsx`, rendered
by the `_chat` layout so a draft's promotion keeps the same `ChatView`
mounted. The route file is now a seven-line stub.

Three fork deltas lived in that file. They moved with it:
`useAdoptedThread`, `useAutoFollowThread` and the
`serverThreadAwaitingFirstAnswer` argument to
`resolveThreadRouteRenderState`, all reading `target.kind === "server" ?
target.threadRef : null` — a draft's reserved ref is the viewer's own
work and the listing carries it without being asked. The
`unlisted-thread-adoption` and `thread-follow` inventory entries were
re-pointed at the new file.

The fork's own delta guard is what caught this. The merge was clean and
typecheck was green; `features.test.ts` failed because
`useAutoFollowThread` was no longer in a file the inventory said it had
to be in.

## Conflicts

8 files, each resolved with the verdict `preflight.mjs` printed. Details
in the tracker entry; the short form:

| file | verdict | resolution |
| --- | --- | --- |
| `routes/_chat.$environmentId.$threadId.tsx` | unlisted | took
upstream's stub, deltas relocated (above) |
| `chat/MessagesTimeline.tsx` | `message-origin-upstream-files` | both
sides of `TimelineRowActivityState`, its memo and its deps merged;
dropped upstream's now-unused `GitPullRequestIcon` |
| `ThreadStatusIndicators.tsx` | `thread-status-indicators` | fork's
memo above upstream's early return — hooks before any conditional
`return null` |
| `settings/ProviderInstanceCard.tsx` | unlisted, in
`moatless-provider-auth` | kept the `FEATURES.providerConfiguration`
ternary, took upstream's container-query classNames inside it |
| `settings/SettingsPanels.tsx` | `settings-surface-gates` | re-stated
the fork's browser clause onto upstream's rewritten `proactive-panels`
text |
| `BranchToolbar.tsx` | `branch-toolbar-gates` | import block, both
sides kept |
| `RightPanelTabs.tsx` | `right-panel-surfaces` | import block, both
sides kept |
| `pnpm-lock.yaml` | `theirs — lockfile` | `--theirs` then `vp i`,
re-derived lockfile committed |

## Path policy closed a hole

`resolution-check` listed eight unlisted paths both sides changed;
**seven carried a real fork delta**, so next merge's `theirs` fallback
would have dropped them silently. All seven are now listed — five new
entries (`command-palette-gates`, `diff-panel-gates`,
`provider-settings-gates`, `chat-layout-route`,
`client-runtime-exports`) plus `rightPanelStore.test.ts` added to
`right-panel-surfaces`. The eighth is the thread route stub, which
resolved to upstream byte for byte.

## Usable as-is

Client work that runs against the Moatless backend today:

- **pingdotgg#12015** worktree setup card no longer flashes or shifts (the
relocation above) · **pingdotgg#12144** thread reading positions are preserved ·
**pingdotgg#12162** header spacing stays stable when the sidebar drawer opens
- **pingdotgg#8641** timestamps on tool rows and turn folds · **pingdotgg#12152** those
timestamps sit before the disclosure chevron · **pingdotgg#12147** thoughts group
into the changing tool activity line
- **pingdotgg#12075** send-shortcut and follow-up controls · **pingdotgg#12160** rich text
composer on by default · **pingdotgg#12165** composer task rows aligned ·
**pingdotgg#11787** tooltips on the composer's environment and workspace controls
· **pingdotgg#12082** simpler agent approval prompts
- **pingdotgg#12139** diff panel defaults to the working tree · **pingdotgg#12190** diff
files collapse by default · **pingdotgg#12142** a linked pull request wins over
an automatic diff
- **pingdotgg#12143** themes picked from chat with colour previews · **pingdotgg#12138**
provider settings adapt to content width · **pingdotgg#12167** follow-up and
license controls aligned
- **pingdotgg#12026** unsupported environments render as neutral rows with their
machine icon · **pingdotgg#12030** a discovered machine's icon survives a relay
refresh · **pingdotgg#12001** dropped folders become path chips locally and are
refused on remote environments
- **pingdotgg#11144** pull-request icon state centralised — a refactor the fork's
own badge filtering now rides

Not fork surfaces, landed for completeness: the mobile work (pingdotgg#11841,
pingdotgg#12169, pingdotgg#12177, version bump), the CLI installer progress bar (pingdotgg#12044),
docs (pingdotgg#11696), release chores and the Fable 5.1 badge (pingdotgg#12173).

## Unsupported in Moatless / needs implementation

- **Pull request surface** — `FEATURES.pullRequestSurface` is off, so
none of this merge's pull-request work is reachable: **pingdotgg#11994** (submit
PR comments with Cmd/Ctrl+Enter), **pingdotgg#12150** (comments easier to scan,
`apps/web/src/components/pullRequest/**` plus a `pullRequest.ts`
contract field), **pingdotgg#12168** (cached GitHub PR details reused across
entry points), **pingdotgg#12125** and **pingdotgg#11728** (author avatars and their
fallback). **pingdotgg#11706** needs backend work on top: private-repository
media in PR tabs goes through a new `packages/contracts/src/assets.ts`
proxy that Moatless would have to serve. Opening the surface means
deleting the `pullRequestSurface` entry and its gates, and dispatching
`pullRequests.list` / `.detail` / `.activity` — only
`pullRequests.summary` is served today.
- **Keybindings settings page** — **pingdotgg#12175** turns every keybinding
command into a searchable settings row pointing at
`/settings/keybindings`, which `FEATURES.serverAdministration` keeps out
of the sidebar and redirects on a typed URL. The rows still match in
settings search and land on that redirect. Left as-is this merge — it is
the same shape as the six `snap-shot-*` rows that have always done this,
and the one-line fix (a `settingsPathEnabled(item.to)` filter in
`filterAvailableSettingsSearchItems`) is a behaviour change that belongs
outside a merge. Recorded in `gaps.md`. Closes properly when
`server.upsertKeybinding` / `removeKeybinding` are dispatched.
- **Device hub** — **pingdotgg#12017** (detect unsupported legacy Android
command-line tools) and **pingdotgg#12033** (resolve Node for standalone helper
scripts) are both `apps/server/src/device/**`. `FEATURES.deviceHub` is
off and Moatless runs no device host at all, so there is nothing to do
and nothing to reproduce.

## Backend behavior to consider reproducing in Moatless

All recorded in `docs/fork/gaps.md`; nothing in this repository holds
them open.

Checkpoint and turn path, under _Runtime fixes upstream made to its own
server_:

- **pingdotgg#12154** keep large sparse checkouts on the fast checkpoint path —
streams `git ls-files --full-name --sparse -z -v` under a 4 KiB cap and
pins `sparse.expectFilesOutsideOfPatterns=false`. Without it a sparse
checkout large enough to blow the output limit drops to the slow path on
every checkpoint.
- **pingdotgg#10944** flush checkpoint objects and refs before publishing them —
otherwise a reader that acts on the announcement can find a ref pointing
at an object that is not there yet. Rare, unreproducible, permanent when
it lands.
- **pingdotgg#8432** keep a ready checkpoint when a later placeholder arrives
(`ProjectionPipeline.ts`) — the symptom is a checkpoint reverting to
pending and never coming back.
- **pingdotgg#11970** keep VCS waits from blocking turn completion
(`ProviderRuntimeIngestion.ts`, `decider.ts`) — a slow git call between
the provider's last event and the turn being marked done. Slower in a
sandbox than upstream.

Settlement, under _Settlement rules Moatless owns_:

- **pingdotgg#12161** settle on the `thread.pull-request-linked` / `-synced`
event with a per-thread sweep rather than waiting for the next periodic
one.
- **pingdotgg#12176** make the cancellation path uninterruptible around
record-and-rollback, so a cancelled worktree setup records its
settlement instead of being left mid-setup.

Client features that are inert until the backend emits or honours
something:

- **pingdotgg#11784** provider thinking traces — `orchestration` gained a
`reasoning` message role and `thread.message.reasoning.delta` /
`.complete` commands behind a `reasoningMessages: true` opt-in on
subscribe. The client renders them when they arrive; Moatless emits
none, so there are no traces.
- **pingdotgg#10822** complete counts and progressive large diffs —
`review.getDiffPreview` gained an optional `file` input (one file's
patch) and an optional `files` stat array ("absent on older servers").
Moatless dispatches the method and honours neither, so large diffs stay
truncated with incomplete counts.
- **pingdotgg#11519** native provider slash commands, exposed server-side and
consumed by the mobile client.
- **pingdotgg#12115** OpenCode Go, Cursor and Grok subscription limits in the
usage scan.

## Verification

`tripwires`, `duplicate-adds`, `resolution-check`, `inventory-check`,
`unsupported-methods`, `lockfile`, `fmt:check`, `lint` and `typecheck`
all pass; tests pass in all 15 packages. Two failures were found and
fixed on the way:

- `TS2552: Cannot find name 'label'` in `ThreadStatusIndicators.tsx` —
pingdotgg#11104/pingdotgg#11180 hoisted `label` onto the presentation object and the
fork's multi-link popover branch still read the removed local.
- The delta-guard test failure described above.

Two operational notes for the next run are in the tracker entry: `vp i`
needs `NODE_OPTIONS=--max-old-space-size=6144` in this sandbox, and
`--force-with-lease` needs the explicit `<ref>:<sha>` form with the SHA
read from `git ls-remote`, because this clone only fetches `main` and
the branch has no lease-eligible tracking ref.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---
Moatless task:
https://moatless.soaplabstest.com/tasks/e3e17736-1c3d-4873-b9af-c434fd31b003
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant