Skip to content

fix(server): keep ready checkpoints when a later placeholder arrives - #8432

Merged
juliusmarminge merged 2 commits into
pingdotgg:mainfrom
Adolanium:fix/sql-checkpoint-placeholder-guard
Sep 17, 2026
Merged

juliusmarminge merged 2 commits into
pingdotgg:mainfrom
Adolanium:fix/sql-checkpoint-placeholder-guard

Conversation

@Adolanium

@Adolanium Adolanium commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What Changed

The SQL turns projector now refuses a later missing placeholder over a checkpoint that is already ready or error.

In-memory projectEvent already had this guard. SQL always wrote checkpointStatus: event.payload.status. Thread detail and diffs read SQL.

Same-turn placeholder then ready is still the happy path. This only stops a later placeholder from wiping a capture that already landed.

Why

Ingestion and CheckpointReactor dispatch onto the same serial queue. If ready lands first, a later missing was a no-op in memory and a clobber in SQL.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes (N/A)
  • I included a video for animation/interaction changes (N/A)

Note

Low Risk
Narrow projection guard on duplicate turn-diff events; improves read-model consistency for checkpoints with limited blast radius.

Overview
Aligns the SQL turn projector with in-memory event projection so a later thread.turn-diff-completed with missing cannot overwrite a turn that already has a real checkpoint (ready or error).

When ready is projected before a trailing placeholder (e.g. shared serial queue ordering), projection_turns used to always take the latest status, so thread detail and diffs could show missing even after a successful capture. The handler now no-ops that downgrade; placeholder-then-ready on the same turn is unchanged.

Adds an integration test that projects ready then missing for the same turn and asserts SQL still stores ready and the checkpoint ref.

Reviewed by Cursor Bugbot for commit c5e84c9. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix ProjectionPipeline to keep ready checkpoints when a later missing placeholder arrives

  • In the thread.turn-diff-completed handler in ProjectionPipeline.ts, adds an early return guard: if an existing turn projection already has a non-null, non-missing checkpointStatus, incoming events with status missing are ignored for that turn.
  • Adds a test in ProjectionPipeline.test.ts that projects a ready checkpoint followed by a missing event for the same turn and asserts the ready status and original checkpointRef are retained.

Macroscope summarized c5e84c9.

Summary by CodeRabbit

  • Bug Fixes
    • Preserved existing ready checkpoints when a later update reports a missing checkpoint.
    • Prevented valid checkpoint references and associated files from being replaced by a missing status.

The in-memory projector already refuses a missing placeholder over a ready capture. SQL always wrote the later status. Thread detail and diffs read SQL, so a concurrent placeholder could hide a finished checkpoint.

Skip the SQL write when the stored checkpoint is not missing and the new event is.
@coderabbitai

coderabbitai Bot commented Aug 27, 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: Advanced

Run ID: 0931882d-6044-4fe3-81f5-95e4d94af2ad

📥 Commits

Reviewing files that changed from the base of the PR and between 8130a9f and cb2898f.

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

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


📝 Walkthrough

Walkthrough

The projection pipeline now ignores a later "missing" checkpoint update when the turn already has a real checkpoint. A regression test verifies that the existing status and reference remain unchanged.

Changes

Checkpoint preservation

Layer / File(s) Summary
Preserve projected checkpoints
apps/server/src/orchestration/Layers/ProjectionPipeline.ts, apps/server/src/orchestration/Layers/ProjectionPipeline.test.ts
The projection returns early for a "missing" update when a real checkpoint already exists. The regression test verifies that the "ready" status and original checkpoint reference remain stored.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to cb289

The projection preserves completed checkpoints when later placeholder events arrive, with regression coverage for the ready-to-missing sequence. The change is ready to merge.

🚥 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 fix: preserving ready checkpoints when a later missing placeholder arrives.
Description check ✅ Passed The description includes the required What Changed, Why, and Checklist information. It explains the SQL projector bug, the intended behavior, the unchanged placeholder-then-ready flow, and the test co…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Aug 27, 2026
@macroscopeapp

macroscopeapp Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at c5e84c9

Macroscope's review found this PR approvable — This is a narrow read-model consistency fix: only a later missing checkpoint event is ignored when a real checkpoint already exists, preserving the stored status and git reference. The accompanying integration test covers the affected ordering, with no schema or cross-cutting runtime changes.

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

@juliusmarminge

Copy link
Copy Markdown
Member

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@juliusmarminge
juliusmarminge merged commit d42a3bd into pingdotgg:main Sep 17, 2026
19 checks passed
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
AIdoesmyjob pushed a commit to AIdoesmyjob/t3code that referenced this pull request Sep 18, 2026
…ingdotgg#8432)

Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
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:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants