fix(enable): advance import progress on skipped and dry-run turns

The progress counter only advanced on TurnWritten, so a fully
idempotent re-import or a --dry-run pass sat at 'turn 0/M' and then
rendered that stale count into the completion line. Add
Progress.TurnSkipped, fired for every turn Run processes without
writing (already imported, or DryRun), and drive the UI counter from
both callbacks: exactly one of TurnWritten/TurnSkipped now fires per
turn, so the counter always sweeps to M/M truthfully.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Entire-Checkpoint: 01KY95QCD5YDCST7YNYKW063FA

Author: ecgang

cmd/entire/cli/agentimport/agentimport.go

@@ -131,6 +131,10 @@ type Result struct {
 // (a progress bar, log lines, a TUI) is entirely up to the caller. Every
 // field is optional, and a nil *Progress (the default) is a no-op — Run's
 // behavior is byte-identical whether or not one is supplied.
+//
+// Invariant: for every turn Run processes, exactly one of TurnWritten or
+// TurnSkipped fires — so summing both callbacks' calls across one session
+// always equals that session's turnCount (as reported by SessionStart).
 type Progress struct {
 	// SessionStart fires once per session, after its transcript has been
 	// split into turns and before any of them are written. sessionIndex is
@@ -139,9 +143,14 @@ type Progress struct {
 	SessionStart func(sessionIndex, sessionTotal int, agentName, sessionID string, turnCount int)
 	// TurnWritten fires once per turn Run actually writes to the checkpoint
 	// store — never for a turn skipped as already-imported, nor under
-	// DryRun. turnIndex is 0-based against turnCount, matching the
-	// turnCount reported by this turn's SessionStart call.
+	// DryRun (see TurnSkipped for those). turnIndex is 0-based against
+	// turnCount, matching the turnCount reported by this turn's
+	// SessionStart call.
 	TurnWritten func(sessionIndex, turnIndex, turnCount int)
+	// TurnSkipped fires once per turn Run processes without writing: a turn
+	// already imported (idempotent re-run) or, under DryRun, every turn
+	// (dry runs never write). Index semantics match TurnWritten exactly.
+	TurnSkipped func(sessionIndex, turnIndex, turnCount int)
 }
 
 func (p *Progress) sessionStart(sessionIndex, sessionTotal int, agentName, sessionID string, turnCount int) {
@@ -158,6 +167,13 @@ func (p *Progress) turnWritten(sessionIndex, turnIndex, turnCount int) {
 	p.TurnWritten(sessionIndex, turnIndex, turnCount)
 }
 
+func (p *Progress) turnSkipped(sessionIndex, turnIndex, turnCount int) {
+	if p == nil || p.TurnSkipped == nil {
+		return
+	}
+	p.TurnSkipped(sessionIndex, turnIndex, turnCount)
+}
+
 // DeriveCheckpointID produces a stable 12-hex checkpoint ID for an imported
 // turn. Re-importing the same (sessionID, turnUUID) yields the same ID, which
 // is how import stays idempotent.
@@ -214,10 +230,12 @@ func Run(ctx context.Context, repo *git.Repository, imp Importer, opts Options)
 			cid := DeriveCheckpointID(sf.SessionID, turn.UUID)
 			if existing[cid.String()] {
 				res.TurnsSkipped++
+				opts.Progress.turnSkipped(sessionIndex, turnIndex, len(turns))
 				continue
 			}
 			if opts.DryRun {
 				res.TurnsImported++ // counts what would import
+				opts.Progress.turnSkipped(sessionIndex, turnIndex, len(turns))
 				continue
 			}
 			if !redacted {

cmd/entire/cli/agentimport/progress_test.go

@@ -110,3 +110,160 @@ func TestRun_NilProgressDoesNotPanic(t *testing.T) {
 		t.Fatalf("want 4 imported, got %+v", resNil)
 	}
 }
+
+// progressRecorder collects Progress callback invocations for assertion.
+type progressRecorder struct {
+	written []progressTurnEvent
+	skipped []progressTurnEvent
+}
+
+func (r *progressRecorder) progress() *Progress {
+	return &Progress{
+		TurnWritten: func(sessionIndex, turnIndex, turnCount int) {
+			r.written = append(r.written, progressTurnEvent{sessionIndex, turnIndex, turnCount})
+		},
+		TurnSkipped: func(sessionIndex, turnIndex, turnCount int) {
+			r.skipped = append(r.skipped, progressTurnEvent{sessionIndex, turnIndex, turnCount})
+		},
+	}
+}
+
+// TestRun_ReimportFiresTurnSkippedNotTurnWritten proves a re-import over an
+// already-imported corpus (the idempotent-skip path) reports every turn via
+// TurnSkipped, in order, and never via TurnWritten — the P2 Codex's pre-push
+// review caught: without this, a TTY progress reporter driven only by
+// TurnWritten freezes at "turn 0/M" on a fully-skipped session.
+func TestRun_ReimportFiresTurnSkippedNotTurnWritten(t *testing.T) {
+	t.Parallel()
+	repo, repoDir := initRepoWithCommit(t)
+	claudeDir := t.TempDir()
+	writeFixtureSession(t, claudeDir, "sess1.jsonl")
+	writeFixtureSession(t, claudeDir, "sess2.jsonl")
+	opts := Options{RepoRoot: repoDir, OverridePath: claudeDir, Now: time.Date(2026, 6, 25, 0, 0, 0, 0, time.UTC)}
+
+	// First run: no progress, just to populate the store so the second run
+	// hits the idempotent-skip path for every turn.
+	if _, err := Run(context.Background(), repo, claudeImporter{}, opts); err != nil {
+		t.Fatal(err)
+	}
+
+	rec := &progressRecorder{}
+	opts.Progress = rec.progress()
+	res, err := Run(context.Background(), repo, claudeImporter{}, opts)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if res.TurnsImported != 0 || res.TurnsSkipped != 4 {
+		t.Fatalf("want 0 imported / 4 skipped on re-import, got %+v", res)
+	}
+
+	if len(rec.written) != 0 {
+		t.Errorf("TurnWritten fired %d times on a fully-skipped re-import, want 0: %+v", len(rec.written), rec.written)
+	}
+	wantSkipped := []progressTurnEvent{
+		{sessionIndex: 0, turnIndex: 0, turnCount: 2},
+		{sessionIndex: 0, turnIndex: 1, turnCount: 2},
+		{sessionIndex: 1, turnIndex: 0, turnCount: 2},
+		{sessionIndex: 1, turnIndex: 1, turnCount: 2},
+	}
+	if !reflect.DeepEqual(rec.skipped, wantSkipped) {
+		t.Fatalf("TurnSkipped events = %+v, want %+v", rec.skipped, wantSkipped)
+	}
+}
+
+// TestRun_DryRunFiresTurnSkippedForEveryTurn proves DryRun — which never
+// writes — reports every turn via TurnSkipped and never via TurnWritten.
+func TestRun_DryRunFiresTurnSkippedForEveryTurn(t *testing.T) {
+	t.Parallel()
+	repo, repoDir := initRepoWithCommit(t)
+	claudeDir := t.TempDir()
+	writeFixtureSession(t, claudeDir, "sess1.jsonl")
+	writeFixtureSession(t, claudeDir, "sess2.jsonl")
+
+	rec := &progressRecorder{}
+	res, err := Run(context.Background(), repo, claudeImporter{}, Options{
+		RepoRoot: repoDir, OverridePath: claudeDir, DryRun: true,
+		Now:      time.Date(2026, 6, 25, 0, 0, 0, 0, time.UTC),
+		Progress: rec.progress(),
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if res.TurnsImported != 4 {
+		// DryRun's Result bookkeeping is unchanged: TurnsImported still means
+		// "would import" (see Run's DryRun branch). TurnSkipped is a separate,
+		// additive signal that nothing was actually written.
+		t.Fatalf("want 4 (would-import), got %+v", res)
+	}
+
+	if len(rec.written) != 0 {
+		t.Errorf("TurnWritten fired %d times under DryRun, want 0: %+v", len(rec.written), rec.written)
+	}
+	if len(rec.skipped) != 4 {
+		t.Fatalf("TurnSkipped fired %d times under DryRun, want 4: %+v", len(rec.skipped), rec.skipped)
+	}
+}
+
+// TestRun_MixedSkipAndWriteSatisfiesInvariant proves the documented
+// invariant — for every turn, exactly one of TurnWritten/TurnSkipped fires,
+// so per-session written+skipped == turnCount — holds when a run mixes
+// already-imported sessions with a brand-new one in a single call.
+func TestRun_MixedSkipAndWriteSatisfiesInvariant(t *testing.T) {
+	t.Parallel()
+	repo, repoDir := initRepoWithCommit(t)
+	claudeDir := t.TempDir()
+	writeFixtureSession(t, claudeDir, "sess1.jsonl")
+	writeFixtureSession(t, claudeDir, "sess2.jsonl")
+	opts := Options{RepoRoot: repoDir, OverridePath: claudeDir, Now: time.Date(2026, 6, 25, 0, 0, 0, 0, time.UTC)}
+
+	// Import sess1 and sess2 first, so a second run finds them already
+	// imported while a newly-added sess3 is still fresh.
+	if _, err := Run(context.Background(), repo, claudeImporter{}, opts); err != nil {
+		t.Fatal(err)
+	}
+	writeFixtureSession(t, claudeDir, "sess3.jsonl")
+
+	rec := &progressRecorder{}
+	opts.Progress = rec.progress()
+	res, err := Run(context.Background(), repo, claudeImporter{}, opts)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if res.TurnsImported != 2 || res.TurnsSkipped != 4 {
+		t.Fatalf("want 2 imported (sess3) / 4 skipped (sess1+sess2), got %+v", res)
+	}
+
+	counts := map[int]struct{ written, skipped int }{}
+	for _, ev := range rec.written {
+		c := counts[ev.sessionIndex]
+		c.written++
+		counts[ev.sessionIndex] = c
+	}
+	for _, ev := range rec.skipped {
+		c := counts[ev.sessionIndex]
+		c.skipped++
+		counts[ev.sessionIndex] = c
+	}
+
+	// Discovery is sorted by path, so sess1=0, sess2=1, sess3=2 (each has 2
+	// turns per writeFixtureSession).
+	wantBySession := map[int]struct{ written, skipped int }{
+		0: {written: 0, skipped: 2}, // sess1: already imported
+		1: {written: 0, skipped: 2}, // sess2: already imported
+		2: {written: 2, skipped: 0}, // sess3: brand new
+	}
+	if len(counts) != len(wantBySession) {
+		t.Fatalf("saw events for %d sessions, want %d: %+v", len(counts), len(wantBySession), counts)
+	}
+	for sessionIndex, want := range wantBySession {
+		got := counts[sessionIndex]
+		if got != want {
+			t.Errorf("session %d: written=%d skipped=%d, want written=%d skipped=%d",
+				sessionIndex, got.written, got.skipped, want.written, want.skipped)
+		}
+		if got.written+got.skipped != 2 {
+			t.Errorf("session %d: written+skipped = %d, want turnCount 2 (invariant violated)",
+				sessionIndex, got.written+got.skipped)
+		}
+	}
+}

cmd/entire/cli/import_progress.go

@@ -33,14 +33,21 @@ func newImportProgressReporter(w io.Writer, agentName string) (progress *agentim
 		update(fmt.Sprintf("Importing %s sessions... (session %d/%d · turn %d/%d)",
 			agentName, curSession, curSessionTotal, turnsDone, curTurnTotal))
 	}
+	advance := func(_, turnIndex, _ int) {
+		render(turnIndex + 1)
+	}
 	progress = &agentimport.Progress{
 		SessionStart: func(sessionIndex, sessionTotal int, _, _ string, turnCount int) {
 			curSession, curSessionTotal, curTurnTotal = sessionIndex+1, sessionTotal, turnCount
 			render(0)
 		},
-		TurnWritten: func(_, turnIndex, _ int) {
-			render(turnIndex + 1)
-		},
+		// TurnWritten and TurnSkipped share the same advance path: the
+		// counter must sweep to turnCount/turnCount regardless of *why* a
+		// turn didn't need writing (already imported, or DryRun), otherwise
+		// a fully-skipped or dry-run session's completion line would freeze
+		// at "turn 0/M".
+		TurnWritten: advance,
+		TurnSkipped: advance,
 	}
 	return progress, spinnerStop
 }

cmd/entire/cli/setup_import_test.go

@@ -375,3 +375,70 @@ func TestRunSelectedImports_NonTTYProgressLines(t *testing.T) {
 		t.Errorf("final summary line missing or changed; want %q in:\n%s", want, out)
 	}
 }
+
+// TestRunSelectedImports_NonTTYProgressLines_Reimport proves a second,
+// idempotent pass over an already-imported corpus (every turn hits
+// agentimport's TurnSkipped path, not TurnWritten) still prints one plain
+// progress line per session and reports the correct "0 imported" summary —
+// the non-TTY side of the P2 Codex's pre-push review caught (the TTY side is
+// covered by agentimport's TestRun_ReimportFiresTurnSkippedNotTurnWritten
+// and this package's TestNewImportProgressReporter_TTYAdvancesOnSkip).
+func TestRunSelectedImports_NonTTYProgressLines_Reimport(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 := context.Background()
+
+	sessionsDir := t.TempDir()
+	writeImportProgressFixtureSession(t, sessionsDir, "sess1.jsonl")
+	writeImportProgressFixtureSession(t, sessionsDir, "sess2.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)
+	}
+	agentName := string(claudeImp.AgentType())
+	selected := []eligibleImport{{
+		imp:         fixedDiscoverImporter{Importer: claudeImp, sessions: sessions},
+		displayName: agentName,
+	}}
+
+	// First pass actually imports; discard its output.
+	runSelectedImports(ctx, io.Discard, dir, selected)
+
+	// Second pass: every turn is already imported, so agentimport.Run's loop
+	// only ever calls TurnSkipped for it — this is the scenario that used to
+	// leave a TTY reporter frozen at "turn 0/M".
+	var buf bytes.Buffer
+	runSelectedImports(ctx, &buf, dir, selected)
+	out := buf.String()
+
+	if strings.ContainsRune(out, '\x1b') {
+		t.Fatalf("re-import output contains an ESC byte on a non-TTY writer: %q", out)
+	}
+	wantLines := []string{
+		fmt.Sprintf("Importing %s session 1/2 (2 turns)...", agentName),
+		fmt.Sprintf("Importing %s session 2/2 (2 turns)...", agentName),
+	}
+	for _, line := range wantLines {
+		if !strings.Contains(out, line) {
+			t.Errorf("missing progress line %q in re-import output:\n%s", line, out)
+		}
+	}
+	if want := "Imported 0 turn(s) from 2 session(s) (4 already imported).\n"; !strings.Contains(out, want) {
+		t.Errorf("re-import summary line missing or wrong; want %q in:\n%s", want, out)
+	}
+}