Skip to content

Route checkpoint reads by ID kind across git backends#1630

Merged
Soph merged 2 commits into
mainfrom
feat/checkpoint-read-routing
Jul 4, 2026
Merged

Route checkpoint reads by ID kind across git backends#1630
Soph merged 2 commits into
mainfrom
feat/checkpoint-read-routing

Conversation

@Soph

@Soph Soph commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

What

Follow-up #1 from the ULID-emission review (#1629). Reads previously resolved only against the configured primary store, so after a git-branch⇄git-refs flip — or while running the two side by side — a checkpoint stored in the other backend was reported missing (and attach could even suggest an unfixable fetch). This adds id-kind read routing so the CLI resolves a checkpoint by its format, wherever it lives.

Routing rules (per Stefan's spec)

  • ULID → git-refs store only; never the branch (regardless of active backend).
  • hex + git-branch primary → branch only.
  • hex + git-refs primary → refs first, then git-branch fallback (a hex checkpoint may still sit on the pre-migration v1 branch, or have been migrated into refs).
  • List unions both backends; GetCheckpointAuthor routes the same way (AuthorReader preserved).
  • Writes are not kind-routed — they stay on the configured primary (+ mirrors); the minted ID already matches the primary's format.

How

checkpoint.Open now returns a kindRoutingStore wrapping the write fanout plus the two git read stores (reusing the primary for its kind, building the sibling). Every general read path — resume, explain, attribution, blame, tokens, attach — inherits routing for free. No config knob; it's automatic.

Tests

routing_store_test.go covers: ULID→refs (incl. "a ULID never reads from the branch"), hex→branch, hex-fallback and migrated-hex-in-refs under a refs primary, List union, session-read routing, author routing. open_config_test.go updated to the new "Persistent is always the routing store" invariant.

Verified: unit + integration (390) + E2E canary on both backends (git-branch 4/4, git-refs 58/59 +1 known skip).

Not in scope (separate follow-up #2)

The write-boundary guard for the ULID⇒refs invariant (warn/reject when a ULID would be condensed onto a git-branch primary). Left deliberately — it needs topology-role awareness (a git-branch mirror of a git-refs primary legitimately receives ULIDs).

Review updates (commit 80c7a4e)

Addressed the Cursor Bugbot findings and a /simplify reuse cleanup:

  • Fetch errors no longer block the branch fallback (Cursor, medium): firstResolved now falls through to the next store on a non-final store's error as well as on "absent". Under a git-refs primary, a hex read whose refs lookup fails on an on-demand ref fetch (network) still resolves from the git-branch store. The final store's result (hit/absent/error) is returned verbatim, and single-store orders are unchanged.
  • List dedups by checkpoint ID (Cursor, low): a checkpoint present in both backends (a mirrored ULID, or a hex on the branch also migrated into refs) now appears once (most-recent kept).
  • Shared sort helper (/simplify reuse): extracted sortCheckpointInfosByRecency, now used by the git-branch, git-refs, and routing List implementations.
  • New tests: refs-fetch-error-falls-back-to-branch, and List-dedup across backends.

The rest of /simplify was judged already-clean: firstResolved/readOrder stay centralized (the fallback fix depends on it), and "routing-as-a-backend / backends-declare-their-id-kind" was deemed over-engineering for two backends. List querying both backends is intentional (union completeness) — a bounded per-command cost, not a hot path.

🤖 Generated with Claude Code


Note

Medium Risk
Touches the central checkpoint.Open read path for resume, explain, attach, and list; behavior changes are intentional for migration but any routing bug could surface as missing checkpoints.

Overview
Checkpoint reads no longer depend on whichever git backend is configured as primary. checkpoint.Open always exposes a kindRoutingStore around the write fanout (primary + mirrors) plus dedicated git-branch and git-refs read stores, built via buildKindReadStores (reusing the primary where it matches).

Routing behavior: ULID IDs are read only from git-refs; legacy hex IDs follow the active primary (branch-only under git-branch, refs then branch fallback under git-refs). List merges both backends; session reads and GetCheckpointAuthor use the same order; writes still go only through the configured primary and mirrors.

Adds routing_store.go with firstResolved fallback semantics, unit tests for migration/coexistence cases, updates open_config_test.go expectations, and documents read routing in sessions-and-checkpoints.md.

Reviewed by Cursor Bugbot for commit 4a1f5fa. Configure here.

Follow-up #1 from the ULID-emission review: reads resolved only against the
configured primary store, so after a git-branch⇄git-refs flip (or during
coexistence) a checkpoint stored in the other backend was reported missing. Now
`checkpoint.Open` returns a kind-routing store that resolves id-keyed reads by the
checkpoint's format:

- A ULID is read from the git-refs store only — never the branch (any backend).
- A hex ID is read from the active primary first; under a git-refs primary it also
  falls back to the git-branch store (a hex checkpoint may still sit on the
  pre-migration v1 branch, or have been migrated into refs).
- List unions both backends; GetCheckpointAuthor routes the same way (AuthorReader
  preserved). Writes are NOT kind-routed — they stay on the configured primary
  (+ mirrors); the minted ID already matches the primary's format.

The router is built once in Open, reusing the primary for its kind and building
the sibling read store. All the general read paths (resume, explain, attribution,
blame, tokens, attach) inherit routing for free.

Tests: routing_store_test.go covers ULID→refs (incl. "a ULID never reads from the
branch"), hex→branch, hex-fallback and migrated-hex-in-refs under a refs primary,
List union, session-read routing, and author routing. Updated open_config_test to
the new "Persistent is always the routing store" invariant.

Verified: unit + integration (390) + canary both backends (git-branch, git-refs).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: fe977d819cd6
@Soph Soph requested a review from a team as a code owner July 4, 2026 16:05
Copilot AI review requested due to automatic review settings July 4, 2026 16:05

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 4a1f5fa. Configure here.

Comment thread cmd/entire/cli/checkpoint/routing_store.go
Comment thread cmd/entire/cli/checkpoint/routing_store.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds checkpoint read routing by ID kind so checkpoint reads work correctly across git-branch and git-refs backends during migrations or when both backends coexist. The routing is implemented by always returning a kindRoutingStore from checkpoint.Open, allowing the CLI to resolve checkpoints by format (ULID vs legacy hex) without requiring config changes.

Changes:

  • Introduces kindRoutingStore to route id-keyed reads across git-branch and git-refs according to the checkpoint ID kind.
  • Updates checkpoint.Open to always wrap the write fanout with the routing store and to build both read backends for routing.
  • Adds/updates tests and documentation describing the new routing behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
docs/architecture/sessions-and-checkpoints.md Documents the new id-kind read routing and list/write behavior.
cmd/entire/cli/checkpoint/routing_store.go Implements the routing store, including read-order rules and list merging.
cmd/entire/cli/checkpoint/routing_store_test.go Adds unit tests covering routing behavior for ULID vs hex IDs and backend combinations.
cmd/entire/cli/checkpoint/open.go Updates Open to return the routing store and builds both read stores for routing.
cmd/entire/cli/checkpoint/open_config_test.go Updates expectations to match the new “Persistent is always the routing store” invariant.

Comment thread cmd/entire/cli/checkpoint/routing_store.go Outdated
Comment thread cmd/entire/cli/checkpoint/routing_store_test.go
Addresses the two Cursor Bugbot findings on the read-routing PR plus one reuse
cleanup from /simplify (the rest of /simplify was judged already-clean — notably
firstResolved/readOrder stay centralized, which the fallback fix relies on):

- Fetch errors no longer block the branch fallback (Cursor, medium): firstResolved
  now falls through to the next store on a non-final store's error as well as on
  "absent". Under a git-refs primary, a hex read whose refs lookup fails on an
  on-demand ref fetch (network) still resolves from the git-branch store. The
  final store's result (hit/absent/error) is still returned verbatim, and
  single-store orders (ULID→refs, hex under a branch primary) are unchanged.
- List dedups by checkpoint ID (Cursor, low): a checkpoint present in both
  backends (a mirrored ULID, or a hex on the branch also migrated into refs) now
  appears once — keeping the most-recent after the sort.
- Extracted sortCheckpointInfosByRecency, shared by the git-branch, git-refs, and
  routing List implementations (drops the duplicated CreatedAt comparator; refs
  no longer imports sort).

Tests: added refs-fetch-error-falls-back-to-branch and List-dedup cases.

Skipped (noted): inlining firstResolved/readOrder (centralizing is worth more,
especially now the fallback logic lives in one place); the metaAndPrompts wrapper
and build-both-stores (negligible per efficiency review); routing-as-a-backend /
backends-declare-their-id-kind (real generalization but over-engineering for two
backends). List querying both backends is intentional (union completeness) — a
bounded per-command cost, not a hot path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 022f449bbfda
@Soph Soph merged commit 4398a44 into main Jul 4, 2026
20 checks passed
@Soph Soph deleted the feat/checkpoint-read-routing branch July 4, 2026 16:49
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.

3 participants