fix(import): gate progress spinner ANSI on ShouldStyle for legacy consoles

The import progress spinner (#1848) drew its animated frames with
cursor-control escapes (\r\033[K) whenever the writer was a terminal,
gating only on IsTerminalWriter and bypassing interactive.ShouldStyle —
the repo's single gate for writer-scoped ANSI. On a console that can't
render ANSI (TERM=cygwin renders the ESC byte as a literal "←", GH #1267)
or when NO_COLOR is set, every frame emitted "←[K" garbage. The per-frame
\033[K this PR added widened a previously stop-line-only escape to the
whole animation, so legacy Windows consoles regressed from clean bare-\r
frames to visible garbage.

- startUpdatableSpinner: fall back to the completion-line-only (ANSI-free)
  path unless w both is a terminal and ShouldStyle(w) — fixes it at the
  source for every spinner caller, not just import.
- newImportProgressReporter: route non-styleable terminals to the plain
  per-session line path (same as non-TTY/ACCESSIBLE), so NO_COLOR/cygwin
  users still get per-session progress instead of a lone final line.
- setup_import_test.go: correct a comment naming a test
  (TestNewImportProgressReporter_TTYAdvancesOnSkip) that does not exist.

The terminal+!ShouldStyle branch is only observable against a real TTY, so
it carries no unit test — matching the PR's existing PTY-bound coverage
limits; the shouldStyle decision itself is already unit-tested.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01KYM4JTVVADFMT258B3020DV9

Author: gtrrz-victor

cmd/entire/cli/import_progress.go

@@ -14,11 +14,14 @@ import (
 // tracks "Importing <agentName> sessions... (session i/N · turn j/M)";
 // callers must call the returned stop exactly once when the run finishes —
 // on both the success and error paths — so no spinner frame is left
-// dangling to corrupt whatever prints next. Otherwise (non-TTY, piped, or
-// ACCESSIBLE mode) it prints one plain, ANSI-free line per session from
-// SessionStart, and stop is a no-op.
+// dangling to corrupt whatever prints next. Otherwise — non-TTY, piped,
+// ACCESSIBLE mode, or a terminal that can't render ANSI (NO_COLOR,
+// TERM=cygwin; see interactive.ShouldStyle) — it prints one plain, ANSI-free
+// line per session from SessionStart, and stop is a no-op. The ShouldStyle
+// gate matches startUpdatableSpinner's own gate, so the spinner branch here
+// is taken only when the animation it drives can actually be rendered.
 func newImportProgressReporter(w io.Writer, agentName string) (progress *agentimport.Progress, stop func(success bool)) {
-	if !interactive.IsTerminalWriter(w) || IsAccessibleMode() {
+	if !interactive.IsTerminalWriter(w) || IsAccessibleMode() || !interactive.ShouldStyle(w) {
 		return &agentimport.Progress{
 			SessionStart: func(sessionIndex, sessionTotal int, _, _ string, turnCount int) {
 				fmt.Fprintf(w, "Importing %s session %d/%d (%d %s)...\n",

cmd/entire/cli/progress.go

@@ -33,11 +33,18 @@ func startSpinner(w io.Writer, msg string) func(success bool) {
 
 // startUpdatableSpinner is startSpinner's variant for an operation whose
 // status text changes while it runs (e.g. "session 2/5 · turn 3/10"). update
-// replaces the message the next frame draws — or, on a non-terminal writer,
+// replaces the message the next frame draws — or, on the non-animated path,
 // the message stop's completion line uses. update is safe to call at any
 // point, including before the spinner's first frame draws and after stop
 // returns. stop behaves exactly like startSpinner's, rendering whichever
 // message update last set (or msg, if update was never called).
+//
+// The live animation is emitted only when w both is a terminal and can render
+// ANSI (interactive.ShouldStyle) — the frames use cursor-control escapes
+// (\r\033[K), which a legacy console that can't handle ANSI (e.g.
+// TERM=cygwin) renders as literal "←[K" garbage, and which NO_COLOR asks us
+// to suppress. When styling is off we fall back to the completion-line-only
+// path, so no escape byte is ever written to such a writer.
 func startUpdatableSpinner(w io.Writer, msg string) (update func(string), stop func(success bool)) {
 	var mu sync.Mutex
 	current := msg
@@ -52,7 +59,7 @@ func startUpdatableSpinner(w io.Writer, msg string) (update func(string), stop f
 		return current
 	}
 
-	if !interactive.IsTerminalWriter(w) {
+	if !interactive.IsTerminalWriter(w) || !interactive.ShouldStyle(w) {
 		return setMsg, func(success bool) {
 			if success {
 				fmt.Fprintf(w, "✓ %s\n", getMsg())

cmd/entire/cli/setup_import_test.go

@@ -380,9 +380,13 @@ func TestRunSelectedImports_NonTTYProgressLines(t *testing.T) {
 // 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).
+// the non-TTY side of the bug the P2 Codex's pre-push review caught. The TTY
+// side (a fully-skipped session must still sweep the spinner to turn M/M) is
+// covered at the agentimport layer by
+// TestRun_ReimportFiresTurnSkippedNotTurnWritten; the cli wiring it depends on
+// — newImportProgressReporter routing TurnSkipped and TurnWritten through one
+// shared advance path — has no direct test because the spinner branch only
+// runs against a real terminal.
 func TestRunSelectedImports_NonTTYProgressLines_Reimport(t *testing.T) {
 	// Not parallel: chdirs into a temp repo and performs real checkpoint writes.
 	dir := t.TempDir()