Skip to content

fix(enable): stop the enable-time history import on Ctrl-C, and take it out of --yes - #1925

Open
Soph wants to merge 9 commits into
mainfrom
soph/enable-import-cancellation
Open

fix(enable): stop the enable-time history import on Ctrl-C, and take it out of --yes#1925
Soph wants to merge 9 commits into
mainfrom
soph/enable-import-cancellation

Conversation

@Soph

@Soph Soph commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

https://entire.io/gh/entireio/cli/trails/991

A user ran entire enable in a repo that never had Entire, hit Ctrl-C, and got a flood of resolve push queue failed; ref not enqueued warnings on their terminal.

First-run enable defaults to the git-refs backend and then offers to import pre-existing agent history. Ctrl-C didn't stop that import: neither agentimport.Run nor gitRefsStore.writeSession observed cancellation, and nothing below them does either (go-git writes and CreateCommit ignore ctx). The only step that did observe it was the push-queue resolution — it shells out to git — so every post-cancel write logged a warning and left its ref out of the queue. That's worse than noisy: the queue is the sole push-discovery mechanism and the writers are idempotent, so those checkpoints were stranded locally forever.

The warnings reached the terminal at all because entire enable never called logging.Init, so every logging.* call under setup fell back to slog.Default().

Changes

  • git-refs store rejects writes on a canceled context; the push-queue record is written with cancellation stripped, since the ref is already on disk by then
  • agentimport.Run checks ctx per session and per turn
  • Interrupted imports report as resumable (Import interrupted after N turn(s)) and stop instead of moving on to the next agent's history
  • entire enable routes its logging to .entire/logs
  • --import-history replaces --yes as the way to opt into importing history
  • Perf: memoize the git common dir — the fork was ~9.5ms of a ~12.9ms checkpoint write (73%, 4.5x), on the live hook path too

Breaking

entire enable --yes no longer imports agent history. Pass --import-history, or run entire import <agent>. --yes means "accept defaults", and the interactive default is to import nothing — so it was doing the opposite of the attended path, and in a non-repo dir a single flag both created a GitHub repo and ingested a month of local transcripts destined for it.

Follow-ups (not in this PR)

  • An interrupted enable still exits 0 and prints "Ready.", which opts out of main.go's deliberate re-raise-the-signal contract. entire import gets this right; enable doesn't. Changing an exit code felt like its own decision.
  • GitStore.writeSession (git-branch) has the identical missing guard — verified, pre-existing. A guard in kindRoutingStore.Write would close both.

Each fix has a test verified to fail without it.

🤖 Generated with Claude Code


Note

Medium Risk
Breaking change to entire enable --yes behavior and checkpoint/import paths on cancellation; git-refs write and push-queue semantics affect data sync if mis-handled, though changes are guarded by tests.

Overview
Fixes Ctrl-C during enable-time or bulk import continuing to mint git-refs checkpoints while spamming push-queue warnings, and changes how history import is opted in.

Cancellation: agentimport.Run checks ctx before each session and turn; gitRefsStore.writeSession refuses canceled writes. Push enqueue uses context.WithoutCancel so refs already written still land in the push queue (missing queue entries strand checkpoints locally). Interrupted runs print a resumable message instead of raw context canceled, and enable-time import stops instead of moving to the next agent.

Enable UX (breaking): New --import-history is the explicit first-run opt-in; entire enable --yes no longer imports agent history (aligned with interactive default: import nothing). Non-first-run --import-history is reported as not applicable with a pointer to entire import.

Logging & perf: entire enable now calls logging.Init so setup warnings go to .entire/logs. resolveGitCommonDir is memoized per worktree root (successes only) to avoid repeated git rev-parse on every ref write.

Reviewed by Cursor Bugbot for commit 1049be5. Configure here.

Soph and others added 7 commits August 7, 2026 16:27
… drop push bookkeeping

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) <noreply@anthropic.com>
Entire-Checkpoint: 01KZE9Z15Z7HP0JS2HA7SP00RM
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) <noreply@anthropic.com>
Entire-Checkpoint: 01KZE9ZAM96YPYDZTB9MJAWAJW
…nuing

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) <noreply@anthropic.com>
Entire-Checkpoint: 01KZE9ZTJZM1XT4D0V93S93FZQ
`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) <noreply@anthropic.com>
Entire-Checkpoint: 01KZEA04HEXTJ5GH14VWWAZP1Z
…-import-history

`--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 <agent>`. 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 <agent>`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01KZEARMVQXP70HGB15MXFPAX9
…er write

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) <noreply@anthropic.com>
Entire-Checkpoint: 01KZEDNTE2ME0C627CDHGEEMP1
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) <noreply@anthropic.com>
Entire-Checkpoint: 01KZEDNWK23F6XBDQVCG5RKH7K
Copilot AI lite review requested due to automatic review settings August 7, 2026 15:35
@Soph
Soph requested a review from a team as a code owner August 7, 2026 15:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves robustness and UX of first-time entire enable (and entire import) by ensuring Ctrl-C/cancellation halts expensive history-import work instead of continuing to mint checkpoints, while also making history import an explicit opt-in (--import-history) rather than being implied by --yes. It also routes enable-time logs to .entire/logs and reduces checkpoint-write overhead by memoizing the git common-dir resolution.

Changes:

  • Make history import cancellation-aware: agentimport.Run checks ctx per session/turn, and the git-refs checkpoint store refuses writes on a canceled context while still enqueueing already-written refs for push.
  • Adjust enable-time import UX: introduce --import-history, remove import behavior from --yes, and report when --import-history is used where it can’t apply.
  • Initialize logging during entire enable and memoize git rev-parse --git-common-dir to reduce per-turn overhead.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
cmd/entire/cli/setup.go Adds --import-history, initializes logging during enable, and reports flag non-applicability on configured repos.
cmd/entire/cli/setup_import.go Implements explicit opt-in import behavior, improves cancellation handling (stop-on-Ctrl-C), and provides user-facing resumable messaging.
cmd/entire/cli/setup_import_test.go Updates/extends unit tests for new --import-history semantics and interruption behavior.
cmd/entire/cli/integration_test/enable_import_test.go Updates integration coverage for first-run import opt-in, --yes no-import behavior, and enable logging initialization.
cmd/entire/cli/import_cmd.go Improves Ctrl-C handling to report partial/resumable import instead of a raw cancellation failure.
cmd/entire/cli/checkpoint/refs_store.go Adds cancellation guard for expensive checkpoint writes and ensures push-queue enqueue proceeds even during shutdown.
cmd/entire/cli/checkpoint/refs_store_test.go Adds regression tests for canceled-write refusal and enqueue behavior during cancellation.
cmd/entire/cli/checkpoint/git_common_dir.go Adds process-wide memoization for resolved git common dir keyed by worktree root.
cmd/entire/cli/checkpoint/git_common_dir_test.go Adds tests to validate cache scoping, canceled-context cache hits, and non-caching of failures.
cmd/entire/cli/agentimport/agentimport.go Adds cancellation checks before reading next session and before each turn to stop promptly on Ctrl-C.
cmd/entire/cli/agentimport/agentimport_test.go Adds tests proving cancellation stops both dry-run and refs-backed imports without continuing work.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

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

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

Reviewed by Cursor Bugbot for commit 1049be5. Configure here.

Comment thread cmd/entire/cli/setup.go Outdated
logging.SetLogLevelGetter(GetLogLevel)
if err := logging.Init(ctx, ""); err == nil {
defer logging.Close()
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rejected enable now mutates repositories

Low Severity

logging.Init creates .entire/logs/entire.log before setup flag and agent validation. Invalid enable invocations now modify previously untouched repositories, and first-run failures before setupEntireDirectory can leave the log path without Entire’s gitignore entry.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1049be5. Configure here.

Soph and others added 2 commits August 7, 2026 18:57
logging.Init creates .entire/logs/, and it ran before the checks that
can still reject the invocation — so `entire enable --local --project`
or `--agent <typo>` 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) <noreply@anthropic.com>
Entire-Checkpoint: 01KZEJHGESMDDPVVPS7TE4PAX0
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) <noreply@anthropic.com>
Entire-Checkpoint: 01KZEJHKE3BSTVGGXPT18HE5P9
@Soph

Soph commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator Author

Both addressed in a3c5b12 and 5130092.

Bugbot — rejected enable now mutates repositories. Confirmed and reproduced: entire enable --local --project in an untouched repo left ?? .entire/ behind, because logging.Init creates .entire/logs/ and ran before the checks that reject the invocation. --agent <typo> did the same. Fixed by resolving --agent before logging starts and initializing after every check that can bail — the placement is now load-bearing in both directions (after the git-repo check so it can't write outside a repo, after validation so it can't seed one enable refuses to configure). Regression test covers both rejection paths.

Trail finding — guard applied only to the git-refs backend. Correct, and it matched this branch's own review. GitStore.writeSession had no guard while its backfill siblings all check ctx.Err(), so branch-backed repos had no store-level brake on Ctrl-C — and git-branch is still the runtime fallback with no checkpoints config. I'd listed it as a follow-up; it's three lines, so it's in. Test verified to fail without the guard.

Also corrected two comments that described the branch-store gap as open.

Follow-up still open: an interrupted enable exits 0 and prints "Ready.", bypassing main.go's deliberate re-raise-the-signal contract. Left out as its own decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants