From 783582781e5817360b270701cc3c5a8bea7382cc Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Fri, 7 Aug 2026 16:27:48 +0200 Subject: [PATCH 1/9] fix(checkpoint): make git-refs writes respect cancellation, but never drop push bookkeeping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The git-refs store was the one write path with no brake on a canceled context. The git-branch store guards its writes, and this store's own backfill/read methods guard theirs, but writeSession — the create path — did not. Nothing downstream of it observes ctx either (go-git object and ref writes and CreateCommit all ignore it), so a bulk writer that itself ignored cancellation kept minting checkpoints after Ctrl-C. The one step that DID observe the canceled ctx was the push-queue resolution, which shells out to `git rev-parse --git-common-dir` — so every post-cancel write logged "resolve push queue failed; ref not enqueued" and left its ref out of the queue. That is worse than noisy: the queue is the only push-discovery mechanism there is, and the writers are idempotent, so no later run re-enqueues a missed ref. Those checkpoints were stranded locally forever. Resolve the queue with the cancellation stripped instead. By that point the ref is already on disk, so the bookkeeping that makes it pushable has to complete too — it is a local, sub-millisecond step, and a second Ctrl-C still force-quits the process. Co-Authored-By: Claude Opus 5 (1M context) Entire-Checkpoint: 01KZE9Z15Z7HP0JS2HA7SP00RM --- cmd/entire/cli/checkpoint/refs_store.go | 21 +++++++- cmd/entire/cli/checkpoint/refs_store_test.go | 57 ++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/cmd/entire/cli/checkpoint/refs_store.go b/cmd/entire/cli/checkpoint/refs_store.go index 997ef736bb..0e181bb659 100644 --- a/cmd/entire/cli/checkpoint/refs_store.go +++ b/cmd/entire/cli/checkpoint/refs_store.go @@ -212,8 +212,19 @@ func (s *gitRefsStore) setRef(ctx context.Context, cid id.CheckpointID, hash plu // enqueueForPush records refName in the push-discovery queue, logging (never // returning) on failure so the local ref write still succeeds. +// +// The queue is resolved with the cancellation stripped from ctx. By the time we +// get here the ref is already written locally (go-git ref writes don't observe +// ctx), and the queue is the ONLY push-discovery mechanism there is — see the +// pushQueueFileName doc. A ref that misses the queue is never pushed, and the +// writers above are idempotent, so a re-run skips it as already-present and +// never re-enqueues it: it stays local-only forever. Resolving the queue shells +// out to `git rev-parse --git-common-dir`, which fails instantly on a canceled +// ctx, so honoring cancellation here would drop the bookkeeping for a write +// that already happened. This local, sub-millisecond step therefore completes +// even during shutdown (a second Ctrl-C still force-quits the process). func (s *gitRefsStore) enqueueForPush(ctx context.Context, refName plumbing.ReferenceName) { - q, err := PushQueueForRepo(ctx, s.repo) + q, err := PushQueueForRepo(context.WithoutCancel(ctx), s.repo) if err != nil { logging.Warn(ctx, "checkpoint: resolve push queue failed; ref not enqueued", slog.String("ref", refName.String()), slog.String("error", err.Error())) @@ -226,6 +237,14 @@ func (s *gitRefsStore) enqueueForPush(ctx context.Context, refName plumbing.Refe } func (s *gitRefsStore) writeSession(ctx context.Context, opts WriteOptions) error { + // Parity with the backfill writers above: a canceled ctx means stop doing + // work, and creating a checkpoint is the most expensive write there is + // (tree building plus a commit). Without this a bulk writer that ignores + // cancellation — `entire import` was one — keeps minting checkpoints after + // Ctrl-C. + if err := ctx.Err(); err != nil { + return err //nolint:wrapcheck // Propagating context cancellation + } if opts.CheckpointID.IsEmpty() { return errors.New("invalid checkpoint options: checkpoint ID is required") } diff --git a/cmd/entire/cli/checkpoint/refs_store_test.go b/cmd/entire/cli/checkpoint/refs_store_test.go index 776fe23428..bc341852cc 100644 --- a/cmd/entire/cli/checkpoint/refs_store_test.go +++ b/cmd/entire/cli/checkpoint/refs_store_test.go @@ -737,3 +737,60 @@ func TestGitRefsStore_BackfillUnknownCheckpointNotFound(t *testing.T) { require.NoError(t, err) assert.Nil(t, summary) } + +// TestGitRefsStore_WriteRefusesCanceledContext proves a canceled context stops +// the refs store from minting checkpoints, matching the git-branch store and +// this store's own backfill writers. Without it a bulk writer that ignores +// cancellation — `entire import`, which `entire enable` runs on a first-time +// repo — kept creating checkpoints after Ctrl-C. +func TestGitRefsStore_WriteRefusesCanceledContext(t *testing.T) { + t.Parallel() + store := newRefsStore(t) + cid := id.MustCheckpointID("a1b2c3d4e5f6") + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := store.Write(ctx, Session{ + CheckpointID: cid, + SessionID: "sess-1", + Strategy: "manual-commit", + Transcript: redact.AlreadyRedacted([]byte("transcript")), + AuthorName: "Test Author", + AuthorEmail: "test@example.com", + }) + require.ErrorIs(t, err, context.Canceled) + + _, refErr := store.repo.Reference(mustRefName(t, cid), true) + assert.ErrorIs(t, refErr, plumbing.ErrReferenceNotFound, + "a write refused for cancellation must not leave a checkpoint ref behind") +} + +// TestGitRefsStore_EnqueuesForPushDuringShutdown proves the push-queue record +// still lands when the context is already canceled. By the time setRef runs the +// ref is on disk (go-git ref writes don't observe ctx) and the queue is the only +// push-discovery mechanism there is, so honoring the cancellation here would +// strand the checkpoint locally forever: writers are idempotent, so no later run +// re-enqueues it. Resolving the queue shells out to git, which is what made this +// the one step on the write path that failed under a canceled ctx — the source +// of the "resolve push queue failed; ref not enqueued" warning flood. +func TestGitRefsStore_EnqueuesForPushDuringShutdown(t *testing.T) { + t.Parallel() + store := newRefsStore(t) + cid := id.MustCheckpointID("a1b2c3d4e5f6") + + head, err := store.repo.Head() + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + require.NoError(t, store.setRef(ctx, cid, head.Hash())) + + q, err := PushQueueForRepo(context.Background(), store.repo) + require.NoError(t, err) + refs, err := q.Drain() + require.NoError(t, err) + assert.Contains(t, refs, mustRefName(t, cid), + "a ref written during shutdown must still be queued for push") +} From a3677ff8990271979ab135da4895e57b1c938b57 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Fri, 7 Aug 2026 16:27:58 +0200 Subject: [PATCH 2/9] fix(import): stop importing agent history when the context is canceled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit agentimport.Run had no cancellation checks in its session or turn loops, and nothing on its write path supplies one, so Ctrl-C during an import did not stop it — the run continued to completion, writing a checkpoint per turn for history the user had just asked it to stop importing. This surfaced through `entire enable`, which on a first-time repo defaults to the git-refs checkpoint backend and then offers to import pre-existing agent history. The git-branch store happened to reject writes on a canceled context; git-refs did not, leaving that configuration with no brake at all. Check per turn, not just per session: one session can carry hundreds of turns, and each turn is a checkpoint write. Co-Authored-By: Claude Opus 5 (1M context) Entire-Checkpoint: 01KZE9ZAM96YPYDZTB9MJAWAJW --- cmd/entire/cli/agentimport/agentimport.go | 12 +++ .../cli/agentimport/agentimport_test.go | 89 +++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/cmd/entire/cli/agentimport/agentimport.go b/cmd/entire/cli/agentimport/agentimport.go index 744a7d1816..9ee7f374b0 100644 --- a/cmd/entire/cli/agentimport/agentimport.go +++ b/cmd/entire/cli/agentimport/agentimport.go @@ -215,6 +215,13 @@ func Run(ctx context.Context, repo *git.Repository, imp Importer, opts Options) authorName, authorEmail := cp.GetGitAuthorFromRepo(repo) for sessionIndex, sf := range files { + // Ctrl-C must stop the import. Nothing else on this path observes + // cancellation — go-git object/ref writes and CreateCommit all ignore + // ctx — so without these checks an interrupted import runs to + // completion, silently minting checkpoints the user asked to stop. + if err := ctx.Err(); err != nil { + return res, err //nolint:wrapcheck // propagate context cancellation + } res.SessionsScanned++ full, readErr := os.ReadFile(sf.Path) if readErr != nil { @@ -233,6 +240,11 @@ func Run(ctx context.Context, repo *git.Repository, imp Importer, opts Options) var red redact.RedactedBytes redacted := false for turnIndex, turn := range turns { + // Per-turn, not just per-session: one session can carry hundreds of + // turns, and each turn is a checkpoint write. + if err := ctx.Err(); err != nil { + return res, err //nolint:wrapcheck // propagate context cancellation + } cid := DeriveCheckpointID(sf.SessionID, turn.UUID) if existing[cid.String()] { res.TurnsSkipped++ diff --git a/cmd/entire/cli/agentimport/agentimport_test.go b/cmd/entire/cli/agentimport/agentimport_test.go index d85a0e8b1f..4dc046be0d 100644 --- a/cmd/entire/cli/agentimport/agentimport_test.go +++ b/cmd/entire/cli/agentimport/agentimport_test.go @@ -2,6 +2,7 @@ package agentimport import ( "context" + "errors" "os" "path/filepath" "strings" @@ -619,3 +620,91 @@ func TestRun_CodexImportSanitizesAndKeepsOffsetsAligned(t *testing.T) { "(CheckpointTranscriptStart from raw line indices) would drift", got, rawLines) } } + +// TestRun_StopsOnContextCancellation proves Ctrl-C mid-import halts Run's own +// loops, independently of whether the configured checkpoint store happens to +// reject a canceled write. DryRun writes nothing, so the loop checks are the +// only thing that can stop this run: without them Run walks every remaining +// session and turn after the cancel. +func TestRun_StopsOnContextCancellation(t *testing.T) { + t.Parallel() + repo, repoDir := initRepoWithCommit(t) + claudeDir := t.TempDir() + writeFixtureSession(t, claudeDir, "sess1.jsonl") + writeFixtureSession(t, claudeDir, "sess2.jsonl") + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Cancel as soon as the first turn is processed, standing in for a Ctrl-C + // during the import. DryRun reports every turn through TurnSkipped. + opts := Options{ + RepoRoot: repoDir, + OverridePath: claudeDir, + Now: time.Date(2026, 6, 25, 0, 0, 0, 0, time.UTC), + DryRun: true, + Progress: &Progress{TurnSkipped: func(int, int, int) { cancel() }}, + } + + res, err := Run(ctx, repo, claudeImporter{}, opts) + if !errors.Is(err, context.Canceled) { + t.Fatalf("Run() error = %v, want context.Canceled", err) + } + // 4 turns across 2 sessions; the cancel lands after the first. + if res.TurnsImported != 1 { + t.Fatalf("run continued past cancellation: processed %d turns, want 1 (%+v)", + res.TurnsImported, res) + } + if res.SessionsScanned != 1 { + t.Fatalf("run walked into session %d after cancellation, want to stop at 1", + res.SessionsScanned) + } +} + +// TestRun_CancellationStopsRefsBackedImport is the end-to-end regression for +// the reported bug: `entire enable` defaults a first-time repo to the git-refs +// checkpoint backend and then offers to import agent history. Unlike the +// git-branch store, the git-refs store did not reject writes on a canceled +// context, and nothing else on that path observes ctx (go-git object/ref +// writes and CreateCommit all ignore it) — so Ctrl-C left the import running +// to completion, minting checkpoints the user had just asked to stop. +func TestRun_CancellationStopsRefsBackedImport(t *testing.T) { + // Not parallel: sets the checkpoint backend via the environment. + t.Setenv("ENTIRE_CHECKPOINTS_PRIMARY", "git-refs") + + repo, repoDir := initRepoWithCommit(t) + claudeDir := t.TempDir() + writeFixtureSession(t, claudeDir, "sess1.jsonl") + writeFixtureSession(t, claudeDir, "sess2.jsonl") + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + opts := Options{ + RepoRoot: repoDir, + OverridePath: claudeDir, + Now: time.Date(2026, 6, 25, 0, 0, 0, 0, time.UTC), + Progress: &Progress{TurnWritten: func(int, int, int) { cancel() }}, + } + + res, err := Run(ctx, repo, claudeImporter{}, opts) + if !errors.Is(err, context.Canceled) { + t.Fatalf("Run() error = %v, want context.Canceled", err) + } + if res.TurnsImported != 1 { + t.Fatalf("import continued past cancellation: TurnsImported = %d, want 1 (%+v)", + res.TurnsImported, res) + } + + stores, err := cp.Open(context.Background(), repo, cp.OpenOptions{}) + if err != nil { + t.Fatal(err) + } + infos, err := stores.Persistent.List(context.Background()) + if err != nil { + t.Fatal(err) + } + if len(infos) != 1 { + t.Fatalf("wrote %d checkpoints after cancellation, want 1 (the in-flight turn)", len(infos)) + } +} From 15fc36d5185f3bb8e2da0af0be1787e2cc7182e0 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Fri, 7 Aug 2026 16:28:15 +0200 Subject: [PATCH 3/9] fix(import): report an interrupted import instead of failing or continuing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With cancellation now propagating out of agentimport.Run, both callers handled it as an ordinary error. The enable-time offer logged "session import failed" and moved on to the NEXT agent's history — the last thing a user who just hit Ctrl-C wants — and `entire import` surfaced a bare "context canceled". An interruption is a resumable state, not a failure: turns already written stay written and a re-run picks up where this one stopped. Say so, name the command that finishes the job, and stop the enable-time loop rather than starting the next agent. Co-Authored-By: Claude Opus 5 (1M context) Entire-Checkpoint: 01KZE9ZTJZM1XT4D0V93S93FZQ --- cmd/entire/cli/import_cmd.go | 10 ++++ cmd/entire/cli/setup_import.go | 14 +++++ cmd/entire/cli/setup_import_test.go | 82 +++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+) diff --git a/cmd/entire/cli/import_cmd.go b/cmd/entire/cli/import_cmd.go index bd00c71547..64246184c1 100644 --- a/cmd/entire/cli/import_cmd.go +++ b/cmd/entire/cli/import_cmd.go @@ -1,6 +1,8 @@ package cli import ( + "context" + "errors" "fmt" "time" @@ -89,6 +91,14 @@ fails even with --dry-run.`, imp.AgentType()), }) stopProgress(err == nil) if err != nil { + // Ctrl-C is not a failure: report the partial import (turns + // already written stay written, and a re-run resumes where + // this one stopped) instead of a raw "context canceled". + if errors.Is(err, context.Canceled) { + c.SilenceUsage = true + fmt.Fprintf(c.OutOrStdout(), "Import interrupted after %d turn(s). Re-run to finish.\n", res.TurnsImported) + return NewSilentError(err) + } return fmt.Errorf("import %s: %w", imp.Name(), err) } verb := "Imported" diff --git a/cmd/entire/cli/setup_import.go b/cmd/entire/cli/setup_import.go index fcdb3997c8..46d0856f5a 100644 --- a/cmd/entire/cli/setup_import.go +++ b/cmd/entire/cli/setup_import.go @@ -2,6 +2,7 @@ package cli import ( "context" + "errors" "fmt" "io" "time" @@ -231,6 +232,19 @@ func runSelectedImports(ctx context.Context, w io.Writer, repoRoot string, selec }) stopProgress(err == nil) if err != nil { + // Ctrl-C: stop here instead of moving on to the next agent's + // history, which is the last thing a user who just interrupted + // wants. Report what landed and how to finish it. + if errors.Is(err, context.Canceled) { + logging.Info(ctx, "session import interrupted", + "agent", e.imp.Name(), "turns", res.TurnsImported) + if res.TurnsImported > 0 { + importedLocalHistory = true + } + fmt.Fprintf(w, "Import interrupted after %d turn(s). Run 'entire import %s' to finish.\n", + res.TurnsImported, e.imp.Name()) + break + } logging.Warn(ctx, "session import failed", "agent", e.imp.Name(), "error", err) fmt.Fprintf(w, "Note: could not import %s history: %v\n", e.displayName, err) continue diff --git a/cmd/entire/cli/setup_import_test.go b/cmd/entire/cli/setup_import_test.go index 4982ee551b..2e59569b12 100644 --- a/cmd/entire/cli/setup_import_test.go +++ b/cmd/entire/cli/setup_import_test.go @@ -446,3 +446,85 @@ func TestRunSelectedImports_NonTTYProgressLines_Reimport(t *testing.T) { t.Errorf("re-import summary line missing or wrong; want %q in:\n%s", want, out) } } + +// cancelOnDiscoverImporter cancels the run's context from Discover, standing in +// for a Ctrl-C landing while the first agent's history is being imported. +type cancelOnDiscoverImporter struct { + agentimport.Importer + + sessions []agentimport.SessionFile + cancel context.CancelFunc +} + +func (c cancelOnDiscoverImporter) Discover(string, string, time.Time, []string) ([]agentimport.SessionFile, error) { + c.cancel() + return c.sessions, nil +} + +// recordDiscoverImporter records whether the import loop reached it at all. +type recordDiscoverImporter struct { + agentimport.Importer + + reached *bool +} + +func (r recordDiscoverImporter) Discover(string, string, time.Time, []string) ([]agentimport.SessionFile, error) { + *r.reached = true + return nil, nil +} + +// TestRunSelectedImports_InterruptedStopsBeforeNextAgent proves Ctrl-C during +// one agent's import ends the whole offer instead of moving straight on to the +// next agent's history — the last thing a user who just interrupted wants — +// and that the interruption is reported as a resumable state rather than a +// bare "context canceled" failure note. +func TestRunSelectedImports_InterruptedStopsBeforeNextAgent(t *testing.T) { + // Not parallel: chdirs into a temp repo and performs real checkpoint writes. + dir := t.TempDir() + testutil.InitRepo(t, dir) + testutil.WriteFile(t, dir, "f.txt", "x") + testutil.GitAdd(t, dir, "f.txt") + testutil.GitCommit(t, dir, "init") + t.Chdir(dir) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + sessionsDir := t.TempDir() + writeImportProgressFixtureSession(t, sessionsDir, "sess1.jsonl") + + var claudeImp agentimport.Importer + for _, imp := range agentimport.All() { + if imp.Name() == testAgentName { + claudeImp = imp + } + } + if claudeImp == nil { + t.Fatal("claude-code importer not registered") + } + sessions, err := claudeImp.Discover(dir, sessionsDir, time.Now(), nil) + if err != nil { + t.Fatalf("discover fixture sessions: %v", err) + } + + var secondReached bool + var buf bytes.Buffer + runSelectedImports(ctx, &buf, dir, []eligibleImport{ + {imp: cancelOnDiscoverImporter{Importer: claudeImp, sessions: sessions, cancel: cancel}, displayName: testAgentClaude}, + {imp: recordDiscoverImporter{Importer: claudeImp, reached: &secondReached}, displayName: "Cursor"}, + }) + out := buf.String() + + if secondReached { + t.Error("import continued to the next agent after the run was interrupted") + } + if !strings.Contains(out, "Import interrupted") { + t.Errorf("interruption not reported to the user, got %q", out) + } + if want := "entire import " + testAgentName; !strings.Contains(out, want) { + t.Errorf("missing resume hint %q in output %q", want, out) + } + if strings.Contains(out, "could not import") { + t.Errorf("interruption reported as an import failure: %q", out) + } +} From e5d55dfa7be3d52a89fb3b2c5a293c4c0ae87951 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Fri, 7 Aug 2026 16:28:25 +0200 Subject: [PATCH 4/9] fix(enable): route setup's logging to .entire/logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `entire enable` never called logging.Init, so the package logger stayed nil and every logging.* call under setup fell back to slog.Default() — which writes to the terminal through the std log package and puts nothing in the log file. Agent detection, hook install, the session import, and the checkpoint layer's push and remote warnings all landed mid-flow on the user's screen, interleaved with the import spinner's line redraws. That is how a Ctrl-C'd enable-time import came to flood a terminal with "resolve push queue failed; ref not enqueued" lines. The refs store even documents the assumption this broke: "logging.Warn alone lands only in .entire/logs/". Placed after the git-repo check so it cannot create .entire/logs/ outside a repository. Co-Authored-By: Claude Opus 5 (1M context) Entire-Checkpoint: 01KZEA04HEXTJ5GH14VWWAZP1Z --- .../integration_test/enable_import_test.go | 22 +++++++++++++++++++ cmd/entire/cli/setup.go | 13 +++++++++++ 2 files changed, 35 insertions(+) diff --git a/cmd/entire/cli/integration_test/enable_import_test.go b/cmd/entire/cli/integration_test/enable_import_test.go index 027e2bec30..3e61851e18 100644 --- a/cmd/entire/cli/integration_test/enable_import_test.go +++ b/cmd/entire/cli/integration_test/enable_import_test.go @@ -96,3 +96,25 @@ func TestEnableOffersImport_NotOfferedOnReEnable(t *testing.T) { require.NotContains(t, second, "already imported", "re-enable must not run import at all; got: %s", second) } + +// TestEnable_RoutesLoggingToLogFile proves `entire enable` initializes file +// logging like every other command. Without it the package logger stays nil +// and every logging.* call under setup — agent detection, hook install, the +// session import, the checkpoint layer's push and remote warnings — falls back +// to slog.Default(), which prints them straight onto the user's terminal +// mid-flow and writes nothing to .entire/logs/. That is how a Ctrl-C'd +// enable-time import came to flood a user's terminal with "resolve push queue +// failed; ref not enqueued" lines. +func TestEnable_RoutesLoggingToLogFile(t *testing.T) { + t.Parallel() + env := freshRepoEnv(t) + logPath := filepath.Join(env.RepoDir, ".entire", "logs", "entire.log") + + // Nothing has run in this repo yet, so the log file's existence after + // enable can only come from enable itself. + require.NoFileExists(t, logPath) + + out := env.RunCLI("enable", "--agent", agentClaudeCode, "--telemetry=false") + require.Contains(t, out, "Ready.", "enable should complete; got: %s", out) + require.FileExists(t, logPath, "enable should route its logging to .entire/logs; got output: %s", out) +} diff --git a/cmd/entire/cli/setup.go b/cmd/entire/cli/setup.go index dcf53df0f8..b3475048d8 100644 --- a/cmd/entire/cli/setup.go +++ b/cmd/entire/cli/setup.go @@ -883,6 +883,19 @@ for you and (optionally) create a matching GitHub repository via the gh CLI.`, }() } + // Route setup's logging to .entire/logs/ like every other command. + // Without Init the package logger stays nil and every logging.* + // call under setup — agent detection, hook install, session import, + // the checkpoint layer's push/remote warnings — falls back to + // slog.Default(), which prints them straight onto the user's + // terminal mid-flow (and writes nothing to the log file). Placed + // after the git-repo check above so it cannot create .entire/logs/ + // outside a repository. + logging.SetLogLevelGetter(GetLogLevel) + if err := logging.Init(ctx, ""); err == nil { + defer logging.Close() + } + if err := validateSetupFlags(opts.UseLocalSettings, opts.UseProjectSettings); err != nil { return err } From 33736868298d27c32c1bc6af0293d0867ce16b90 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Fri, 7 Aug 2026 16:41:47 +0200 Subject: [PATCH 5/9] feat(enable)!: take history import out of --yes, behind an explicit --import-history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--yes` means "accept all defaults", and the interactive import default is to import nothing — the multi-select pre-checks nothing, so pressing enter through it skips. Implying an import from --yes therefore made the unattended path do the opposite of the attended one. It also let a single flag carry two unrelated decisions. In a non-repo directory `--yes` already means "init git, create a private GitHub repo, commit, and push"; folding history import in meant one flag both created a remote and ingested a month of local transcripts destined for it. Publishing existing agent history is its own decision. `--import-history` is now the non-interactive way to say yes. `--yes` takes the same path as any other run that makes no choice: import nothing, point at `entire import `. The flag is first-run only, like the offer it drives, and says so when it cannot apply rather than being silently dropped. BREAKING CHANGE: `entire enable --yes` no longer imports pre-existing agent history. Pass --import-history, or run `entire import `. Co-Authored-By: Claude Opus 5 (1M context) Entire-Checkpoint: 01KZEARMVQXP70HGB15MXFPAX9 --- .../integration_test/enable_import_test.go | 54 +++++++++++-- cmd/entire/cli/setup.go | 18 ++++- cmd/entire/cli/setup_import.go | 52 +++++++++--- cmd/entire/cli/setup_import_test.go | 80 +++++++++++++++++-- 4 files changed, 175 insertions(+), 29 deletions(-) diff --git a/cmd/entire/cli/integration_test/enable_import_test.go b/cmd/entire/cli/integration_test/enable_import_test.go index 3e61851e18..74da34e9bc 100644 --- a/cmd/entire/cli/integration_test/enable_import_test.go +++ b/cmd/entire/cli/integration_test/enable_import_test.go @@ -28,7 +28,7 @@ func freshRepoEnv(t *testing.T) *TestEnv { return env } -func TestEnableOffersImport_FirstRunAutoImportsWithYes(t *testing.T) { +func TestEnableOffersImport_FirstRunImportsWithImportHistory(t *testing.T) { t.Parallel() env := freshRepoEnv(t) @@ -37,17 +37,57 @@ func TestEnableOffersImport_FirstRunAutoImportsWithYes(t *testing.T) { filepath.Join(env.ClaudeProjectDir, "sess1.jsonl"), []byte(claudeImportFixture), 0o644)) - // --yes ("accept all defaults") auto-imports the selected agent's - // discoverable history on first-time enable, even non-interactively. - out := env.RunCLI("enable", "--agent", agentClaudeCode, "--yes", "--telemetry=false") + // --import-history is the explicit, non-interactive opt-in to importing + // the selected agent's discoverable history on first-time enable. + out := env.RunCLI("enable", "--agent", agentClaudeCode, "--import-history", "--telemetry=false") require.Contains(t, out, "Ready.", "enable should complete; got: %s", out) - require.Contains(t, out, "Imported 2 turn(s)", "first-time enable --yes should import discovered history; got: %s", out) + require.Contains(t, out, "Imported 2 turn(s)", "--import-history should import discovered history; got: %s", out) // The imported turns are real checkpoints on the v1 metadata branch. require.Contains(t, env.RunCLI("checkpoint", "list"), "[imported]", "imported checkpoints should be listed") } +// TestEnableOffersImport_YesDoesNotImport pins the decision that --yes does +// not carry a history import with it. --yes means "accept all defaults", and +// the interactive default is to import nothing, so an unattended enable must +// not ingest a month of local transcripts on its own. +func TestEnableOffersImport_YesDoesNotImport(t *testing.T) { + t.Parallel() + env := freshRepoEnv(t) + + require.NoError(t, os.WriteFile( + filepath.Join(env.ClaudeProjectDir, "sess1.jsonl"), + []byte(claudeImportFixture), 0o644)) + + out := env.RunCLI("enable", "--agent", agentClaudeCode, "--yes", "--telemetry=false") + require.Contains(t, out, "Ready.", "enable should complete; got: %s", out) + require.NotContains(t, out, "Imported", "--yes must not import agent history; got: %s", out) + require.Contains(t, out, "entire import", "should point at the manual import command; got: %s", out) + + require.NotContains(t, env.RunCLI("checkpoint", "list"), "[imported]", + "no checkpoints should be imported under --yes alone") +} + +// TestEnableImportHistory_OnConfiguredRepoIsReported proves the flag is not +// silently dropped when the repo is already set up (the offer is first-run +// only), so a user cannot come away believing history was imported. +func TestEnableImportHistory_OnConfiguredRepoIsReported(t *testing.T) { + t.Parallel() + env := freshRepoEnv(t) + + require.NoError(t, os.WriteFile( + filepath.Join(env.ClaudeProjectDir, "sess1.jsonl"), + []byte(claudeImportFixture), 0o644)) + + env.RunCLI("enable", "--agent", agentClaudeCode, "--telemetry=false") + + out := env.RunCLI("enable", "--import-history") + require.Contains(t, out, "import-history", "re-run should report the flag does not apply; got: %s", out) + require.Contains(t, out, "entire import", "should point at the manual import command; got: %s", out) + require.NotContains(t, out, "Imported", "a non-first-run enable must not import; got: %s", out) +} + func TestEnableOffersImport_NonInteractiveWithoutYesHints(t *testing.T) { t.Parallel() env := freshRepoEnv(t) @@ -86,8 +126,8 @@ func TestEnableOffersImport_NotOfferedOnReEnable(t *testing.T) { filepath.Join(env.ClaudeProjectDir, "sess1.jsonl"), []byte(claudeImportFixture), 0o644)) - // First enable imports (--yes accepts the import). - first := env.RunCLI("enable", "--agent", agentClaudeCode, "--yes", "--telemetry=false") + // First enable imports (--import-history opts in). + first := env.RunCLI("enable", "--agent", agentClaudeCode, "--import-history", "--telemetry=false") require.Contains(t, first, "Imported 2 turn(s)", "first enable should import; got: %s", first) // Re-enable must not re-offer or re-import, even though history is still present. diff --git a/cmd/entire/cli/setup.go b/cmd/entire/cli/setup.go index b3475048d8..64d5cecd75 100644 --- a/cmd/entire/cli/setup.go +++ b/cmd/entire/cli/setup.go @@ -49,6 +49,7 @@ const ( flagLocalDev = "local-dev" flagSearchSkill = "search-skill" flagAgentHelpSkill = "agent-help-skill" + flagImportHistory = "import-history" checkpointProviderGitHub = "github" ) @@ -78,8 +79,13 @@ type EnableOptions struct { // presentation of the final state (commit, push, done). SuppressDoneMessage bool Yes bool - SearchSkill bool - AgentHelpSkill bool + // ImportHistory opts into importing the selected agents' pre-existing + // session history during first-time setup. Deliberately NOT implied by + // Yes: ingesting a month of local transcripts is not a setup default (see + // maybeOfferSessionImport). + ImportHistory bool + SearchSkill bool + AgentHelpSkill bool } // applyStrategyOptions sets strategy_options on settings from CLI flags. @@ -949,7 +955,8 @@ for you and (optionally) create a matching GitHub repository via the gh CLI.`, cmd.Flags().BoolVar(&opts.AbsoluteGitHookPath, flagAbsoluteGitHookPath, false, "Embed full binary path in git hooks (for GUI git clients that don't source shell profiles)") cmd.Flags().BoolVar(&opts.SearchSkill, flagSearchSkill, false, "Install the optional Entire search skill for selected agent(s)") cmd.Flags().BoolVar(&opts.AgentHelpSkill, flagAgentHelpSkill, false, "Install the stable Entire agent-help skill (points agents at `entire agent-help`) for selected agent(s)") - cmd.Flags().BoolVarP(&opts.Yes, "yes", "y", false, "Accept all defaults without prompting (in a non-repo directory: init git, create private GitHub repo, commit, and push; then enable all agents and accept telemetry)") + cmd.Flags().BoolVarP(&opts.Yes, "yes", "y", false, "Accept all defaults without prompting (in a non-repo directory: init git, create private GitHub repo, commit, and push; then enable all agents and accept telemetry). Does not import existing agent history — see --"+flagImportHistory) + cmd.Flags().BoolVar(&opts.ImportHistory, flagImportHistory, false, importHistoryFlagUsage()) addInsecureHTTPAuthFlag(cmd, &insecureHTTPAuth) // Bootstrap flags for non-git-repo folders. @@ -1109,6 +1116,11 @@ To completely remove Entire integrations from this repository, use --uninstall: // flag or reports current status. func runEnableOnConfiguredRepo(ctx context.Context, cmd *cobra.Command, opts EnableOptions) error { w := cmd.OutOrStdout() + // This path is by definition not a first run, so it never reaches the + // import offer. Say so rather than dropping the flag silently. + if opts.ImportHistory { + noteImportHistoryNotApplicable(w) + } usedSetupFlow := enableUsesSetupFlow(cmd, "") if usedSetupFlow { if hasStrategyFlags(cmd) { diff --git a/cmd/entire/cli/setup_import.go b/cmd/entire/cli/setup_import.go index 46d0856f5a..5b4e4cee9f 100644 --- a/cmd/entire/cli/setup_import.go +++ b/cmd/entire/cli/setup_import.go @@ -34,20 +34,45 @@ var ( sessionImportRun = runSelectedImports ) +// importHistoryFlagUsage is the --import-history help text. It lives here, next +// to the behavior it describes, so the advertised lookback cannot drift from +// agentimport's. +func importHistoryFlagUsage() string { + return fmt.Sprintf( + "During first-time setup, import the selected agents' existing session history (last %d days) without prompting", + agentimport.LookbackDays) +} + +// noteImportHistoryNotApplicable tells a user who asked for a history import +// that this run cannot do one. The offer is first-time-setup only, so on an +// already-configured repo the standalone command is the way in; silently +// dropping the flag would leave them believing history had been imported. +func noteImportHistoryNotApplicable(w io.Writer) { + fmt.Fprintf(w, "Note: --%s applies to first-time setup. Run 'entire import ' to import existing history.\n", + flagImportHistory) +} + // maybeOfferSessionImport offers, on first-time enable only, to import // pre-existing agent history for the just-selected agents. Granularity is // agent-level: choosing an agent imports all its discoverable sessions (30-day // lookback, matching `entire import`). It is best-effort — discovery or import // failures are logged and reported to the user but never fail enable. // -// Import only happens on an explicit choice: an interactive run presents a -// multi-select (nothing pre-checked) and imports what the user selects; `--yes` -// ("accept all defaults") auto-imports all eligible agents. A non-interactive -// run without `--yes` (a script, a piped shell, or an agent with no TTY) makes -// no choice, so it imports nothing and just points at `entire import` — silently -// importing history there would be surprising. +// Import only happens on an explicit choice. An interactive run presents a +// multi-select with nothing pre-checked, so its default is to import nothing; +// `--import-history` is the non-interactive way to say yes. +// +// `--yes` deliberately does NOT import. It means "accept all defaults", and the +// interactive default here is to skip — so implying an import from it would +// make the unattended path do the opposite of the attended one. Ingesting a +// month of local transcripts (which a later push publishes) is its own +// decision, not a setup default, so `--yes` takes the same path as any other +// run that makes no choice: import nothing, and point at `entire import`. func maybeOfferSessionImport(ctx context.Context, w io.Writer, agents []agent.Agent, opts EnableOptions, firstRun bool) { if !firstRun { + if opts.ImportHistory { + noteImportHistoryNotApplicable(w) + } return } @@ -64,11 +89,16 @@ func maybeOfferSessionImport(ctx context.Context, w io.Writer, agents []agent.Ag } selected := eligible - if !opts.Yes { - if !interactive.CanPromptInteractively() { - // Non-interactive without --yes: don't silently import. Leave a - // pointer so scripted/agent enables can still import on demand. - logging.Info(ctx, "session import offer skipped: non-interactive without --yes", "eligible", len(eligible)) + if !opts.ImportHistory { + // No explicit opt-in, and no way (or no intent) to ask: don't silently + // import. Leave a pointer so scripted/agent/--yes enables can still + // import on demand. The hint names the standalone command rather than + // the flag: by the time this prints, setup has written its settings, so + // a re-run of enable is no longer a first run and the flag would not + // apply. + if opts.Yes || !interactive.CanPromptInteractively() { + logging.Info(ctx, "session import offer skipped: no explicit opt-in", + "eligible", len(eligible), "yes", opts.Yes) fmt.Fprintf(w, "Found importable history for %s. Run 'entire import ' to import it.\n", pluralAgents(len(eligible))) return } diff --git a/cmd/entire/cli/setup_import_test.go b/cmd/entire/cli/setup_import_test.go index 2e59569b12..e689169bc6 100644 --- a/cmd/entire/cli/setup_import_test.go +++ b/cmd/entire/cli/setup_import_test.go @@ -98,7 +98,7 @@ func TestMaybeOfferSessionImport_FirstRunGate(t *testing.T) { } } -func TestMaybeOfferSessionImport_NonInteractiveAutoImportsAll(t *testing.T) { +func TestMaybeOfferSessionImport_ImportHistoryImportsAllWithoutPrompting(t *testing.T) { // Not parallel: overrides seams and chdirs into a temp repo. dir := t.TempDir() testutil.InitRepo(t, dir) @@ -119,17 +119,81 @@ func TestMaybeOfferSessionImport_NonInteractiveAutoImportsAll(t *testing.T) { func(_ context.Context, _ io.Writer, _ string, sel []eligibleImport) { ran = sel }, ) - // opts.Yes forces the non-interactive path even if a TTY is present. - maybeOfferSessionImport(context.Background(), io.Discard, nil, EnableOptions{Yes: true}, true) + // --import-history is the explicit, non-interactive opt-in: it imports + // every eligible agent and never asks. + maybeOfferSessionImport(context.Background(), io.Discard, nil, EnableOptions{ImportHistory: true}, true) if promptCalled { - t.Error("prompt shown under --yes; non-interactive enable must not prompt") + t.Error("prompt shown under --import-history; the flag is already the answer") } if len(ran) != len(eligible) { t.Fatalf("imported %d agents, want all %d", len(ran), len(eligible)) } } -func TestMaybeOfferSessionImport_NonInteractiveWithoutYesSkips(t *testing.T) { +// TestMaybeOfferSessionImport_YesDoesNotImport pins the decision that --yes +// ("accept all defaults") does not import agent history. The interactive +// default is to import nothing — the multi-select pre-checks nothing — so +// implying an import from --yes would make the unattended path do the opposite +// of the attended one, and ingesting a month of local transcripts is its own +// decision rather than a setup default. +func TestMaybeOfferSessionImport_YesDoesNotImport(t *testing.T) { + // Not parallel: overrides seams, chdirs into a temp repo, and sets env. + dir := t.TempDir() + testutil.InitRepo(t, dir) + t.Chdir(dir) + // A real TTY is available: --yes must still not import, and must not fall + // through to the prompt either. + t.Setenv("ENTIRE_TEST_TTY", "1") + + promptCalled := false + var ran []eligibleImport + withImportSeams(t, + func(context.Context, []agent.Agent, string) []eligibleImport { + return []eligibleImport{{displayName: testAgentClaude, sessionCount: 3}} + }, + func(context.Context, io.Writer, []eligibleImport) ([]eligibleImport, error) { + promptCalled = true + return nil, nil + }, + func(_ context.Context, _ io.Writer, _ string, sel []eligibleImport) { ran = sel }, + ) + + var buf bytes.Buffer + maybeOfferSessionImport(context.Background(), &buf, nil, EnableOptions{Yes: true}, true) + if promptCalled { + t.Error("prompt shown under --yes; it means accept defaults without prompting") + } + if len(ran) != 0 { + t.Errorf("--yes auto-imported %d agent(s); history import needs its own opt-in", len(ran)) + } + if got := buf.String(); !strings.Contains(got, "entire import") { + t.Errorf("expected a pointer to 'entire import', got %q", got) + } +} + +// TestMaybeOfferSessionImport_ImportHistoryOnNonFirstRunIsReported proves the +// flag is not silently dropped when it cannot apply — the user would otherwise +// believe their history had been imported. +func TestMaybeOfferSessionImport_ImportHistoryOnNonFirstRunIsReported(t *testing.T) { + // Not parallel: overrides package seams. + called := false + withImportSeams(t, + func(context.Context, []agent.Agent, string) []eligibleImport { + called = true + return nil + }, nil, nil) + + var buf bytes.Buffer + maybeOfferSessionImport(context.Background(), &buf, nil, EnableOptions{ImportHistory: true}, false /* firstRun */) + if called { + t.Error("discovery ran on a non-first-run enable") + } + if got := buf.String(); !strings.Contains(got, flagImportHistory) || !strings.Contains(got, "entire import") { + t.Errorf("expected a note that --%s does not apply plus a pointer to the command, got %q", flagImportHistory, got) + } +} + +func TestMaybeOfferSessionImport_NonInteractiveWithoutOptInSkips(t *testing.T) { // Not parallel: overrides seams and chdirs into a temp repo. dir := t.TempDir() testutil.InitRepo(t, dir) @@ -150,15 +214,15 @@ func TestMaybeOfferSessionImport_NonInteractiveWithoutYesSkips(t *testing.T) { func(_ context.Context, _ io.Writer, _ string, sel []eligibleImport) { ran = sel }, ) - // No --yes and no TTY: neither prompt nor auto-import; just hint at the - // manual command. + // No opt-in flag and no TTY: neither prompt nor auto-import; just hint at + // the manual command. var buf bytes.Buffer maybeOfferSessionImport(context.Background(), &buf, nil, EnableOptions{}, true) if promptCalled { t.Error("prompt shown in a non-interactive context") } if len(ran) != 0 { - t.Errorf("auto-imported %d agent(s) without --yes in a non-interactive context; expected skip", len(ran)) + t.Errorf("auto-imported %d agent(s) without --import-history in a non-interactive context; expected skip", len(ran)) } if got := buf.String(); !strings.Contains(got, "entire import") { t.Errorf("expected a pointer to 'entire import', got %q", got) From 637b802e8ec866e466f2a2efd7f39881a301cbcb Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Fri, 7 Aug 2026 17:32:41 +0200 Subject: [PATCH 6/9] perf(checkpoint): memoize the git common dir instead of forking git per write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveGitCommonDir spawns `git rev-parse --git-common-dir`, and its two callers run on per-turn paths: the push-discovery queue resolves it on every checkpoint ref write, and the ephemeral store on every shadow-branch write. Measured on an M4 Max, the fork is ~9.5ms of a ~12.9ms checkpoint write — 73% of the cost, and 4.5x once memoized. A 30-day import of this repo's own Claude history is ~1,100 turns, so `entire enable --import-history` was paying ~10s of pure fork/exec. The value cannot change for a worktree's lifetime, so cache it by worktree root. session.getGitCommonDir already caches this same value process-wide for its own callers; this is its unmemoized twin. Only successes are cached — a canceled context fails the subprocess instantly, and caching that would poison every later caller. Co-Authored-By: Claude Opus 5 (1M context) Entire-Checkpoint: 01KZEDNTE2ME0C627CDHGEEMP1 --- cmd/entire/cli/checkpoint/git_common_dir.go | 34 +++++++- .../cli/checkpoint/git_common_dir_test.go | 86 +++++++++++++++++++ 2 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 cmd/entire/cli/checkpoint/git_common_dir_test.go diff --git a/cmd/entire/cli/checkpoint/git_common_dir.go b/cmd/entire/cli/checkpoint/git_common_dir.go index de9716452e..9aa2a698c4 100644 --- a/cmd/entire/cli/checkpoint/git_common_dir.go +++ b/cmd/entire/cli/checkpoint/git_common_dir.go @@ -7,10 +7,35 @@ import ( "os/exec" "path/filepath" "strings" + "sync" "github.com/go-git/go-git/v6" ) +// commonDirCache memoizes resolved git common dirs by worktree root. The value +// cannot change for a worktree's lifetime, and resolving it costs a git +// subprocess (~10ms) — which the push-discovery queue pays on EVERY checkpoint +// ref write and the ephemeral store pays on every shadow-branch write, i.e. +// once per agent turn on the hook path and once per turn imported by `entire +// import`. Mirrors session.getGitCommonDir, which already caches this same +// value process-wide for its own callers. +// +// Only successes are cached: a failure may be transient (a canceled context +// fails the subprocess instantly), and caching it would poison every later +// caller in the process. +var ( + commonDirMu sync.RWMutex + commonDirCache = map[string]string{} // worktree root -> resolved common dir +) + +// cachedCommonDir returns the memoized common dir for a worktree root. +func cachedCommonDir(root string) (string, bool) { + commonDirMu.RLock() + defer commonDirMu.RUnlock() + dir, ok := commonDirCache[root] + return dir, ok +} + func resolveGitCommonDir(ctx context.Context, repo *git.Repository) (string, error) { worktree, err := repo.Worktree() if err != nil { @@ -20,6 +45,9 @@ func resolveGitCommonDir(ctx context.Context, repo *git.Repository) (string, err if root == "" { return "", errors.New("resolve worktree root for git common dir") } + if cached, ok := cachedCommonDir(root); ok { + return cached, nil + } cmd := exec.CommandContext(ctx, "git", "-C", root, "rev-parse", "--git-common-dir") // Use Output (not CombinedOutput) so stderr never pollutes the resolved @@ -42,5 +70,9 @@ func resolveGitCommonDir(ctx context.Context, repo *git.Repository) (string, err if !filepath.IsAbs(commonDir) { commonDir = filepath.Join(root, commonDir) } - return filepath.Clean(commonDir), nil + commonDir = filepath.Clean(commonDir) + commonDirMu.Lock() + commonDirCache[root] = commonDir + commonDirMu.Unlock() + return commonDir, nil } diff --git a/cmd/entire/cli/checkpoint/git_common_dir_test.go b/cmd/entire/cli/checkpoint/git_common_dir_test.go new file mode 100644 index 0000000000..6a501c631b --- /dev/null +++ b/cmd/entire/cli/checkpoint/git_common_dir_test.go @@ -0,0 +1,86 @@ +package checkpoint + +import ( + "context" + "testing" + + git "github.com/go-git/go-git/v6" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/entireio/cli/cmd/entire/cli/testutil" +) + +// openTestRepo initializes a repo with one commit and opens it. +func openTestRepo(t *testing.T) *git.Repository { + t.Helper() + dir := t.TempDir() + testutil.InitRepo(t, dir) + testutil.WriteFile(t, dir, "README.md", "# test") + testutil.GitAdd(t, dir, "README.md") + testutil.GitCommit(t, dir, "init") + repo, err := git.PlainOpen(dir) + require.NoError(t, err) + return repo +} + +// TestResolveGitCommonDir_CachesPerWorktree proves the memo is keyed by +// worktree root rather than shared globally: two repos must never resolve to +// each other's common dir. A single cached value (or a cwd-keyed one) would +// hand the second repo the first's .git, silently writing one repo's +// push-discovery queue into another's. +func TestResolveGitCommonDir_CachesPerWorktree(t *testing.T) { + t.Parallel() + ctx := context.Background() + + repoA, repoB := openTestRepo(t), openTestRepo(t) + + firstA, err := resolveGitCommonDir(ctx, repoA) + require.NoError(t, err) + firstB, err := resolveGitCommonDir(ctx, repoB) + require.NoError(t, err) + assert.NotEqual(t, firstA, firstB, "distinct repos must resolve to distinct common dirs") + + // Second resolution is served from the memo and must agree with the first. + secondA, err := resolveGitCommonDir(ctx, repoA) + require.NoError(t, err) + assert.Equal(t, firstA, secondA) + secondB, err := resolveGitCommonDir(ctx, repoB) + require.NoError(t, err) + assert.Equal(t, firstB, secondB) +} + +// TestResolveGitCommonDir_CachedValueServesCanceledContext proves a cache hit +// needs no subprocess: once resolved, a canceled context still yields the +// common dir. This is what lets enqueueForPush record a ref during shutdown +// without paying (or failing) a `git rev-parse`. +func TestResolveGitCommonDir_CachedValueServesCanceledContext(t *testing.T) { + t.Parallel() + repo := openTestRepo(t) + + want, err := resolveGitCommonDir(context.Background(), repo) + require.NoError(t, err) + + canceled, cancel := context.WithCancel(context.Background()) + cancel() + got, err := resolveGitCommonDir(canceled, repo) + require.NoError(t, err, "a cached common dir must not need the subprocess") + assert.Equal(t, want, got) +} + +// TestResolveGitCommonDir_FailureIsNotCached proves a failed resolution leaves +// the memo empty, so a transient failure (e.g. a canceled context) cannot +// poison every later caller in the process. +func TestResolveGitCommonDir_FailureIsNotCached(t *testing.T) { + t.Parallel() + repo := openTestRepo(t) + + canceled, cancel := context.WithCancel(context.Background()) + cancel() + _, err := resolveGitCommonDir(canceled, repo) + require.Error(t, err, "a canceled context should fail the resolving subprocess") + + got, err := resolveGitCommonDir(context.Background(), repo) + require.NoError(t, err, "the earlier failure must not be cached") + assert.NotEmpty(t, got) +} From 1049be5af250c63ca64aae5d4d505573c00c087a Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Fri, 7 Aug 2026 17:32:43 +0200 Subject: [PATCH 7/9] refactor: fold review cleanups into the import cancellation work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-review pass over the branch: - Correct two comments that claimed the git-branch store rejects writes on a canceled context. It does not: GitStore.writeSession has no guard and nothing on its path observes ctx, so both stores share that hole — git-refs is simply the one the reported bug ran on. Asserting a parity that does not exist would mislead the next reader. - Reuse the existing fixedDiscoverImporter seam (via an optional onDiscover hook) instead of adding two near-identical stub importers. - Extract writeClaudeHistory in the enable-import integration tests; the fixture-write stanza had been copy-pasted five times. - Flatten maybeOfferSessionImport's nested opt-in check into one guard clause, so `selected` is declared next to its only reassignment. - Trim test doc comments that restated their production counterpart's rationale verbatim — four rationales that could drift apart. Each now states what it pins and points at the code holding the why. - importHistoryFlagUsage is a package var, not a no-arg function. Co-Authored-By: Claude Opus 5 (1M context) Entire-Checkpoint: 01KZEDNWK23F6XBDQVCG5RKH7K --- cmd/entire/cli/agentimport/agentimport.go | 13 +++--- .../cli/agentimport/agentimport_test.go | 17 +++----- cmd/entire/cli/checkpoint/refs_store_test.go | 18 +++----- .../integration_test/enable_import_test.go | 43 ++++++++----------- cmd/entire/cli/setup.go | 2 +- cmd/entire/cli/setup_import.go | 32 +++++++------- cmd/entire/cli/setup_import_test.go | 39 +++++------------ 7 files changed, 64 insertions(+), 100 deletions(-) diff --git a/cmd/entire/cli/agentimport/agentimport.go b/cmd/entire/cli/agentimport/agentimport.go index 9ee7f374b0..4f44d0f6ce 100644 --- a/cmd/entire/cli/agentimport/agentimport.go +++ b/cmd/entire/cli/agentimport/agentimport.go @@ -215,10 +215,7 @@ func Run(ctx context.Context, repo *git.Repository, imp Importer, opts Options) authorName, authorEmail := cp.GetGitAuthorFromRepo(repo) for sessionIndex, sf := range files { - // Ctrl-C must stop the import. Nothing else on this path observes - // cancellation — go-git object/ref writes and CreateCommit all ignore - // ctx — so without these checks an interrupted import runs to - // completion, silently minting checkpoints the user asked to stop. + // Stop before reading and splitting the next transcript. if err := ctx.Err(); err != nil { return res, err //nolint:wrapcheck // propagate context cancellation } @@ -240,8 +237,12 @@ func Run(ctx context.Context, repo *git.Repository, imp Importer, opts Options) var red redact.RedactedBytes redacted := false for turnIndex, turn := range turns { - // Per-turn, not just per-session: one session can carry hundreds of - // turns, and each turn is a checkpoint write. + // Ctrl-C must stop the import, and per turn rather than per session: + // one session can carry hundreds of turns, each a checkpoint write. + // Nothing below this observes cancellation — go-git object/ref + // writes and CreateCommit all ignore ctx, and neither git store + // guards its create path — so without this an interrupted import + // runs to completion, minting checkpoints the user asked to stop. if err := ctx.Err(); err != nil { return res, err //nolint:wrapcheck // propagate context cancellation } diff --git a/cmd/entire/cli/agentimport/agentimport_test.go b/cmd/entire/cli/agentimport/agentimport_test.go index 4dc046be0d..2bcdb7f616 100644 --- a/cmd/entire/cli/agentimport/agentimport_test.go +++ b/cmd/entire/cli/agentimport/agentimport_test.go @@ -622,10 +622,9 @@ func TestRun_CodexImportSanitizesAndKeepsOffsetsAligned(t *testing.T) { } // TestRun_StopsOnContextCancellation proves Ctrl-C mid-import halts Run's own -// loops, independently of whether the configured checkpoint store happens to -// reject a canceled write. DryRun writes nothing, so the loop checks are the -// only thing that can stop this run: without them Run walks every remaining -// session and turn after the cancel. +// loops, independently of whether the configured checkpoint store rejects a +// canceled write. DryRun writes nothing, so the loop checks are the only thing +// that can stop this run. func TestRun_StopsOnContextCancellation(t *testing.T) { t.Parallel() repo, repoDir := initRepoWithCommit(t) @@ -662,12 +661,10 @@ func TestRun_StopsOnContextCancellation(t *testing.T) { } // TestRun_CancellationStopsRefsBackedImport is the end-to-end regression for -// the reported bug: `entire enable` defaults a first-time repo to the git-refs -// checkpoint backend and then offers to import agent history. Unlike the -// git-branch store, the git-refs store did not reject writes on a canceled -// context, and nothing else on that path observes ctx (go-git object/ref -// writes and CreateCommit all ignore it) — so Ctrl-C left the import running -// to completion, minting checkpoints the user had just asked to stop. +// the reported bug, in the configuration it was reported on: `entire enable` +// defaults a first-time repo to the git-refs checkpoint backend and then +// offers to import agent history. See gitRefsStore.writeSession for why no +// layer below this one stopped the run. func TestRun_CancellationStopsRefsBackedImport(t *testing.T) { // Not parallel: sets the checkpoint backend via the environment. t.Setenv("ENTIRE_CHECKPOINTS_PRIMARY", "git-refs") diff --git a/cmd/entire/cli/checkpoint/refs_store_test.go b/cmd/entire/cli/checkpoint/refs_store_test.go index bc341852cc..3ff3376b75 100644 --- a/cmd/entire/cli/checkpoint/refs_store_test.go +++ b/cmd/entire/cli/checkpoint/refs_store_test.go @@ -738,11 +738,8 @@ func TestGitRefsStore_BackfillUnknownCheckpointNotFound(t *testing.T) { assert.Nil(t, summary) } -// TestGitRefsStore_WriteRefusesCanceledContext proves a canceled context stops -// the refs store from minting checkpoints, matching the git-branch store and -// this store's own backfill writers. Without it a bulk writer that ignores -// cancellation — `entire import`, which `entire enable` runs on a first-time -// repo — kept creating checkpoints after Ctrl-C. +// TestGitRefsStore_WriteRefusesCanceledContext pins that a canceled context +// stops the refs store from minting checkpoints — see writeSession for why. func TestGitRefsStore_WriteRefusesCanceledContext(t *testing.T) { t.Parallel() store := newRefsStore(t) @@ -766,14 +763,9 @@ func TestGitRefsStore_WriteRefusesCanceledContext(t *testing.T) { "a write refused for cancellation must not leave a checkpoint ref behind") } -// TestGitRefsStore_EnqueuesForPushDuringShutdown proves the push-queue record -// still lands when the context is already canceled. By the time setRef runs the -// ref is on disk (go-git ref writes don't observe ctx) and the queue is the only -// push-discovery mechanism there is, so honoring the cancellation here would -// strand the checkpoint locally forever: writers are idempotent, so no later run -// re-enqueues it. Resolving the queue shells out to git, which is what made this -// the one step on the write path that failed under a canceled ctx — the source -// of the "resolve push queue failed; ref not enqueued" warning flood. +// TestGitRefsStore_EnqueuesForPushDuringShutdown pins that a ref written during +// shutdown is still queued for push — see enqueueForPush for why dropping the +// record would strand the checkpoint locally forever. func TestGitRefsStore_EnqueuesForPushDuringShutdown(t *testing.T) { t.Parallel() store := newRefsStore(t) diff --git a/cmd/entire/cli/integration_test/enable_import_test.go b/cmd/entire/cli/integration_test/enable_import_test.go index 74da34e9bc..c71db99f36 100644 --- a/cmd/entire/cli/integration_test/enable_import_test.go +++ b/cmd/entire/cli/integration_test/enable_import_test.go @@ -16,6 +16,15 @@ const claudeImportFixture = `{"type":"user","uuid":"u1","timestamp":"2026-06-20T {"type":"user","uuid":"u2","timestamp":"2026-06-20T00:01:00Z","message":{"role":"user","content":"second"}} ` +// writeClaudeHistory drops a discoverable two-turn Claude transcript for the +// env's repo, so a first-time enable has something to offer to import. +func writeClaudeHistory(t *testing.T, env *TestEnv) { + t.Helper() + require.NoError(t, os.WriteFile( + filepath.Join(env.ClaudeProjectDir, "sess1.jsonl"), + []byte(claudeImportFixture), 0o644)) +} + // freshRepoEnv builds a repo with an initial commit but WITHOUT Entire enabled, // so `entire enable` runs its real first-time flow. func freshRepoEnv(t *testing.T) *TestEnv { @@ -32,10 +41,7 @@ func TestEnableOffersImport_FirstRunImportsWithImportHistory(t *testing.T) { t.Parallel() env := freshRepoEnv(t) - // Pre-existing Claude history for this repo. - require.NoError(t, os.WriteFile( - filepath.Join(env.ClaudeProjectDir, "sess1.jsonl"), - []byte(claudeImportFixture), 0o644)) + writeClaudeHistory(t, env) // --import-history is the explicit, non-interactive opt-in to importing // the selected agent's discoverable history on first-time enable. @@ -56,9 +62,7 @@ func TestEnableOffersImport_YesDoesNotImport(t *testing.T) { t.Parallel() env := freshRepoEnv(t) - require.NoError(t, os.WriteFile( - filepath.Join(env.ClaudeProjectDir, "sess1.jsonl"), - []byte(claudeImportFixture), 0o644)) + writeClaudeHistory(t, env) out := env.RunCLI("enable", "--agent", agentClaudeCode, "--yes", "--telemetry=false") require.Contains(t, out, "Ready.", "enable should complete; got: %s", out) @@ -76,9 +80,7 @@ func TestEnableImportHistory_OnConfiguredRepoIsReported(t *testing.T) { t.Parallel() env := freshRepoEnv(t) - require.NoError(t, os.WriteFile( - filepath.Join(env.ClaudeProjectDir, "sess1.jsonl"), - []byte(claudeImportFixture), 0o644)) + writeClaudeHistory(t, env) env.RunCLI("enable", "--agent", agentClaudeCode, "--telemetry=false") @@ -92,10 +94,7 @@ func TestEnableOffersImport_NonInteractiveWithoutYesHints(t *testing.T) { t.Parallel() env := freshRepoEnv(t) - // Pre-existing Claude history for this repo. - require.NoError(t, os.WriteFile( - filepath.Join(env.ClaudeProjectDir, "sess1.jsonl"), - []byte(claudeImportFixture), 0o644)) + writeClaudeHistory(t, env) // A non-interactive (no-TTY) enable without --yes must NOT silently import; // it points at the manual command instead. @@ -122,9 +121,7 @@ func TestEnableOffersImport_NoHistoryIsSilent(t *testing.T) { func TestEnableOffersImport_NotOfferedOnReEnable(t *testing.T) { t.Parallel() env := freshRepoEnv(t) - require.NoError(t, os.WriteFile( - filepath.Join(env.ClaudeProjectDir, "sess1.jsonl"), - []byte(claudeImportFixture), 0o644)) + writeClaudeHistory(t, env) // First enable imports (--import-history opts in). first := env.RunCLI("enable", "--agent", agentClaudeCode, "--import-history", "--telemetry=false") @@ -137,14 +134,10 @@ func TestEnableOffersImport_NotOfferedOnReEnable(t *testing.T) { "re-enable must not run import at all; got: %s", second) } -// TestEnable_RoutesLoggingToLogFile proves `entire enable` initializes file -// logging like every other command. Without it the package logger stays nil -// and every logging.* call under setup — agent detection, hook install, the -// session import, the checkpoint layer's push and remote warnings — falls back -// to slog.Default(), which prints them straight onto the user's terminal -// mid-flow and writes nothing to .entire/logs/. That is how a Ctrl-C'd -// enable-time import came to flood a user's terminal with "resolve push queue -// failed; ref not enqueued" lines. +// TestEnable_RoutesLoggingToLogFile pins that `entire enable` initializes file +// logging like every other command — see the logging.Init call in enable's +// RunE for why its absence put operational warnings on the user's terminal +// instead of in the log. func TestEnable_RoutesLoggingToLogFile(t *testing.T) { t.Parallel() env := freshRepoEnv(t) diff --git a/cmd/entire/cli/setup.go b/cmd/entire/cli/setup.go index 64d5cecd75..463cab7400 100644 --- a/cmd/entire/cli/setup.go +++ b/cmd/entire/cli/setup.go @@ -956,7 +956,7 @@ for you and (optionally) create a matching GitHub repository via the gh CLI.`, cmd.Flags().BoolVar(&opts.SearchSkill, flagSearchSkill, false, "Install the optional Entire search skill for selected agent(s)") cmd.Flags().BoolVar(&opts.AgentHelpSkill, flagAgentHelpSkill, false, "Install the stable Entire agent-help skill (points agents at `entire agent-help`) for selected agent(s)") cmd.Flags().BoolVarP(&opts.Yes, "yes", "y", false, "Accept all defaults without prompting (in a non-repo directory: init git, create private GitHub repo, commit, and push; then enable all agents and accept telemetry). Does not import existing agent history — see --"+flagImportHistory) - cmd.Flags().BoolVar(&opts.ImportHistory, flagImportHistory, false, importHistoryFlagUsage()) + cmd.Flags().BoolVar(&opts.ImportHistory, flagImportHistory, false, importHistoryFlagUsage) addInsecureHTTPAuthFlag(cmd, &insecureHTTPAuth) // Bootstrap flags for non-git-repo folders. diff --git a/cmd/entire/cli/setup_import.go b/cmd/entire/cli/setup_import.go index 5b4e4cee9f..95754422d2 100644 --- a/cmd/entire/cli/setup_import.go +++ b/cmd/entire/cli/setup_import.go @@ -37,11 +37,9 @@ var ( // importHistoryFlagUsage is the --import-history help text. It lives here, next // to the behavior it describes, so the advertised lookback cannot drift from // agentimport's. -func importHistoryFlagUsage() string { - return fmt.Sprintf( - "During first-time setup, import the selected agents' existing session history (last %d days) without prompting", - agentimport.LookbackDays) -} +var importHistoryFlagUsage = fmt.Sprintf( + "During first-time setup, import the selected agents' existing session history (last %d days) without prompting", + agentimport.LookbackDays) // noteImportHistoryNotApplicable tells a user who asked for a history import // that this run cannot do one. The offer is first-time-setup only, so on an @@ -88,20 +86,20 @@ func maybeOfferSessionImport(ctx context.Context, w io.Writer, agents []agent.Ag return } + // No explicit opt-in, and no way (or no intent) to ask: don't silently + // import. Leave a pointer so scripted/agent/--yes enables can still import + // on demand. The hint names the standalone command rather than the flag: + // by the time this prints, setup has written its settings, so a re-run of + // enable is no longer a first run and the flag would not apply. + if !opts.ImportHistory && (opts.Yes || !interactive.CanPromptInteractively()) { + logging.Info(ctx, "session import offer skipped: no explicit opt-in", + "eligible", len(eligible), "yes", opts.Yes) + fmt.Fprintf(w, "Found importable history for %s. Run 'entire import ' to import it.\n", pluralAgents(len(eligible))) + return + } + selected := eligible if !opts.ImportHistory { - // No explicit opt-in, and no way (or no intent) to ask: don't silently - // import. Leave a pointer so scripted/agent/--yes enables can still - // import on demand. The hint names the standalone command rather than - // the flag: by the time this prints, setup has written its settings, so - // a re-run of enable is no longer a first run and the flag would not - // apply. - if opts.Yes || !interactive.CanPromptInteractively() { - logging.Info(ctx, "session import offer skipped: no explicit opt-in", - "eligible", len(eligible), "yes", opts.Yes) - fmt.Fprintf(w, "Found importable history for %s. Run 'entire import ' to import it.\n", pluralAgents(len(eligible))) - return - } selected, err = sessionImportPrompt(ctx, w, eligible) if err != nil { // Best-effort: a prompt/UI failure must never fail enable. Log, diff --git a/cmd/entire/cli/setup_import_test.go b/cmd/entire/cli/setup_import_test.go index e689169bc6..337635e734 100644 --- a/cmd/entire/cli/setup_import_test.go +++ b/cmd/entire/cli/setup_import_test.go @@ -358,9 +358,16 @@ type fixedDiscoverImporter struct { agentimport.Importer sessions []agentimport.SessionFile + // onDiscover, when set, runs as Discover is reached — the seam for + // observing that the import loop got this far, or for injecting a + // cancellation at that moment. + onDiscover func() } func (f fixedDiscoverImporter) Discover(string, string, time.Time, []string) ([]agentimport.SessionFile, error) { + if f.onDiscover != nil { + f.onDiscover() + } return f.sessions, nil } @@ -511,32 +518,6 @@ func TestRunSelectedImports_NonTTYProgressLines_Reimport(t *testing.T) { } } -// cancelOnDiscoverImporter cancels the run's context from Discover, standing in -// for a Ctrl-C landing while the first agent's history is being imported. -type cancelOnDiscoverImporter struct { - agentimport.Importer - - sessions []agentimport.SessionFile - cancel context.CancelFunc -} - -func (c cancelOnDiscoverImporter) Discover(string, string, time.Time, []string) ([]agentimport.SessionFile, error) { - c.cancel() - return c.sessions, nil -} - -// recordDiscoverImporter records whether the import loop reached it at all. -type recordDiscoverImporter struct { - agentimport.Importer - - reached *bool -} - -func (r recordDiscoverImporter) Discover(string, string, time.Time, []string) ([]agentimport.SessionFile, error) { - *r.reached = true - return nil, nil -} - // TestRunSelectedImports_InterruptedStopsBeforeNextAgent proves Ctrl-C during // one agent's import ends the whole offer instead of moving straight on to the // next agent's history — the last thing a user who just interrupted wants — @@ -574,8 +555,10 @@ func TestRunSelectedImports_InterruptedStopsBeforeNextAgent(t *testing.T) { var secondReached bool var buf bytes.Buffer runSelectedImports(ctx, &buf, dir, []eligibleImport{ - {imp: cancelOnDiscoverImporter{Importer: claudeImp, sessions: sessions, cancel: cancel}, displayName: testAgentClaude}, - {imp: recordDiscoverImporter{Importer: claudeImp, reached: &secondReached}, displayName: "Cursor"}, + // The first agent's import is interrupted as it starts; the second must + // never be reached. + {imp: fixedDiscoverImporter{Importer: claudeImp, sessions: sessions, onDiscover: cancel}, displayName: testAgentClaude}, + {imp: fixedDiscoverImporter{Importer: claudeImp, onDiscover: func() { secondReached = true }}, displayName: "Cursor"}, }) out := buf.String() From a3c5b126e334fd5173ee2c3e2b5cae72fca98dc7 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Fri, 7 Aug 2026 18:57:42 +0200 Subject: [PATCH 8/9] fix(enable): don't create .entire/ on a rejected invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit logging.Init creates .entire/logs/, and it ran before the checks that can still reject the invocation — so `entire enable --local --project` or `--agent ` left an untracked .entire/ behind in a repo that had never been touched, and that does not yet carry Entire's gitignore entry to cover it. Reported by Bugbot on #1925 and reproduced. Resolve --agent before logging starts and initialize after every check that can bail, so a rejected enable leaves the repo exactly as it found it. The placement is now load-bearing in both directions: after the git-repo check so it cannot write outside a repository, and after validation so it cannot seed one enable refuses to configure. Co-Authored-By: Claude Opus 5 (1M context) Entire-Checkpoint: 01KZEJHGESMDDPVVPS7TE4PAX0 --- .../integration_test/enable_import_test.go | 23 +++++++++++ cmd/entire/cli/setup.go | 41 ++++++++++++------- 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/cmd/entire/cli/integration_test/enable_import_test.go b/cmd/entire/cli/integration_test/enable_import_test.go index c71db99f36..42a415f44a 100644 --- a/cmd/entire/cli/integration_test/enable_import_test.go +++ b/cmd/entire/cli/integration_test/enable_import_test.go @@ -151,3 +151,26 @@ func TestEnable_RoutesLoggingToLogFile(t *testing.T) { require.Contains(t, out, "Ready.", "enable should complete; got: %s", out) require.FileExists(t, logPath, "enable should route its logging to .entire/logs; got output: %s", out) } + +// TestEnable_RejectedInvocationLeavesRepoUntouched pins that enable's logging +// init cannot seed a repo that enable then refuses to configure. Init creates +// .entire/logs/, so running it before the invocation is known-valid left an +// untracked .entire/ behind on every rejected `entire enable` — in a repo that +// does not yet carry Entire's gitignore entry to cover it. +func TestEnable_RejectedInvocationLeavesRepoUntouched(t *testing.T) { + t.Parallel() + env := freshRepoEnv(t) + + for _, tc := range []struct { + name string + args []string + }{ + {"mutually exclusive scopes", []string{"enable", "--local", "--project"}}, + {"unknown agent", []string{"enable", "--agent", "definitely-not-an-agent"}}, + } { + out, err := env.RunCLIWithError(tc.args...) + require.Error(t, err, "%s: expected enable to be rejected; got: %s", tc.name, out) + require.NoDirExists(t, filepath.Join(env.RepoDir, ".entire"), + "%s: a rejected enable must not create .entire/; got: %s", tc.name, out) + } +} diff --git a/cmd/entire/cli/setup.go b/cmd/entire/cli/setup.go index 463cab7400..f72ea2caca 100644 --- a/cmd/entire/cli/setup.go +++ b/cmd/entire/cli/setup.go @@ -889,19 +889,6 @@ for you and (optionally) create a matching GitHub repository via the gh CLI.`, }() } - // Route setup's logging to .entire/logs/ like every other command. - // Without Init the package logger stays nil and every logging.* - // call under setup — agent detection, hook install, session import, - // the checkpoint layer's push/remote warnings — falls back to - // slog.Default(), which prints them straight onto the user's - // terminal mid-flow (and writes nothing to the log file). Placed - // after the git-repo check above so it cannot create .entire/logs/ - // outside a repository. - logging.SetLogLevelGetter(GetLogLevel) - if err := logging.Init(ctx, ""); err == nil { - defer logging.Close() - } - if err := validateSetupFlags(opts.UseLocalSettings, opts.UseProjectSettings); err != nil { return err } @@ -917,16 +904,42 @@ for you and (optionally) create a matching GitHub repository via the gh CLI.`, return NewSilentError(errors.New("missing agent name")) } + // Resolve --agent before logging starts, so a bad agent name is + // rejected without touching the repo (see below). + var selectedAgent agent.Agent if agentName != "" { ag, err := agent.Get(types.AgentName(agentName)) if err != nil { printWrongAgentError(cmd.ErrOrStderr(), agentName) return NewSilentError(errors.New("wrong agent name")) } + selectedAgent = ag + } + + // Route setup's logging to .entire/logs/ like every other command. + // Without Init the package logger stays nil and every logging.* + // call under setup — agent detection, hook install, session import, + // the checkpoint layer's push/remote warnings — falls back to + // slog.Default(), which prints them straight onto the user's + // terminal mid-flow (and writes nothing to the log file). + // + // Placement is load-bearing in both directions: after the git-repo + // check above so it cannot create .entire/logs/ outside a + // repository, and after every check that can still reject this + // invocation, because Init CREATES .entire/logs/ — a rejected + // enable must leave a previously untouched repo untouched rather + // than seeding it with an untracked directory Entire's gitignore + // entry does not exist yet to cover. + logging.SetLogLevelGetter(GetLogLevel) + if err := logging.Init(ctx, ""); err == nil { + defer logging.Close() + } + + if selectedAgent != nil { // --agent is a targeted operation: set up this specific agent without // affecting other agents. Unlike the interactive path, it does not // uninstall hooks for other previously-enabled agents. - return setupAgentHooksNonInteractive(ctx, cmd.OutOrStdout(), ag, opts) + return setupAgentHooksNonInteractive(ctx, cmd.OutOrStdout(), selectedAgent, opts) } // Any setup-mutating flags should behave like `configure` on repos that From 513009238d6b35457efb98ddbc7d65b19dfd271a Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Fri, 7 Aug 2026 18:57:46 +0200 Subject: [PATCH 9/9] fix(checkpoint): guard the git-branch store's create path too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitStore.writeSession had no cancellation guard, even though its own backfill siblings all check ctx.Err() and nothing on its path observes cancellation either (ensureSessionsBranch, tree building, CreateCommit). The git-refs guard alone left every branch-backed repo without a store-level brake on Ctrl-C — and git-branch is still the runtime fallback when no checkpoints config is present, and stays selectable via `--checkpoint-backend branch`. Flagged by the trail finding on #1925 and by this branch's own review; the PR had it listed as a follow-up. It is three lines and closes the inconsistency, so do it here rather than leave the fix half-applied. Co-Authored-By: Claude Opus 5 (1M context) Entire-Checkpoint: 01KZEJHKE3BSTVGGXPT18HE5P9 --- cmd/entire/cli/agentimport/agentimport.go | 7 ++-- cmd/entire/cli/checkpoint/persistent.go | 9 +++++ .../checkpoint/persistent_imported_test.go | 35 +++++++++++++++++++ cmd/entire/cli/checkpoint/refs_store.go | 10 +++--- 4 files changed, 52 insertions(+), 9 deletions(-) diff --git a/cmd/entire/cli/agentimport/agentimport.go b/cmd/entire/cli/agentimport/agentimport.go index 4f44d0f6ce..33374745f3 100644 --- a/cmd/entire/cli/agentimport/agentimport.go +++ b/cmd/entire/cli/agentimport/agentimport.go @@ -239,10 +239,9 @@ func Run(ctx context.Context, repo *git.Repository, imp Importer, opts Options) for turnIndex, turn := range turns { // Ctrl-C must stop the import, and per turn rather than per session: // one session can carry hundreds of turns, each a checkpoint write. - // Nothing below this observes cancellation — go-git object/ref - // writes and CreateCommit all ignore ctx, and neither git store - // guards its create path — so without this an interrupted import - // runs to completion, minting checkpoints the user asked to stop. + // The store guards its create path too, but only this stops the + // per-turn work leading up to it (reading, splitting, redacting), + // and it is the only brake at all under DryRun, which never writes. if err := ctx.Err(); err != nil { return res, err //nolint:wrapcheck // propagate context cancellation } diff --git a/cmd/entire/cli/checkpoint/persistent.go b/cmd/entire/cli/checkpoint/persistent.go index 0a4aeb9404..4402945d6c 100644 --- a/cmd/entire/cli/checkpoint/persistent.go +++ b/cmd/entire/cli/checkpoint/persistent.go @@ -59,6 +59,15 @@ var chunkTranscript = agent.ChunkTranscript // - For incremental checkpoints: checkpoints/NNN-.json // - For final checkpoints: checkpoint.json and agent-.jsonl func (s *GitStore) writeSession(ctx context.Context, opts WriteOptions) error { + // Parity with this store's backfill writers and with the git-refs store: a + // canceled ctx means stop doing work, and creating a checkpoint is the most + // expensive write there is. Nothing below here observes cancellation + // (ensureSessionsBranch, tree building and CreateCommit all ignore ctx), so + // without this a bulk writer that ignores cancellation keeps minting + // checkpoints after Ctrl-C. + if err := ctx.Err(); err != nil { + return err //nolint:wrapcheck // Propagating context cancellation + } // Validate identifiers to prevent path traversal and malformed data if opts.CheckpointID.IsEmpty() { return errors.New("invalid checkpoint options: checkpoint ID is required") diff --git a/cmd/entire/cli/checkpoint/persistent_imported_test.go b/cmd/entire/cli/checkpoint/persistent_imported_test.go index 8f4cff582c..a44b818fbd 100644 --- a/cmd/entire/cli/checkpoint/persistent_imported_test.go +++ b/cmd/entire/cli/checkpoint/persistent_imported_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "testing" "github.com/go-git/go-git/v6" @@ -198,3 +199,37 @@ func TestImportedCheckpoint_CommitSHASurvivesSummaryRewrite(t *testing.T) { t.Fatalf("expected both sessions in the rewritten summary, got %d", len(summary.Sessions)) } } + +// TestGitStore_WriteRefusesCanceledContext pins that the git-branch store, like +// the git-refs store and like its own backfill writers, stops creating +// checkpoints once the context is canceled. It is still the runtime fallback +// when no checkpoints config is present and remains selectable via +// `--checkpoint-backend branch`, so a guard applied only to git-refs would +// leave every branch-backed repo without a store-level brake on Ctrl-C. +func TestGitStore_WriteRefusesCanceledContext(t *testing.T) { + t.Parallel() + store, transcript := newImportedTestStore(t) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := store.Write(ctx, Session{ + CheckpointID: id.MustCheckpointID("a1b2c3d4e5f6"), + SessionID: "sess-1", + Strategy: "manual-commit", + Transcript: transcript, + AuthorName: "Test Author", + AuthorEmail: "test@example.com", + }) + if !errors.Is(err, context.Canceled) { + t.Fatalf("Write() with a canceled context = %v, want context.Canceled", err) + } + + infos, listErr := store.List(context.Background()) + if listErr != nil { + t.Fatalf("List: %v", listErr) + } + if len(infos) != 0 { + t.Fatalf("a write refused for cancellation left %d checkpoint(s) behind", len(infos)) + } +} diff --git a/cmd/entire/cli/checkpoint/refs_store.go b/cmd/entire/cli/checkpoint/refs_store.go index 0e181bb659..7b00762c36 100644 --- a/cmd/entire/cli/checkpoint/refs_store.go +++ b/cmd/entire/cli/checkpoint/refs_store.go @@ -237,11 +237,11 @@ func (s *gitRefsStore) enqueueForPush(ctx context.Context, refName plumbing.Refe } func (s *gitRefsStore) writeSession(ctx context.Context, opts WriteOptions) error { - // Parity with the backfill writers above: a canceled ctx means stop doing - // work, and creating a checkpoint is the most expensive write there is - // (tree building plus a commit). Without this a bulk writer that ignores - // cancellation — `entire import` was one — keeps minting checkpoints after - // Ctrl-C. + // Parity with the backfill writers above and with GitStore.writeSession: a + // canceled ctx means stop doing work, and creating a checkpoint is the most + // expensive write there is (tree building plus a commit). Without this a + // bulk writer that ignores cancellation — `entire import` was one — keeps + // minting checkpoints after Ctrl-C. if err := ctx.Err(); err != nil { return err //nolint:wrapcheck // Propagating context cancellation }