fix: clarify external checkpoint discovery warning#889
fix: clarify external checkpoint discovery warning#889peyton-alt wants to merge 3 commits intomainfrom
Conversation
fe2de3e to
b2d247b
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves user feedback after pushing checkpoints to an external checkpoint remote by warning (once per process) when .entire/settings.json isn’t tracked, clarifying that entire.io may not be able to discover the checkpoint remote. It also adds support for compacting GitHub Copilot CLI transcripts into the normalized transcript.jsonl format.
Changes:
- Emit a consequence-first post-push note for external checkpoint remote pushes when
.entire/settings.jsonis untracked (deduped viasync.Once). - Add Copilot transcript compaction (
assistant.message/tool.execution_complete→ inlined tool results) plus fixtures and a unit test. - Rename the Codex
"message"constant to a sharedtranscriptTypeMessagefor reuse in format detection.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/strategy/push_common.go | Adds printSettingsCommitHint + isSettingsTrackedByGit to print the clarified warning after successful URL pushes. |
| cmd/entire/cli/strategy/push_v2.go | Invokes the new post-push settings hint after successful v2 ref pushes. |
| cmd/entire/cli/strategy/push_common_test.go | Adds tests covering settings tracking detection and hint printing/deduping. |
| cmd/entire/cli/transcript/compact/compact.go | Routes Copilot-format transcripts through the new compactor. |
| cmd/entire/cli/transcript/compact/copilot.go | Implements Copilot JSONL detection and compaction with inlined tool results. |
| cmd/entire/cli/transcript/compact/copilot_test.go | Adds fixture-based test for Copilot compaction. |
| cmd/entire/cli/transcript/compact/testdata/copilot_full.jsonl | Adds a Copilot raw transcript fixture. |
| cmd/entire/cli/transcript/compact/testdata/copilot_expected.jsonl | Adds expected normalized output for the Copilot fixture. |
| cmd/entire/cli/transcript/compact/codex.go | Renames the "message" type constant used by Codex parsing. |
| old := os.Stderr | ||
| r, w, err := os.Pipe() | ||
| require.NoError(t, err) | ||
| os.Stderr = w | ||
|
|
||
| printSettingsCommitHint(context.Background(), "origin") | ||
|
|
||
| w.Close() | ||
| var buf bytes.Buffer | ||
| if _, readErr := buf.ReadFrom(r); readErr != nil { | ||
| t.Fatalf("read pipe: %v", readErr) | ||
| } | ||
| os.Stderr = old | ||
|
|
There was a problem hiding this comment.
These tests temporarily replace the process-wide os.Stderr but don’t restore it via t.Cleanup/defer, so a t.Fatalf/require failure before restoration can leak the redirected stderr into later tests. Consider using t.Cleanup to always restore os.Stderr (see strategy/checkpoint_token_test.go for an example), and also close both pipe ends via defer to avoid FD leaks on early returns.
8ffca11 to
4926cc9
Compare
When pushing checkpoints to an external checkpoint remote, entire.io needs .entire/settings.json committed to discover where checkpoint data lives. Show a note after successful pushes when settings.json is not tracked by git. Refs: #859 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: a887063691eb
Address review feedback: - Use sync.Once to print the settings hint at most once per push - Use repo-root-relative pathspec (:/) for git ls-files to work correctly from subdirectories - Add tests for isSettingsTrackedByGit and printSettingsCommitHint Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 06fbfcec1fa0
Entire-Checkpoint: d2972e70b180
4926cc9 to
ed5414c
Compare
When checkpoints are pushed to a separate checkpoint remote, the push can succeed even though entire.io may not be able to discover that remote from the repository. The old hint only told the user to commit .entire/settings.json, but it didn’t explain the consequence.
This change makes the post-push warning consequence-first:
[entire] Note: Commit and push .entire/settings.json or entire.io may not find checkpoints on this remote.
Scope:
Refs: #859
Note
Medium Risk
Adds a new post-push hint gated by an external checkpoint URL and a
git ls-filescheck, plus introduces a new Copilot JSONL compaction path; both affect user-facing CLI output and transcript parsing behavior.Overview
After successful pushes to an external checkpoint URL remote, the CLI now emits a consequence-first hint when
/.entire/settings.jsonis not tracked by git, and ensures this message prints only once per process viasync.Once(applied to both v1doPushBranchand v2doPushRef).The transcript compactor gains Copilot JSONL support: format detection is added post-truncation, a new
compactCopilotconverter inlinestool.execution_completeresults into precedingtool_useblocks, and fixture-based tests + sample data validate the transformation.Reviewed by Cursor Bugbot for commit fe2de3e. Configure here.