fix: stop kanban spinner for not-started sessions and fix truncated manual backup size (#1622)

apps/backend/internal/system/backups/retention.go

@@ -32,6 +32,10 @@ const (
 	autoPrefix   = "kandev-"
 	manualPrefix = "manual-"
 	dbSuffix     = ".db"
+	// tmpSuffix marks an in-progress VACUUM INTO sidecar. classify() skips
+	// it (no .db suffix) so it is never listed, and runCreate sweeps any
+	// leftovers from a crashed run before writing a new snapshot.
+	tmpSuffix = ".tmp"
 )
 
 // preResetPrefix marks recovery snapshots written by the Factory Reset

apps/backend/internal/system/backups/store.go

@@ -10,6 +10,8 @@ import (
 	"strings"
 	"time"
 
+	"go.uber.org/zap"
+
 	"github.com/kandev/kandev/internal/common/logger"
 	"github.com/kandev/kandev/internal/db"
 	"github.com/kandev/kandev/internal/persistence"
@@ -130,15 +132,34 @@ func (s *Service) runCreate(_ context.Context) (map[string]interface{}, error) {
 	if err := s.ensureBackupsDir(); err != nil {
 		return nil, err
 	}
+	// A .tmp sidecar is normally renamed away on success and removed on the
+	// error paths below, but a crash between SnapshotSQLite and os.Rename
+	// leaves one behind. classify() hides it from List()/Delete(), so it can
+	// never be cleaned up through the UI and would leak disk (up to the size
+	// of the live DB) indefinitely. Sweep any leftovers before writing a new
+	// one so crash debris is reclaimed on the next manual backup.
+	s.sweepStaleTmpFiles()
 	// Nanosecond precision so double-clicks or concurrent /backups POSTs do
 	// not collide on the same filename and silently overwrite one job's
 	// snapshot with another.
 	name := fmt.Sprintf("%s%d%s", manualPrefix, time.Now().UTC().UnixNano(), dbSuffix)
 	path := filepath.Join(s.backupsDir(), name)
-	size, err := persistence.SnapshotSQLite(s.pool.Writer(), path)
+	// VACUUM INTO writes the multi-hundred-MB snapshot incrementally. If we
+	// wrote directly to the final "manual-*.db" name, a concurrent List()
+	// (the UI refetches immediately after the 202) would os.Stat a
+	// half-written file and report a truncated size. Write to a ".tmp"
+	// sidecar first — classify() ignores non-.db suffixes so it is never
+	// listed — then atomically rename it into place at its full size.
+	tmpPath := path + tmpSuffix
+	size, err := persistence.SnapshotSQLite(s.pool.Writer(), tmpPath)
 	if err != nil {
+		_ = os.Remove(tmpPath)
 		return nil, err
 	}
+	if err := os.Rename(tmpPath, path); err != nil {
+		_ = os.Remove(tmpPath)
+		return nil, fmt.Errorf("rename snapshot into place: %w", err)
+	}
 	return map[string]interface{}{
 		"name":       name,
 		"path":       path,
@@ -146,6 +167,39 @@ func (s *Service) runCreate(_ context.Context) (map[string]interface{}, error) {
 	}, nil
 }
 
+// staleTmpAge is how old a ".tmp" sidecar must be before the sweep reclaims
+// it. Concurrent backup-create jobs are not serialized (jobs.Tracker runs each
+// in its own goroutine), so a just-created sidecar may belong to another
+// in-flight VACUUM INTO. Only files older than this are treated as crash debris,
+// which keeps concurrent creates safe while still reclaiming leaked files. The
+// threshold is far above any realistic VACUUM INTO duration.
+const staleTmpAge = 10 * time.Minute
+
+// sweepStaleTmpFiles removes leftover ".tmp" VACUUM INTO sidecars from a
+// previously crashed runCreate, skipping any modified within staleTmpAge so a
+// concurrent create's in-progress sidecar is never deleted out from under it.
+// Best-effort: read/stat/remove failures are logged and ignored so a stale
+// file never blocks a fresh backup.
+func (s *Service) sweepStaleTmpFiles() {
+	entries, err := os.ReadDir(s.backupsDir())
+	if err != nil {
+		return
+	}
+	for _, e := range entries {
+		if e.IsDir() || !strings.HasSuffix(e.Name(), tmpSuffix) {
+			continue
+		}
+		info, err := e.Info()
+		if err != nil || time.Since(info.ModTime()) < staleTmpAge {
+			continue
+		}
+		p := filepath.Join(s.backupsDir(), e.Name())
+		if err := os.Remove(p); err != nil && s.log != nil {
+			s.log.Warn("backups: failed to remove stale tmp snapshot", zap.String("path", p), zap.Error(err))
+		}
+	}
+}
+
 // Restore validates the confirm token, then runs the restore as a job.
 // Returns the job ID on success, or an error if the token is wrong.
 func (s *Service) Restore(ctx context.Context, name, confirm string) (string, error) {

apps/backend/internal/system/backups/store_test.go

@@ -160,6 +160,139 @@ func TestCreate_ProducesManualVacuumIntoFile(t *testing.T) {
 	}
 }
 
+// Regression for the truncated-size bug: the manual snapshot must be written
+// to a ".tmp" sidecar and atomically renamed into place, so a concurrent
+// List() never stats a half-written file. After a successful Create there
+// must be exactly one "manual-*.db" and no leftover ".tmp" file, and the
+// reported size must equal the final file's on-disk size.
+func TestCreate_AtomicRenameLeavesNoTmpAndReportsFullSize(t *testing.T) {
+	svc, dataDir := newTestService(t)
+	id := svc.Create(context.Background())
+	waitForJob(t, svc.jobs, id, jobs.StateSucceeded)
+
+	backupsDir := filepath.Join(dataDir, "backups")
+	entries, err := os.ReadDir(backupsDir)
+	if err != nil {
+		t.Fatalf("readdir: %v", err)
+	}
+	var manualFile string
+	for _, e := range entries {
+		if strings.HasSuffix(e.Name(), ".tmp") {
+			t.Errorf("leftover tmp file after Create: %s", e.Name())
+		}
+		if strings.HasPrefix(e.Name(), "manual-") && strings.HasSuffix(e.Name(), ".db") {
+			manualFile = filepath.Join(backupsDir, e.Name())
+		}
+	}
+	if manualFile == "" {
+		t.Fatalf("manual-*.db not produced; dir contents: %+v", entries)
+	}
+
+	// The size reported in the job result must match the final file on disk,
+	// not a partial VACUUM INTO write.
+	info, err := os.Stat(manualFile)
+	if err != nil {
+		t.Fatalf("stat manual file: %v", err)
+	}
+	job := svc.jobs.Get(id)
+	if job == nil {
+		t.Fatal("job not found")
+	}
+	reported, ok := job.Result["size_bytes"].(int64)
+	if !ok {
+		t.Fatalf("size_bytes missing or wrong type in job result: %+v", job.Result)
+	}
+	if reported != info.Size() {
+		t.Errorf("reported size %d != on-disk size %d", reported, info.Size())
+	}
+}
+
+// A ".tmp" sidecar (an in-progress VACUUM INTO) must never appear in List(),
+// so the UI cannot show or restore a half-written snapshot.
+func TestList_IgnoresTmpSidecar(t *testing.T) {
+	svc, dataDir := newTestService(t)
+	backupsDir := filepath.Join(dataDir, "backups")
+	if err := os.MkdirAll(backupsDir, 0o755); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+	seed := []string{
+		"manual-1700000000.db",
+		"manual-1700000001.db.tmp",
+	}
+	for _, n := range seed {
+		if err := os.WriteFile(filepath.Join(backupsDir, n), []byte("x"), 0o644); err != nil {
+			t.Fatalf("write %s: %v", n, err)
+		}
+	}
+
+	got, err := svc.List()
+	if err != nil {
+		t.Fatalf("List: %v", err)
+	}
+	if len(got) != 1 || got[0].Name != "manual-1700000000.db" {
+		t.Errorf("expected only the completed .db snapshot, got %+v", got)
+	}
+}
+
+// A crash between SnapshotSQLite and os.Rename leaves a ".tmp" sidecar that
+// classify() hides from List()/Delete(), so it could never be cleaned up via
+// the UI. Create must sweep such stale (old) sidecars before writing a new
+// snapshot.
+func TestCreate_SweepsStaleTmpSidecar(t *testing.T) {
+	svc, dataDir := newTestService(t)
+	backupsDir := filepath.Join(dataDir, "backups")
+	if err := os.MkdirAll(backupsDir, 0o755); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+	stale := filepath.Join(backupsDir, "manual-1700000000.db.tmp")
+	if err := os.WriteFile(stale, []byte("crashed vacuum debris"), 0o644); err != nil {
+		t.Fatalf("seed stale tmp: %v", err)
+	}
+	// Backdate past the age guard so the sweep treats it as crash debris.
+	old := time.Now().Add(-2 * staleTmpAge)
+	if err := os.Chtimes(stale, old, old); err != nil {
+		t.Fatalf("chtimes stale tmp: %v", err)
+	}
+
+	id := svc.Create(context.Background())
+	waitForJob(t, svc.jobs, id, jobs.StateSucceeded)
+
+	if _, err := os.Stat(stale); !os.IsNotExist(err) {
+		t.Errorf("stale tmp sidecar not swept: stat err = %v", err)
+	}
+	entries, err := os.ReadDir(backupsDir)
+	if err != nil {
+		t.Fatalf("readdir: %v", err)
+	}
+	for _, e := range entries {
+		if strings.HasSuffix(e.Name(), ".tmp") {
+			t.Errorf("leftover tmp file after Create: %s", e.Name())
+		}
+	}
+}
+
+// A recently-modified ".tmp" sidecar may be the in-progress VACUUM INTO of a
+// concurrent Create job (jobs are not serialized), so the sweep must leave it
+// alone — deleting it would make the other job's os.Rename fail with ENOENT.
+func TestCreate_PreservesRecentTmpSidecar(t *testing.T) {
+	svc, dataDir := newTestService(t)
+	backupsDir := filepath.Join(dataDir, "backups")
+	if err := os.MkdirAll(backupsDir, 0o755); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+	recent := filepath.Join(backupsDir, "manual-1700000001.db.tmp")
+	if err := os.WriteFile(recent, []byte("another job's in-progress vacuum"), 0o644); err != nil {
+		t.Fatalf("seed recent tmp: %v", err)
+	}
+
+	id := svc.Create(context.Background())
+	waitForJob(t, svc.jobs, id, jobs.StateSucceeded)
+
+	if _, err := os.Stat(recent); err != nil {
+		t.Errorf("recent tmp sidecar was swept but should have been preserved: %v", err)
+	}
+}
+
 // Core retention property: persistence.PruneBackups(dir, 0) must remove all
 // auto snapshots but must NOT touch manual snapshots. This is the contract
 // that lets manual snapshots survive the existing pre-migration pruning.

apps/web/lib/ui/state-icons.test.tsx

@@ -72,6 +72,24 @@ describe("shouldShowTaskRunningSpinner", () => {
     expect(shouldShowTaskRunningSpinner("IN_PROGRESS", "IDLE")).toBe(false);
   });
 
+  it("suppresses the spinner for a CREATED primary session on an inactive task", () => {
+    // Repro from the stuck kanban cards (PR #11571 / #11502): task CREATED,
+    // sitting in a Waiting column, with a primary session that was persisted in
+    // CREATED and never advanced (no executor, no turns). CREATED means "agent
+    // not started", so it must defer to the task state instead of spinning.
+    expect(shouldShowTaskRunningSpinner("CREATED", "CREATED")).toBe(false);
+    expect(shouldShowTaskRunningSpinner("REVIEW", "CREATED")).toBe(false);
+    expect(shouldShowTaskRunningSpinner("COMPLETED", "CREATED")).toBe(false);
+  });
+
+  it("still spins for a CREATED primary session during a genuine launch", () => {
+    // During an actual launch the task state is SCHEDULING/IN_PROGRESS while the
+    // session momentarily sits in CREATED. Deferring to the task state keeps the
+    // spinner on for that startup window.
+    expect(shouldShowTaskRunningSpinner("SCHEDULING", "CREATED")).toBe(true);
+    expect(shouldShowTaskRunningSpinner("IN_PROGRESS", "CREATED")).toBe(true);
+  });
+
   it("suppresses the spinner for TODO regardless of primary session state", () => {
     // TODO is the queued/not-started column. A stale primary session state
     // (e.g. task moved back from IN_PROGRESS with the session still alive)

apps/web/lib/ui/state-icons.tsx

@@ -73,11 +73,10 @@ export function shouldUseQuestionTaskIcon(
 }
 
 // Session states where the agent is actively running work. Anything outside
-// this set (WAITING_FOR_INPUT, IDLE, COMPLETED, FAILED, CANCELLED) is paused
-// or terminal and must not drive the spinner — even when the task is still
-// in the IN_PROGRESS workflow column.
+// this set (CREATED, WAITING_FOR_INPUT, IDLE, COMPLETED, FAILED, CANCELLED) is
+// not-yet-started, paused, or terminal and must not drive the spinner on its
+// own — even when the task is still in the IN_PROGRESS workflow column.
 const ACTIVE_SESSION_STATES: ReadonlySet<string> = new Set<TaskSessionState>([
-  "CREATED",
   "STARTING",
   "RUNNING",
 ]);
@@ -93,6 +92,11 @@ const ACTIVE_SESSION_STATES: ReadonlySet<string> = new Set<TaskSessionState>([
  * we still show the spinner so users see the imminent work; otherwise we
  * require an active session state.
  *
+ * `CREATED` means the session row exists but the agent has not started. During
+ * a genuine launch the task state is SCHEDULING/IN_PROGRESS, so we defer to the
+ * task state; but an orphaned/resting CREATED session on an otherwise inactive
+ * task (e.g. task CREATED, sitting in a Waiting column) must not spin.
+ *
  * Exception: `TODO` is the queued/not-started column. Any active session
  * state reported there is stale (task moved back from IN_PROGRESS, session
  * still alive) or transient, and the spinner would mislead — suppress it.
@@ -102,7 +106,11 @@ export function shouldShowTaskRunningSpinner(
   primarySessionState?: string | null,
 ): boolean {
   if (taskState === "TODO") return false;
-  if (primarySessionState) return ACTIVE_SESSION_STATES.has(primarySessionState);
+  const sessionIsKnownAndNotCreated =
+    primarySessionState != null && primarySessionState !== "CREATED";
+  if (sessionIsKnownAndNotCreated) {
+    return ACTIVE_SESSION_STATES.has(primarySessionState);
+  }
   return taskState === "IN_PROGRESS" || taskState === "SCHEDULING";
 }