checkpoint: extract persistent contract to api/checkpoint#1504
Open
Soph wants to merge 4 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Extracts the persistent-checkpoint storage contract into a new github.com/entireio/cli/api/checkpoint package so non-CLI backends can implement persistence without depending on the CLI’s git/TUI/runtime packages, while preserving existing CLI imports via re-exports.
Changes:
- Introduces
api/checkpointcontaining persistent metadata types, read/write interfaces, the sealedWriteRequestunion, helper readers, and sentinel errors. - Re-exports the new contract from
cmd/entire/cli/checkpointviaaliases.goto avoid call-site churn. - Updates the git-backed persistent implementation and tests to use the exported
PrecomputedTranscriptBlobs.IsUsableand the updated “persistent” error message.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/checkpoint/persistent.go | Updates transcript replacement flow to use exported IsUsable on precomputed transcript blobs. |
| cmd/entire/cli/checkpoint/persistent_write.go | Removes local contract types/docs and relies on the re-exported api/checkpoint request union for write dispatch. |
| cmd/entire/cli/checkpoint/persistent_write_test.go | Drops now-unreachable in-package “unknown request” test and keeps per-request dispatch coverage. |
| cmd/entire/cli/checkpoint/persistent_reader.go | Keeps implementation-only helpers/capabilities (AuthorReader, summary normalization) after contract extraction. |
| cmd/entire/cli/checkpoint/persistent_reader_test.go | Updates expected error text to “read persistent checkpoint”. |
| cmd/entire/cli/checkpoint/checkpoint.go | Removes persistent-contract definitions from the implementation package, leaving the generic/ephemeral surface. |
| cmd/entire/cli/checkpoint/aliases.go | Re-exports api/checkpoint symbols (types, interfaces, helpers, sentinel errors) to preserve existing imports. |
| api/checkpoint/doc.go | Adds package docs explaining the extracted persistent contract and why it exists. |
| api/checkpoint/errors.go | Defines sentinel errors and the persistent metadata format constant in the contract package. |
| api/checkpoint/interfaces.go | Defines persistent reader/store interfaces, sealed write union, and helper read functions. |
| api/checkpoint/metadata.go | Hosts persistent metadata/options/types moved out of the CLI implementation package, including IsUsable. |
Soph
added a commit
that referenced
this pull request
Jun 23, 2026
Address Copilot review on #1504: the verbatim-moved WriteOptions/UpdateOptions doc comments still said 'committed checkpoint' (the store concept) — reword to 'persistent checkpoint' to match the rest of the api/checkpoint contract. Rename the ReadCheckpoint tests (TestReadCommittedCheckpoint* -> TestReadCheckpoint*) to match the renamed function. References to the git 'committed tree' and the TotalCommitted/total_committed attribution field are left as-is — those correctly describe git commits, not the store. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 6206fb18d245
Soph
added a commit
that referenced
this pull request
Jun 23, 2026
Address Copilot review on #1504: the verbatim-moved WriteOptions/UpdateOptions doc comments still said 'committed checkpoint' (the store concept) — reword to 'persistent checkpoint' to match the rest of the api/checkpoint contract. Rename the ReadCheckpoint tests (TestReadCommittedCheckpoint* -> TestReadCheckpoint*) to match the renamed function. References to the git 'committed tree' and the TotalCommitted/total_committed attribution field are left as-is — those correctly describe git commits, not the store. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 6206fb18d245
3f7067c to
221c89f
Compare
3210376 to
5a4925d
Compare
Move the persistent-checkpoint contract — the persisted document types (Metadata, CheckpointSummary, Summary, Attribution, ...), the option types (WriteOptions/UpdateOptions/PrecomputedTranscriptBlobs), the reader/writer interfaces, the Write request union, the sentinel errors, and the CheckpointVersionBranchV1 const — into a new github.com/entireio/cli/api/checkpoint package, born with the persistent vocabulary. The contract depends only on leaf packages (agent/types, checkpoint/id, redact, go-git plumbing), so a storage backend can implement it without the CLI's agent/TUI/git machinery. The git implementation (GitStore, Open, the Stores facade, AuthorReader, and the ephemeral shadow-branch surface) stays in cmd/entire/cli/checkpoint, which imports the contract and re-exports every moved symbol via aliases (aliases.go) — all existing call sites compile unchanged. Notes: - PrecomputedTranscriptBlobs.isUsable is now exported (IsUsable); its one caller is across the new package boundary. - The Write union is sealed to api/checkpoint, so the impl-package unknown-request test was removed (no longer expressible); per-request dispatch coverage remains. - normalizeCheckpointSummary stays in the impl (read-time normalization). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: a808b47c0633
Address Copilot review on #1504: the verbatim-moved WriteOptions/UpdateOptions doc comments still said 'committed checkpoint' (the store concept) — reword to 'persistent checkpoint' to match the rest of the api/checkpoint contract. Rename the ReadCheckpoint tests (TestReadCommittedCheckpoint* -> TestReadCheckpoint*) to match the renamed function. References to the git 'committed tree' and the TotalCommitted/total_committed attribution field are left as-is — those correctly describe git commits, not the store. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 6206fb18d245
The four SessionReader methods (ReadSessionMetadata, ReadSessionPrompts, ReadSessionMetadataAndPrompts, ReadSessionContent) each repeated the same prologue: context check, getFetchingTree, navigate to the checkpoint tree, then the session subtree, with identical ErrCheckpointNotFound wrapping. Extract that navigation into getSessionTree so each reader is just "resolve session tree, read the files it needs." Behavior is unchanged — the helper returns the same ErrCheckpointNotFound (wrapping "session N not found" when the session subtree is missing). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: 9cd97a095dda
2904262 to
089e4b6
Compare
pfleidi
reviewed
Jun 23, 2026
pfleidi
approved these changes
Jun 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://entire.io/gh/entireio/cli/trails/647
What
The final step of the #1433 reorder: extract the persistent-checkpoint contract into a new
github.com/entireio/cli/api/checkpointpackage — born with the persistent vocabulary and scope-based names (the whole reason we did the rename before relocating).Moved to
api/checkpoint:Metadata,CheckpointSummary,Summary/LearningsSummary/CodeLearning,Attribution,SessionFilePaths,SessionMetrics,CheckpointInfo,SessionContent.WriteOptions,UpdateOptions,PrecomputedTranscriptBlobs.CheckpointReader(Read,List) +SessionReader(the four session reads), composed intoPersistentStore;Writer+ the sealedWriteRequestunion (Session/SessionTranscript/SessionSummary/CheckpointAttribution);ReadCheckpoint,ReadLatestSessionContent,ReadRawSessionLogForCheckpoint), the sentinel errors, andCheckpointVersionBranchV1.The contract depends only on leaf packages (
agent/types,checkpoint/id,redact, go-gitplumbing) — a backend can implement it without the CLI's agent/TUI/git machinery.Stays in
cmd/entire/cli/checkpoint:GitStore+ methods,Open, theStoresfacade,PersistentRefs,AuthorReader,normalizeCheckpointSummary, and the entire git-only ephemeral surface.Zero call-site churn
cmd/entire/cli/checkpointimports the contract and re-exports every moved symbol via aliases (aliases.go):All existing importers compile unchanged; the backend imports
api/checkpointdirectly. (Helpers are thin wrapper funcs, not mutable vars — addressing the earlier review note.)Cleanup (post-review
/simplifypass)getSessionTreehelper: the fourSessionReadermethods each repeated the same context-check → fetching-tree → checkpoint-tree → session-subtree navigation (with identicalErrCheckpointNotFoundwrapping) before reading their files. Behavior unchanged.Notes
PrecomputedTranscriptBlobs.isUsable→ exportedIsUsable(its one caller is now across the package boundary).Writeunion is sealed toapi/checkpoint, so the impl-package unknown-request test was dropped (no longer expressible); per-request dispatch coverage remains.Verification
go build ./..., fullgo test ./..., andmise run lint(0 issues) green; validated withcodex exec review(which confirmederrors.Isstill works across the alias boundary) and a 4-agent/simplifyreview (reuse / simplification / efficiency / altitude).