Isolate Token Store in CLI Tests · Entire

Phase 0 Implementation Summary

New seam (cmd/entire/cli/checkpoint/open.go): Open(ctx, repo, OpenOptions) (*Stores, error) — the single construction point, with OpenOptions{BlobFetcher, Settings, Refs} and the facade Stores{Primary} plus accessors Temporary(), Refs(), Repository().

Migrated all 26 production NewGitStore(...) sites through Open, across cli (resume, attach, explain, rewind, attribution, review_context, head_checkpoint_flags), dispatch, strategy (the getCheckpointStore seam + cleanup/common/hooks/rewind/condensation), and the in-package checkpoint.LookupSessionLog. Accessor sites (Refs(), Repository()) and temporary-method sites now route through the facade so Phase 1 changes internals only.

Verification: go build ./... ✓, mise run lint → 0 issues, checkpoint/strategy/dispatch tests ✓, full cli package ✓.

Decisions Worth Flagging

  1. Type is checkpoint.Stores, not CheckpointStores — revive flags the latter as a stutter; checkpoint.Stores is idiomatic and reads cleanly (stores.Primary). Trivial to rename if you want it to match the issue verbatim.
  2. getCheckpointStore now returns (*GitStore, error) (propagated through its 7 callers); folded the old withBlobFetcher into OpenOptions.BlobFetcher.
  3. generateCheckpointSummary now takes the facade (it needs both committed-write and Repository() — clean migrate-once).
  4. Open's always-nil error has a //nolint:unparam explaining it's the forward-looking contract per the issue.
  5. No-behavior-change preserved: I initially had the summary mirror read stores.Refs(), which broke REDACTED (the test opens its store with DefaultV1Refs but opts into the mirror via settings). Reverted that one line back to ResolveCommittedRefs(ctx).
  6. Left the ~131 test-file sites and benchutil on NewGitStore (test infra; NewGitStore is retained as Open's internal constructor).

Recommendations

Do Open now, skip the literal shared singleton. It captures ~all the real benefit, keeps Phase 0 a clean no-behavior-change mechanical diff (which is the whole point of landing it first), and the singleton can be layered on trivially later if profiling ever shows repo-opens mattering. attach stays the documented exception either way since it needs PrimaryAsRead().

Branch Summary

Branch 1 — soph/pluggable-stores-phase0 (commit 18b938d6a) Phase 0 of issue #1433: centralized checkpoint store construction behind checkpoint.Open(ctx, repo, OpenOptions). Migrated all 26 production NewGitStore(...) sites; facade exposes Primary/Temporary()/Refs()/Repository() so Phase 1 only changes internals. Build ✓, lint 0 issues, checkpoint/strategy/dispatch/cli tests ✓.

Branch 2 — soph/cli-testmain-token-isolation (commit 6a2ed442d, off main) Fixes the keychain prompt you hit. The cli TestMain only mocked the in-process zalando keyring; tests that spawn the real entire binary (or a git hook) run in a child where testing.Testing() is false, so the child's tokenstore default backend reached your real OS keychain. Now TestMain sets ENTIRE_TOKEN_STORE=file (+ isolated token/auth/config/cache paths) process-wide before m.Run, so children inherit file-backed isolation. Verified by running go test ./cmd/entire/cli/ with the external env vars explicitly unset — passes, no keychain prompt.