diff --git a/README.md b/README.md index d8a0d84a2..b201dfea4 100644 --- a/README.md +++ b/README.md @@ -313,7 +313,7 @@ zero context context-budget report zero repo-map deterministic repository map zero repo-info local repository summary zero search | find search local session history -zero sessions inspect, resume, fork, and rewind sessions +zero sessions inspect, resume, fork, rewind, and prune sessions zero spec manage spec-mode drafts zero specialist manage specialist subagents zero skills manage markdown instruction skills diff --git a/README_ZH.md b/README_ZH.md index 9ff7f7b84..2da2a9e2a 100644 --- a/README_ZH.md +++ b/README_ZH.md @@ -218,7 +218,7 @@ zero context 上下文预算报告 zero repo-map 确定性仓库映射 zero repo-info 本地仓库摘要 zero search | find 搜索本地会话历史 -zero sessions 检查、恢复、分叉和回滚会话 +zero sessions 检查、恢复、分叉、回滚和清理会话 zero spec 管理规范模式草稿 zero specialist 管理专业子智能体 zero skills 管理 Markdown 指令技能 diff --git a/internal/acp/agent.go b/internal/acp/agent.go index 7c301044c..27f16b281 100644 --- a/internal/acp/agent.go +++ b/internal/acp/agent.go @@ -301,6 +301,12 @@ func (a *Agent) activatePersistedSession(ctx context.Context, p LoadSessionParam if operation == persistedSessionResume && historyErr != nil { return nil, RPCError(codeInternalError, "restore session history: "+historyErr.Error()) } + // Load stays best effort about a history it cannot read, but not about one + // prune holds: publishing it would leave the client using a session prune may + // be removing, one this process could not hold open. + if errors.Is(historyErr, sessions.ErrPruning) { + return nil, RPCError(codeInternalError, historyErr.Error()) + } model, models, restrictModels, err := a.resolveModelChoices(ctx, root) if err != nil { return nil, RPCError(codeInternalError, "config: "+err.Error()) @@ -908,9 +914,20 @@ func (a *Agent) loadHistory(sessionID string, requireHistoryLog bool) ([]turnRec // enough: rehydration substitutes the compaction event in place of the events // it replaced, so a loop that skips everything but EventMessage would drop the // summary exactly as before. It is projected below. Reported by @jatmn. + // + // The session was picked from its metadata a moment ago. Make sure it is + // still there, and held, before restoring it: see sessions.HoldToContinue. + if err := a.deps.Store.HoldToContinue(sessionID); err != nil { + return nil, nil, nil, err + } events, eventLogPresent, err := a.deps.Store.ReadRehydratedEventsWithPresence(sessionID) var rehydrateWarning error if err != nil { + if errors.Is(err, sessions.ErrPruning) { + // Not a rehydration failure: the raw read would continue the session + // without holding it while zero sessions prune may be removing it. + return nil, nil, nil, err + } rehydrateWarning = err events, eventLogPresent, err = a.deps.Store.ReadEventsWithPresence(sessionID) if err != nil { diff --git a/internal/acp/agent_test.go b/internal/acp/agent_test.go index 41457549e..364fa120d 100644 --- a/internal/acp/agent_test.go +++ b/internal/acp/agent_test.go @@ -2418,3 +2418,68 @@ func payloadString(payload any, key string) string { value, _ := decoded[key].(string) return value } + +// Load is best effort about a history it cannot read, but a session that zero +// sessions prune holds is refused by load and resume alike, and not published: +// this process could not hold it open while prune may be removing it. +func TestACPLoadAndResumeAreRefusedWhilePruneHoldsTheSession(t *testing.T) { + deps := testDeps(t) + cwd := t.TempDir() + meta, err := deps.Store.Create(sessions.CreateInput{Title: "ACP session", Cwd: cwd}) + if err != nil { + t.Fatalf("create session: %v", err) + } + // Created by an earlier process, which has since exited. + deps.Store.Release(meta.SessionID) + + release, locked, err := sessions.NewStore(sessions.StoreOptions{RootDir: deps.Store.RootDir}).HoldExclusive(meta.SessionID) + if err != nil || !locked { + t.Fatalf("take the lease the way prune does: locked=%v, %v", locked, err) + } + defer release() + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + h := newHarness(t, deps) + defer h.stop() + if err := h.client.Call(ctx, MethodSessionLoad, LoadSessionParams{SessionID: meta.SessionID, Cwd: cwd, McpServers: []McpServer{}}, &LoadSessionResult{}); err == nil || !strings.Contains(err.Error(), "locked by zero sessions prune") { + t.Errorf("session/load while prune holds the session: err = %v, want it refused", err) + } + if err := h.client.Call(ctx, MethodSessionResume, ResumeSessionParams{SessionID: meta.SessionID, Cwd: cwd, McpServers: []McpServer{}}, &ResumeSessionResult{}); err == nil || !strings.Contains(err.Error(), "locked by zero sessions prune") { + t.Errorf("session/resume while prune holds the session: err = %v, want it refused", err) + } + if err := h.client.Call(ctx, MethodSessionPrompt, PromptParams{ + SessionID: meta.SessionID, + Prompt: []ContentBlock{TextBlock("carry on")}, + }, &PromptResult{}); err == nil { + t.Fatal("a session refused while prune held it was still promptable") + } +} + +// Activation reads the session's metadata before it restores the history. A +// session prune removes in between is refused as removed, which activation then +// refuses to publish for load as well as resume, instead of restoring it as an +// empty conversation. +func TestACPLoadHistoryRefusesASessionRemovedAfterItWasPicked(t *testing.T) { + deps := testDeps(t) + meta, err := deps.Store.Create(sessions.CreateInput{Title: "ACP session", Cwd: t.TempDir()}) + if err != nil { + t.Fatalf("create session: %v", err) + } + deps.Store.Release(meta.SessionID) + dir := filepath.Join(deps.Store.RootDir, meta.SessionID) + if err := os.Remove(filepath.Join(dir, sessions.MetadataFile)); err != nil { + t.Fatal(err) + } + + a := &Agent{deps: deps} + if _, _, _, err := a.loadHistory(meta.SessionID, false); !errors.Is(err, sessions.ErrPruning) { + t.Errorf("load history of a session prune is removing: err = %v, want it refused", err) + } + if err := os.RemoveAll(dir); err != nil { + t.Fatal(err) + } + if _, _, _, err := a.loadHistory(meta.SessionID, false); !errors.Is(err, sessions.ErrPruning) { + t.Errorf("load history of a session prune removed: err = %v, want it refused", err) + } +} diff --git a/internal/cli/sessions.go b/internal/cli/sessions.go index b2f76f596..26dc41e90 100644 --- a/internal/cli/sessions.go +++ b/internal/cli/sessions.go @@ -18,6 +18,8 @@ type sessionCommandOptions struct { excludeTarget bool preserveLast int maxPromptChars int + olderThan string + dryRun bool } func runSessions(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) int { @@ -72,6 +74,11 @@ func runSessions(args []string, stdout io.Writer, stderr io.Writer, deps appDeps return writeExecUsageError(stderr, "sessions compact-plan requires a session id") } return runSessionsCompactPlan(store, remaining[0], options, stdout, stderr) + case "prune": + if len(remaining) != 0 { + return writeExecUsageError(stderr, "sessions prune does not accept positional arguments") + } + return runSessionsPrune(store, options, stdout, stderr, deps) default: return writeExecUsageError(stderr, fmt.Sprintf("unknown sessions command %q", command)) } @@ -91,6 +98,8 @@ func parseSessionsArgs(args []string) (string, []string, sessionCommandOptions, options.json = true case "--exclude-target": options.excludeTarget = true + case "--dry-run": + options.dryRun = true case "--kind": value, next, err := nextFlagValue(args, index, arg) if err != nil { @@ -168,6 +177,21 @@ func parseSessionsArgs(args []string) (string, []string, sessionCommandOptions, } options.preserveLast = preserveLast continue + case arg == "--older-than": + value, next, err := nextFlagValue(args, index, arg) + if err != nil { + return command, remaining, options, false, err + } + options.olderThan = value + index = next + continue + case strings.HasPrefix(arg, "--older-than="): + value, err := parseNonEmptySessionsFlag("--older-than", strings.TrimPrefix(arg, "--older-than=")) + if err != nil { + return command, remaining, options, false, err + } + options.olderThan = value + continue case arg == "--max-prompt-chars": value, next, err := nextFlagValue(args, index, arg) if err != nil { @@ -225,7 +249,7 @@ func parseSessionKindFlag(value string) (sessions.SessionKind, error) { func isSessionsCommand(command string) bool { switch command { - case "list", "children", "lineage", "tree", "rewind-plan", "rewind", "compact-plan": + case "list", "children", "lineage", "tree", "rewind-plan", "rewind", "compact-plan", "prune": return true default: return false @@ -244,6 +268,9 @@ func validateSessionCommandFlags(command string, options sessionCommandOptions) if hasCompactionFlag && command != "compact-plan" { return execUsageError{"--preserve-last and --max-prompt-chars are only valid for sessions compact-plan"} } + if (options.olderThan != "" || options.dryRun) && command != "prune" { + return execUsageError{"--older-than and --dry-run are only valid for sessions prune"} + } return nil } @@ -532,6 +559,7 @@ Commands: rewind-plan Preview events kept and dropped by a rewind rewind Restore workspace files and truncate the log to a checkpoint compact-plan Preview events compacted and preserved by compaction + prune Remove sessions last updated before a cutoff Flags: --json Print JSON output @@ -541,7 +569,13 @@ Flags: --exclude-target Drop the target event (rewind-plan, rewind) --preserve-last Keep recent events in compact-plan --max-prompt-chars Limit compact-plan summary prompt + --older-than Prune cutoff, as days (30d) or a duration (720h); at least 1d. + Without it, prune uses sessions.retentionDays from your user config. + --dry-run List what prune would remove, and why it keeps the rest -h, --help Show this help + +prune only runs when you run it. It never removes a session another Zero process +has open, or an ancestor of a session it keeps, and --dry-run shows why. `) return err } diff --git a/internal/cli/sessions_prune.go b/internal/cli/sessions_prune.go new file mode 100644 index 000000000..4a7a84994 --- /dev/null +++ b/internal/cli/sessions_prune.go @@ -0,0 +1,171 @@ +package cli + +import ( + "fmt" + "io" + "strconv" + "strings" + "time" + + "github.com/Gitlawb/zero/internal/config" + "github.com/Gitlawb/zero/internal/redaction" + "github.com/Gitlawb/zero/internal/sessions" +) + +// maxPruneDays keeps a day count from overflowing time.Duration. +const maxPruneDays = 36500 + +// runSessionsPrune removes sessions last updated before a cutoff, or with +// --dry-run lists what would go and why. It runs only when asked (#971). +func runSessionsPrune(store *sessions.Store, options sessionCommandOptions, stdout io.Writer, stderr io.Writer, deps appDeps) int { + olderThan, err := pruneCutoff(options, deps) + if err != nil { + return writeExecUsageError(stderr, err.Error()) + } + report, err := store.Prune(sessions.PruneOptions{OlderThan: olderThan, DryRun: options.dryRun}) + if err != nil { + return writeSessionCommandError(stderr, err) + } + if options.json { + if err := writePrettyJSON(stdout, redaction.RedactValue(report, redaction.Options{})); err != nil { + return exitCrash + } + } else if _, err := fmt.Fprint(stdout, formatPruneReport(report)); err != nil { + return exitCrash + } + if len(report.Failed) > 0 { + return exitCrash + } + return exitSuccess +} + +// pruneCutoff is --older-than, or else sessions.retentionDays from the user's +// own config, and never below sessions.MinimumPruneAge. Project config has no +// say in it: see config.SessionsConfig. +func pruneCutoff(options sessionCommandOptions, deps appDeps) (time.Duration, error) { + var age time.Duration + if options.olderThan != "" { + parsed, err := parsePruneAge(options.olderThan) + if err != nil { + return 0, err + } + age = parsed + } else { + days := 0 + if deps.userConfigPath != nil { + path, err := deps.userConfigPath() + if err != nil { + return 0, fmt.Errorf("sessions prune could not find your config: %w", err) + } + settings, err := config.ReadSessionsConfig(path) + if err != nil { + return 0, fmt.Errorf("sessions prune could not read sessions.retentionDays: %w", err) + } + days = settings.RetentionDays + } + if days == 0 { + return 0, execUsageError{"sessions prune needs a cutoff: pass --older-than (for example --older-than 30d) or set sessions.retentionDays in your user config"} + } + if days > maxPruneDays { + return 0, execUsageError{fmt.Sprintf("sessions.retentionDays %d is more than %d", days, maxPruneDays)} + } + age = time.Duration(days) * 24 * time.Hour + } + if age < sessions.MinimumPruneAge { + return 0, execUsageError{fmt.Sprintf("sessions prune does not remove anything updated in the last %d hours; use --older-than 1d or more", int(sessions.MinimumPruneAge.Hours()))} + } + return age, nil +} + +// parsePruneAge accepts a whole number of days ("30d") or a Go duration +// ("720h"). +func parsePruneAge(value string) (time.Duration, error) { + invalid := execUsageError{fmt.Sprintf("invalid --older-than %q: expected days like 30d or a duration like 720h", value)} + trimmed := strings.TrimSpace(value) + if days, ok := strings.CutSuffix(trimmed, "d"); ok { + count, err := strconv.Atoi(days) + if err != nil || count <= 0 || count > maxPruneDays { + return 0, invalid + } + return time.Duration(count) * 24 * time.Hour, nil + } + age, err := time.ParseDuration(trimmed) + if err != nil || age <= 0 { + return 0, invalid + } + return age, nil +} + +func formatPruneReport(report sessions.PruneReport) string { + var out strings.Builder + verb := "Removed" + if report.DryRun { + verb = "Would remove" + } + if len(report.Removed) == 0 { + fmt.Fprintf(&out, "No sessions to remove: nothing last updated before %s can go.\n", report.Cutoff) + } else { + var total int64 + for _, entry := range report.Removed { + total += entry.Bytes + } + fmt.Fprintf(&out, "%s %d %s last updated before %s (%s):\n", verb, len(report.Removed), plural(len(report.Removed), "session", "sessions"), report.Cutoff, formatPruneBytes(total)) + for _, entry := range report.Removed { + fmt.Fprintf(&out, " %s\n", formatPruneEntry(entry, redact(entry.Title))) + } + } + if len(report.Kept) > 0 { + fmt.Fprintf(&out, "Kept %d old enough to remove:\n", len(report.Kept)) + for _, entry := range report.Kept { + fmt.Fprintf(&out, " %s\n", formatPruneEntry(entry, entry.Reason)) + } + } + if len(report.Failed) > 0 { + failed := "Could not remove" + if report.DryRun { + failed = "Could not check" + } + fmt.Fprintf(&out, "%s %d:\n", failed, len(report.Failed)) + for _, entry := range report.Failed { + fmt.Fprintf(&out, " %s\n", formatPruneEntry(entry, redact(entry.Reason))) + } + } + if report.DryRun { + out.WriteString("Dry run: nothing was removed.\n") + } + return out.String() +} + +func formatPruneEntry(entry sessions.PruneEntry, note string) string { + updated := entry.UpdatedAt + if len(updated) >= len("2006-01-02") { + updated = updated[:len("2006-01-02")] + } + line := redact(entry.SessionID) + " " + updated + if note = strings.TrimSpace(note); note != "" { + line += " " + note + } + return line +} + +func formatPruneBytes(bytes int64) string { + const unit = 1024 + if bytes < unit { + return fmt.Sprintf("%d B", bytes) + } + value, suffix := float64(bytes)/unit, "KB" + for _, next := range []string{"MB", "GB", "TB"} { + if value < unit { + break + } + value, suffix = value/unit, next + } + return fmt.Sprintf("%.1f %s", value, suffix) +} + +func plural(count int, one, many string) string { + if count == 1 { + return one + } + return many +} diff --git a/internal/cli/sessions_prune_test.go b/internal/cli/sessions_prune_test.go new file mode 100644 index 000000000..b23f3f5c5 --- /dev/null +++ b/internal/cli/sessions_prune_test.go @@ -0,0 +1,220 @@ +package cli + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/Gitlawb/zero/internal/sessions" +) + +// pruneCLIFixture is a session root holding one session last written in 2020 +// and one written now, neither held open, plus deps whose user config path is +// the returned file (which does not exist until a test writes it). +func pruneCLIFixture(t *testing.T) (string, string, appDeps) { + t.Helper() + root := t.TempDir() + for _, session := range []struct { + id string + at func() time.Time + }{ + {"old-session", func() time.Time { return time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) }}, + {"recent-session", time.Now}, + } { + store := sessions.NewStore(sessions.StoreOptions{RootDir: root, Now: session.at}) + if _, err := store.Create(sessions.CreateInput{SessionID: session.id, Title: "title of " + session.id}); err != nil { + t.Fatalf("create %s: %v", session.id, err) + } + store.Release(session.id) + } + configPath := filepath.Join(t.TempDir(), "config.json") + deps := appDeps{ + newSessionStore: func() *sessions.Store { return sessions.NewStore(sessions.StoreOptions{RootDir: root}) }, + userConfigPath: func() (string, error) { return configPath, nil }, + } + return root, configPath, deps +} + +func runPruneCLI(t *testing.T, deps appDeps, args ...string) (int, string, string) { + t.Helper() + var stdout, stderr bytes.Buffer + code := runWithDeps(append([]string{"sessions"}, args...), &stdout, &stderr, deps) + return code, stdout.String(), stderr.String() +} + +func pruneSessionExists(t *testing.T, root, id string) bool { + t.Helper() + _, err := os.Stat(filepath.Join(root, id)) + return err == nil +} + +func TestSessionsPruneDryRunListsWithoutRemoving(t *testing.T) { + root, _, deps := pruneCLIFixture(t) + code, stdout, stderr := runPruneCLI(t, deps, "prune", "--older-than", "30d", "--dry-run") + if code != exitSuccess { + t.Fatalf("exit %d, stderr %q", code, stderr) + } + for _, want := range []string{"Would remove 1 session", "old-session", "Dry run: nothing was removed."} { + if !strings.Contains(stdout, want) { + t.Errorf("output does not contain %q:\n%s", want, stdout) + } + } + if strings.Contains(stdout, "recent-session") { + t.Errorf("the recent session was listed:\n%s", stdout) + } + if !pruneSessionExists(t, root, "old-session") { + t.Fatal("a dry run removed the session") + } +} + +// A dry run removes nothing, so a session it could not check is not reported +// as one it could not remove. +func TestSessionsPruneDryRunReportsFailuresAsUnchecked(t *testing.T) { + failed := []sessions.PruneEntry{{SessionID: "old-session", UpdatedAt: "2020-01-01T00:00:00Z", Reason: "the session disappeared while pruning"}} + dryRun := formatPruneReport(sessions.PruneReport{Cutoff: "2026-08-27T00:00:00Z", DryRun: true, Failed: failed}) + if !strings.Contains(dryRun, "Could not check 1:") || strings.Contains(dryRun, "Could not remove") { + t.Errorf("dry run report:\n%s", dryRun) + } + run := formatPruneReport(sessions.PruneReport{Cutoff: "2026-08-27T00:00:00Z", Failed: failed}) + if !strings.Contains(run, "Could not remove 1:") { + t.Errorf("report:\n%s", run) + } +} + +func TestSessionsPruneRemovesOnlyOldSessions(t *testing.T) { + root, _, deps := pruneCLIFixture(t) + code, stdout, stderr := runPruneCLI(t, deps, "prune", "--older-than=30d") + if code != exitSuccess { + t.Fatalf("exit %d, stderr %q", code, stderr) + } + if !strings.Contains(stdout, "Removed 1 session") || !strings.Contains(stdout, "old-session") { + t.Fatalf("output does not report the removal:\n%s", stdout) + } + if pruneSessionExists(t, root, "old-session") { + t.Fatal("the old session is still there") + } + if !pruneSessionExists(t, root, "recent-session") { + t.Fatal("the recent session was removed") + } +} + +func TestSessionsPruneReportsJSON(t *testing.T) { + _, _, deps := pruneCLIFixture(t) + code, stdout, stderr := runPruneCLI(t, deps, "prune", "--older-than", "720h", "--json") + if code != exitSuccess { + t.Fatalf("exit %d, stderr %q", code, stderr) + } + var report sessions.PruneReport + if err := json.Unmarshal([]byte(stdout), &report); err != nil { + t.Fatalf("output is not a prune report: %v\n%s", err, stdout) + } + if len(report.Removed) != 1 || report.Removed[0].SessionID != "old-session" || report.DryRun { + t.Fatalf("report = %+v", report) + } +} + +func TestSessionsPruneRefusesABadOrShortCutoff(t *testing.T) { + root, _, deps := pruneCLIFixture(t) + for _, tc := range []struct { + value string + want string + }{ + {"12h", "last 24 hours"}, + {"0d", "invalid --older-than"}, + {"soon", "invalid --older-than"}, + {"-5d", "invalid --older-than"}, + } { + code, _, stderr := runPruneCLI(t, deps, "prune", "--older-than="+tc.value) + if code != exitUsage || !strings.Contains(stderr, tc.want) { + t.Errorf("--older-than %s: exit %d, stderr %q, want a usage error naming %q", tc.value, code, stderr, tc.want) + } + } + if !pruneSessionExists(t, root, "old-session") { + t.Fatal("a refused prune removed a session") + } +} + +func TestSessionsPruneNeedsACutoff(t *testing.T) { + root, _, deps := pruneCLIFixture(t) + code, _, stderr := runPruneCLI(t, deps, "prune") + if code != exitUsage || !strings.Contains(stderr, "needs a cutoff") { + t.Fatalf("exit %d, stderr %q, want a usage error asking for a cutoff", code, stderr) + } + if !pruneSessionExists(t, root, "old-session") { + t.Fatal("prune without a cutoff removed a session") + } +} + +func TestSessionsPruneUsesTheUserRetentionSetting(t *testing.T) { + root, configPath, deps := pruneCLIFixture(t) + if err := os.WriteFile(configPath, []byte(`{"sessions":{"retentionDays":30}}`), 0o600); err != nil { + t.Fatal(err) + } + code, stdout, stderr := runPruneCLI(t, deps, "prune") + if code != exitSuccess { + t.Fatalf("exit %d, stderr %q", code, stderr) + } + if !strings.Contains(stdout, "Removed 1 session") || pruneSessionExists(t, root, "old-session") { + t.Fatalf("the user retention setting was not applied:\n%s", stdout) + } +} + +// A REPOSITORY CANNOT CHOOSE WHAT PRUNE DELETES. Sessions are the user's, not +// the project's, so sessions.retentionDays in a workspace's .zero/config.json +// is not a cutoff, even when prune runs from inside that workspace. +func TestSessionsPruneIgnoresRetentionInProjectConfig(t *testing.T) { + root, _, deps := pruneCLIFixture(t) + workspace := t.TempDir() + if err := os.MkdirAll(filepath.Join(workspace, ".zero"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(workspace, ".zero", "config.json"), []byte(`{"sessions":{"retentionDays":1}}`), 0o600); err != nil { + t.Fatal(err) + } + deps.getwd = func() (string, error) { return workspace, nil } + t.Chdir(workspace) + + code, _, stderr := runPruneCLI(t, deps, "prune") + if code != exitUsage || !strings.Contains(stderr, "needs a cutoff") { + t.Fatalf("exit %d, stderr %q: the project's retention setting was used", code, stderr) + } + if !pruneSessionExists(t, root, "old-session") { + t.Fatal("a project config setting removed a session") + } +} + +func TestSessionsPruneFlagsBelongToPrune(t *testing.T) { + _, _, deps := pruneCLIFixture(t) + for _, args := range [][]string{{"list", "--dry-run"}, {"list", "--older-than", "30d"}} { + code, _, stderr := runPruneCLI(t, deps, args...) + if code != exitUsage || !strings.Contains(stderr, "only valid for sessions prune") { + t.Errorf("%v: exit %d, stderr %q", args, code, stderr) + } + } +} + +// An empty cutoff is a usage error, never a fall back to sessions.retentionDays. +func TestSessionsPruneRejectsAnEmptyCutoff(t *testing.T) { + root, configPath, deps := pruneCLIFixture(t) + if err := os.WriteFile(configPath, []byte(`{"sessions":{"retentionDays":1}}`), 0o600); err != nil { + t.Fatal(err) + } + for _, args := range [][]string{ + {"prune", "--older-than", ""}, + {"prune", "--older-than", " "}, + {"prune", "--older-than="}, + {"list", "--older-than", ""}, + } { + code, _, stderr := runPruneCLI(t, deps, args...) + if code != exitUsage || !strings.Contains(stderr, "--older-than requires a value") { + t.Errorf("%q: exit %d, stderr %q, want a usage error", args, code, stderr) + } + } + if !pruneSessionExists(t, root, "old-session") { + t.Fatal("an empty cutoff fell back to the retention setting and removed a session") + } +} diff --git a/internal/config/sessions_config.go b/internal/config/sessions_config.go new file mode 100644 index 000000000..22c1a57b6 --- /dev/null +++ b/internal/config/sessions_config.go @@ -0,0 +1,20 @@ +package config + +import ( + "errors" + "os" +) + +// ReadSessionsConfig reads the sessions settings from the config file at path, +// which callers pass as the user config. A missing file is an empty config. See +// SessionsConfig for why nothing else supplies these settings. +func ReadSessionsConfig(path string) (SessionsConfig, error) { + cfg, err := loadConfigFile(path) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return SessionsConfig{}, nil + } + return SessionsConfig{}, err + } + return cfg.Sessions, nil +} diff --git a/internal/config/sessions_config_test.go b/internal/config/sessions_config_test.go new file mode 100644 index 000000000..643a16cd3 --- /dev/null +++ b/internal/config/sessions_config_test.go @@ -0,0 +1,76 @@ +package config + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSessionsConfigRoundTripsAsAKnownKey(t *testing.T) { + var cfg FileConfig + if err := json.Unmarshal([]byte(`{"sessions":{"retentionDays":14}}`), &cfg); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if cfg.Sessions.RetentionDays != 14 { + t.Fatalf("retentionDays = %d, want 14", cfg.Sessions.RetentionDays) + } + if _, stray := cfg.Extra["sessions"]; stray { + t.Fatal("sessions was kept as an unknown key as well as parsed") + } + data, err := json.Marshal(cfg) + if err != nil { + t.Fatalf("marshal: %v", err) + } + if !strings.Contains(string(data), `"sessions":{"retentionDays":14}`) { + t.Fatalf("marshalled config lost the setting: %s", data) + } + empty, err := json.Marshal(FileConfig{}) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(empty), "sessions") { + t.Fatalf("an unset sessions block was written out: %s", empty) + } +} + +func TestSessionsConfigRejectsANegativeRetention(t *testing.T) { + var cfg FileConfig + err := json.Unmarshal([]byte(`{"sessions":{"retentionDays":-1}}`), &cfg) + if err == nil || !strings.Contains(err.Error(), "retentionDays") { + t.Fatalf("a negative retention was accepted: %v", err) + } +} + +// A write that changes something else must not drop the setting. +func TestSessionsConfigSurvivesAnUnrelatedWrite(t *testing.T) { + path := filepath.Join(t.TempDir(), "config.json") + if err := os.WriteFile(path, []byte(`{"sessions":{"retentionDays":21}}`), 0o600); err != nil { + t.Fatal(err) + } + if _, err := SetTheme(path, "dark"); err != nil { + t.Fatalf("SetTheme: %v", err) + } + settings, err := ReadSessionsConfig(path) + if err != nil { + t.Fatalf("ReadSessionsConfig: %v", err) + } + if settings.RetentionDays != 21 { + t.Fatalf("after an unrelated write retentionDays = %d, want 21", settings.RetentionDays) + } +} + +func TestReadSessionsConfig(t *testing.T) { + dir := t.TempDir() + if settings, err := ReadSessionsConfig(filepath.Join(dir, "missing.json")); err != nil || settings != (SessionsConfig{}) { + t.Fatalf("a missing config = %+v, %v; want empty and no error", settings, err) + } + broken := filepath.Join(dir, "broken.json") + if err := os.WriteFile(broken, []byte(`{"sessions":`), 0o600); err != nil { + t.Fatal(err) + } + if _, err := ReadSessionsConfig(broken); err == nil { + t.Fatal("a config that does not parse was read as empty") + } +} diff --git a/internal/config/types.go b/internal/config/types.go index 9343c7c94..adade9620 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -177,6 +177,19 @@ const ( STTProviderDeepgram STTProviderKind = "deepgram" ) +// SessionsConfig holds settings for Zero's saved sessions. +// +// READ FROM THE USER'S OWN CONFIG ONLY. It decides what `zero sessions prune` +// deletes, and sessions belong to the user rather than to a repository, so a +// project's .zero/config.json has no say: the resolver does not merge it, and +// the prune command reads it straight from the user config file. +type SessionsConfig struct { + // RetentionDays is the cutoff `zero sessions prune` uses when it is given no + // --older-than. Unset (0) means prune needs an explicit cutoff; nothing is + // ever removed without one being asked for. + RetentionDays int `json:"retentionDays,omitempty"` +} + // STTConfig configures speech-to-text dictation. All fields are optional; empty // values take the documented defaults. Booleans that need a real tri-state // (distinguishing "unset" from "false") use *bool, matching PreferencesConfig.Recaps. @@ -364,6 +377,7 @@ type FileConfig struct { LocalControl LocalControlConfig `json:"localControl,omitempty"` STT STTConfig `json:"stt,omitempty"` CrossSessionInbound string `json:"crossSessionInbound,omitempty"` + Sessions SessionsConfig `json:"sessions,omitempty"` // maxTurnsSet records that some merge source supplied a positive maxTurns — // i.e. the value is configured, not the built-in default. Unexported like // Tools.deferThresholdSet: merge bookkeeping, not a config key. @@ -388,6 +402,7 @@ func (cfg FileConfig) MarshalJSON() ([]byte, error) { LocalControl *LocalControlConfig `json:"localControl,omitempty"` STT *STTConfig `json:"stt,omitempty"` CrossSessionInbound string `json:"crossSessionInbound,omitempty"` + Sessions *SessionsConfig `json:"sessions,omitempty"` } raw := rawConfig{ ActiveProvider: cfg.ActiveProvider, @@ -408,6 +423,9 @@ func (cfg FileConfig) MarshalJSON() ([]byte, error) { if !cfg.STT.Empty() { raw.STT = &cfg.STT } + if cfg.Sessions != (SessionsConfig{}) { + raw.Sessions = &cfg.Sessions + } known, err := json.Marshal(raw) if err != nil || len(cfg.Extra) == 0 { return known, err @@ -546,6 +564,7 @@ func (cfg *FileConfig) UnmarshalJSON(data []byte) error { LocalControl LocalControlConfig `json:"localControl"` STT STTConfig `json:"stt"` CrossSessionInbound string `json:"crossSessionInbound"` + Sessions SessionsConfig `json:"sessions"` MCPServers map[string]MCPServerConfig `json:"mcpServers"` MCPServersSnake map[string]MCPServerConfig `json:"mcp_servers"` } @@ -587,6 +606,10 @@ func (cfg *FileConfig) UnmarshalJSON(data []byte) error { cfg.LocalControl = raw.LocalControl cfg.STT = raw.STT cfg.CrossSessionInbound = raw.CrossSessionInbound + if raw.Sessions.RetentionDays < 0 { + return fmt.Errorf("invalid sessions.retentionDays %d: must be >= 0", raw.Sessions.RetentionDays) + } + cfg.Sessions = raw.Sessions cfg.Extra = extra if cfg.MCP.Servers == nil && (len(raw.MCPServers) > 0 || len(raw.MCPServersSnake) > 0) { cfg.MCP.Servers = map[string]MCPServerConfig{} diff --git a/internal/sessions/exec_session.go b/internal/sessions/exec_session.go index 28e778ab3..41540e9b5 100644 --- a/internal/sessions/exec_session.go +++ b/internal/sessions/exec_session.go @@ -2,6 +2,7 @@ package sessions import ( "encoding/json" + "errors" "fmt" "log" "sort" @@ -151,10 +152,20 @@ func PrepareExec(options PrepareExecOptions) (PreparedExec, error) { } func readExecContextEvents(store *Store, sessionID string) ([]Event, error) { + // The session was picked from its metadata a moment ago. Make sure it is + // still there, and held, before reading it to continue: see HoldToContinue. + if err := store.HoldToContinue(sessionID); err != nil { + return nil, err + } contextEvents, err := store.ReadRehydratedEvents(sessionID) if err == nil { return contextEvents, nil } + if errors.Is(err, ErrPruning) { + // Not a rehydration failure: the raw read would continue the session + // without holding it while prune may be removing it. + return nil, err + } rawEvents, rawErr := store.ReadEvents(sessionID) if rawErr != nil { return nil, err diff --git a/internal/sessions/lease.go b/internal/sessions/lease.go new file mode 100644 index 000000000..469304ee2 --- /dev/null +++ b/internal/sessions/lease.go @@ -0,0 +1,207 @@ +package sessions + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "sync" +) + +// leaseFileName is the file whose lock says a process has the session open. +const leaseFileName = "lease.lock" + +func (store *Store) leasePath(sessionID string) string { + return filepath.Join(store.sessionPath(sessionID), leaseFileName) +} + +// Hold marks sessionID as open in this process until the process exits. +// +// A SESSION HOLDS NO LOCK BETWEEN WRITES. session.lock is taken around each +// write and released straight after, so nothing told `zero sessions prune` in +// another terminal that a TUI, an exec run or an editor still had the session +// open, and an idle open session looked exactly like an abandoned one. Hold takes +// a shared lock on the session's lease file and keeps it for the life of the +// process; prune takes the same lock exclusively and leaves alone any session it +// cannot get. Every write holds its session this way (lockSession). +// +// Hold never waits and never fails its caller. A lease that cannot be taken, +// because the session directory is gone or prune holds it at this moment, only +// means prune cannot see this process, and that is enough for a write: it lands +// under session.lock, where prune checks the session again, so either prune sees +// the write and keeps the session or the write finds the session gone. A read +// changes nothing prune checks, so the reads that continue a session, and the +// creations that read a parent, use holdOrRefuse instead. +func (store *Store) Hold(sessionID string) { + store.hold(sessionID) +} + +// ErrPruning is matched, through errors.Is, by the error a process gets when it +// tries to continue a session, or to create a session under it, while zero +// sessions prune holds that session or once it has removed it. +var ErrPruning = errors.New("locked by zero sessions prune") + +// pruneRefusal is an error matching ErrPruning whose message says which of the +// two it was. +type pruneRefusal struct{ message string } + +func (refusal pruneRefusal) Error() string { return refusal.message } + +func (refusal pruneRefusal) Is(target error) bool { return target == ErrPruning } + +func pruneBusy(sessionID string) error { + return pruneRefusal{"zero session " + sessionID + " is locked by zero sessions prune; try again"} +} + +func pruneRemoved(sessionID string) error { + return pruneRefusal{"zero session " + sessionID + " was removed while it was being opened"} +} + +// holdOrRefuse holds a session that is about to be read in order to continue it +// (the rehydrated read behind the TUI's resume, exec --resume and --fork, and +// ACP's session/load and session/resume) or to create a session under it (Fork, +// CreateChild, Create with a parent). When prune holds it at this moment the +// caller is refused rather than left to read it without the lease: prune could +// then remove a session this process goes on to use, or one a new session is +// created under, whose Lineage and Tree fail on a missing ancestor. +func (store *Store) holdOrRefuse(sessionID string) error { + if store.hold(sessionID) { + return pruneBusy(sessionID) + } + return nil +} + +// holdParent is holdOrRefuse for the session a new one is created under: Fork, +// CreateChild, and Create with a ParentSessionID. +// +// HOLDING A LEASE FILE DOES NOT PROVE THE SESSION IS STILL THERE. Prune removes +// the metadata first and unlinks lease.lock after it, so a process that takes +// the lease just then creates a fresh lease.lock in a directory prune is +// emptying, and locks it with nothing to contend with. So a parent whose +// directory still exists without its metadata is refused once the lease is +// held. A parent whose directory is gone altogether is left alone, as before: a +// session may name a parent this store never had. +func (store *Store) holdParent(parentSessionID string) error { + if err := store.holdOrRefuse(parentSessionID); err != nil { + return err + } + if store.beingRemoved(parentSessionID) { + store.dropStrayLease(parentSessionID) + return pruneRemoved(parentSessionID) + } + return nil +} + +// HoldToContinue holds a session the caller has already picked, having read its +// metadata, and is about to continue: the TUI's resume, exec --resume and +// --fork, and ACP's session/load and session/resume. It refuses, with an error +// matching ErrPruning, a session prune holds and one that is gone by the time it +// is held, whether prune is part way through removing it or has finished. +// Callers must not fall back to reading the session another way on that error. +// ReadRehydratedEvents on its own still reads a session that does not exist as +// an empty one, for callers that picked nothing. +func (store *Store) HoldToContinue(sessionID string) error { + if err := store.holdOrRefuse(sessionID); err != nil { + return err + } + if _, err := os.Stat(store.metadataPath(sessionID)); errors.Is(err, fs.ErrNotExist) { + store.dropStrayLease(sessionID) + return pruneRemoved(sessionID) + } + return nil +} + +// beingRemoved reports a session directory that exists without its metadata: +// one prune is part way through removing. +func (store *Store) beingRemoved(sessionID string) bool { + if _, err := os.Stat(store.metadataPath(sessionID)); !errors.Is(err, fs.ErrNotExist) { + return false + } + info, err := os.Stat(store.sessionPath(sessionID)) + return err == nil && info.IsDir() +} + +// dropStrayLease gives back a lease taken on a session that turned out to be +// gone, and removes the lease file it may have created afresh in a directory +// prune is emptying, so that prune can still remove the directory. +func (store *Store) dropStrayLease(sessionID string) { + store.Release(sessionID) + if store.beingRemoved(sessionID) { + _ = os.Remove(store.leasePath(sessionID)) + } +} + +// hold is Hold, reporting busy when the lease could not be taken because prune +// holds it exclusively right now. +func (store *Store) hold(sessionID string) (busy bool) { + if !ValidSessionID(sessionID) { + return false + } + store.leasesMu.Lock() + defer store.leasesMu.Unlock() + if _, held := store.leases[sessionID]; held { + return false + } + // Creates the lease file, never the directory: a session that is gone stays + // gone. + file, err := openLeaseFile(store.leasePath(sessionID)) + if err != nil { + return false + } + locked, err := tryLockLease(file, false) + if err != nil || !locked { + _ = file.Close() + return err == nil + } + if store.leases == nil { + store.leases = map[string]*os.File{} + } + store.leases[sessionID] = file + return false +} + +// Release gives up this Store's lease on sessionID. Nothing in Zero calls it +// yet: a process holds every session it has touched until it exits, which +// releases every lease with it, and tests use Release to stand in for that +// exit. A long-lived process that switches sessions, like the TUI on /new, could +// call it for the session it leaves. +func (store *Store) Release(sessionID string) { + store.leasesMu.Lock() + defer store.leasesMu.Unlock() + file, held := store.leases[sessionID] + if !held { + return + } + delete(store.leases, sessionID) + unlockLease(file) + _ = file.Close() +} + +// HoldExclusive takes sessionID's lease exclusively without waiting, which is how +// prune keeps every other process out of a session while it checks or removes +// it. It reports false, holding nothing, when any process holds the lease. While +// it is held, Hold takes nothing and holdOrRefuse refuses. release lets it go, +// and does nothing after the first call. +func (store *Store) HoldExclusive(sessionID string) (release func(), locked bool, err error) { + nothing := func() {} + if !ValidSessionID(sessionID) { + return nothing, false, fmt.Errorf("invalid zero session id %q", sessionID) + } + file, err := openLeaseFile(store.leasePath(sessionID)) + if err != nil { + return nothing, false, err + } + locked, err = tryLockLease(file, true) + if err != nil || !locked { + _ = file.Close() + return nothing, false, err + } + var once sync.Once + return func() { + once.Do(func() { + unlockLease(file) + _ = file.Close() + }) + }, true, nil +} diff --git a/internal/sessions/lease_unix.go b/internal/sessions/lease_unix.go new file mode 100644 index 000000000..d323bcf57 --- /dev/null +++ b/internal/sessions/lease_unix.go @@ -0,0 +1,40 @@ +//go:build !windows + +package sessions + +import ( + "errors" + "os" + + "golang.org/x/sys/unix" +) + +// tryLockLease takes the lease lock on file without waiting: shared for a +// process that has the session open, exclusive for prune. It reports false when +// another holder's lock conflicts. flock locks belong to the open file +// description, so a second descriptor in the same process conflicts like another +// process would. +func tryLockLease(file *os.File, exclusive bool) (bool, error) { + how := unix.LOCK_SH + if exclusive { + how = unix.LOCK_EX + } + err := unix.Flock(int(file.Fd()), how|unix.LOCK_NB) + if err == nil { + return true, nil + } + if errors.Is(err, unix.EWOULDBLOCK) { + return false, nil + } + return false, err +} + +// openLeaseFile opens (creating it if needed) the lease file of a session whose +// directory exists. +func openLeaseFile(path string) (*os.File, error) { + return os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0o600) +} + +func unlockLease(file *os.File) { + _ = unix.Flock(int(file.Fd()), unix.LOCK_UN) +} diff --git a/internal/sessions/lease_windows.go b/internal/sessions/lease_windows.go new file mode 100644 index 000000000..207bb7b1e --- /dev/null +++ b/internal/sessions/lease_windows.go @@ -0,0 +1,56 @@ +//go:build windows + +package sessions + +import ( + "errors" + "os" + + "golang.org/x/sys/windows" +) + +// tryLockLease takes the lease lock on file without waiting: shared for a +// process that has the session open, exclusive for prune. It reports false when +// another holder's lock conflicts. LockFileEx locks belong to the handle, so a +// second handle in the same process conflicts like another process would. +func tryLockLease(file *os.File, exclusive bool) (bool, error) { + flags := uint32(windows.LOCKFILE_FAIL_IMMEDIATELY) + if exclusive { + flags |= windows.LOCKFILE_EXCLUSIVE_LOCK + } + err := windows.LockFileEx(windows.Handle(file.Fd()), flags, 0, 1, 0, new(windows.Overlapped)) + if err == nil { + return true, nil + } + if errors.Is(err, windows.ERROR_LOCK_VIOLATION) { + return false, nil + } + return false, err +} + +// openLeaseFile opens (creating it if needed) the lease file of a session whose +// directory exists. +// +// WITH FILE_SHARE_DELETE, unlike session.lock. A lease is held for the life of +// the process, and a file held open without share-delete cannot be removed: the +// session directory could then never be deleted while any process had it open, +// by prune or by anything else, including a test's temp-directory cleanup. With +// it, a delete removes the name at once (POSIX semantics) while the holder's +// handle stays valid. +func openLeaseFile(path string) (*os.File, error) { + pathUTF16, err := windows.UTF16PtrFromString(path) + if err != nil { + return nil, &os.PathError{Op: "open", Path: path, Err: err} + } + handle, err := windows.CreateFile(pathUTF16, windows.GENERIC_READ|windows.GENERIC_WRITE, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + nil, windows.OPEN_ALWAYS, windows.FILE_ATTRIBUTE_NORMAL, 0) + if err != nil { + return nil, &os.PathError{Op: "open", Path: path, Err: err} + } + return os.NewFile(uintptr(handle), path), nil +} + +func unlockLease(file *os.File) { + _ = windows.UnlockFileEx(windows.Handle(file.Fd()), 0, 1, 0, new(windows.Overlapped)) +} diff --git a/internal/sessions/lineage.go b/internal/sessions/lineage.go index 433dbe5dd..36d4dd7ac 100644 --- a/internal/sessions/lineage.go +++ b/internal/sessions/lineage.go @@ -10,6 +10,9 @@ func (store *Store) CreateChild(parentSessionID string, input ChildInput) (Metad if !ValidSessionID(parentSessionID) { return Metadata{}, fmt.Errorf("invalid zero session id %q", parentSessionID) } + if err := store.holdParent(parentSessionID); err != nil { + return Metadata{}, err + } parent, err := store.Get(parentSessionID) if err != nil { return Metadata{}, err diff --git a/internal/sessions/prune.go b/internal/sessions/prune.go new file mode 100644 index 000000000..2077bf1b6 --- /dev/null +++ b/internal/sessions/prune.go @@ -0,0 +1,369 @@ +package sessions + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" + "time" +) + +// MinimumPruneAge is the shortest cutoff Prune accepts. +// +// A floor under the lease, not a substitute for it. An older Zero, or anything +// else that writes a session without taking the lease, is invisible to prune, +// and a day is long enough that a session such a writer is actively using has +// been written since. +const MinimumPruneAge = 24 * time.Hour + +// PruneOptions chooses what Prune removes. +type PruneOptions struct { + // OlderThan makes a session a candidate when it was last updated at least + // this long ago. It must be at least MinimumPruneAge. + OlderThan time.Duration + // DryRun decides everything a real run would and removes nothing. It takes + // the same locks to decide, so a Zero opening a session at the moment it is + // being checked is told to try again, as it would be during a real run. + DryRun bool +} + +// PruneEntry is one session Prune removed (or in a dry run would remove), kept +// although it was old enough, or failed to remove. +type PruneEntry struct { + SessionID string `json:"sessionId"` + Title string `json:"title,omitempty"` + Kind SessionKind `json:"kind,omitempty"` + UpdatedAt string `json:"updatedAt"` + Bytes int64 `json:"bytes"` + Reason string `json:"reason,omitempty"` +} + +// PruneReport says what Prune did, or in a dry run what it would do. Sessions +// newer than the cutoff are not listed. +type PruneReport struct { + Cutoff string `json:"cutoff"` + DryRun bool `json:"dryRun"` + // Removed are the sessions removed, or in a dry run the ones a real run + // would remove. + Removed []PruneEntry `json:"removed"` + // Kept are sessions Prune left for a reason other than being recent. + Kept []PruneEntry `json:"kept"` + // Failed are sessions whose removal went wrong, or in a dry run could not be + // checked; Reason says how. A failure after the metadata was removed leaves + // a directory List no longer shows. + Failed []PruneEntry `json:"failed"` +} + +// The reasons Prune keeps a session that is old enough to remove. +const ( + PruneKeptOpen = "open in another Zero process" + PruneKeptParent = "parent of a session that is kept" + PruneKeptUpdated = "updated while pruning" + PruneKeptUndated = "its last update time could not be read" +) + +// pruneRemoveSeam runs after Prune holds a session's lease and write lock and +// before it re-reads the metadata. Nil in production. +var pruneRemoveSeam func(sessionID string) + +// pruneRemoveDirSeam runs just before Prune removes a session's directory, and +// says whether Prune still holds the lease. Nil in production. +var pruneRemoveDirSeam func(sessionID string, leaseHeld bool) + +// prunePlannedSeam runs once Prune has made its plan and before it removes +// anything. Nil in production. +var prunePlannedSeam func() + +// Prune removes sessions last updated before the cutoff that OlderThan sets. +// +// Only on request: nothing in Zero calls it by itself (#971). It never removes: +// - a session another process has open, which holds its lease (see Hold); +// - a session with a descendant that is kept, including one created after the +// plan was made, because Lineage and Tree fail on a missing ancestor; +// - a session written between the plan and its removal; +// - a session whose last update time cannot be read. +// +// Descendants are removed before their ancestors, so a descendant that turns +// out to be open part way through still keeps every ancestor above it. Within a +// session the metadata goes first, under both the lease and the write lock: +// from then on List and Get do not show it, so a removal that fails part way +// leaves nothing that looks like a session. +func (store *Store) Prune(options PruneOptions) (PruneReport, error) { + if options.OlderThan < MinimumPruneAge { + return PruneReport{}, fmt.Errorf("prune cutoff %s is shorter than the minimum of %s", options.OlderThan, MinimumPruneAge) + } + cutoff := store.now().UTC().Add(-options.OlderThan) + report := PruneReport{ + Cutoff: cutoff.Format(time.RFC3339), + DryRun: options.DryRun, + Removed: []PruneEntry{}, + Kept: []PruneEntry{}, + Failed: []PruneEntry{}, + } + all, err := store.List() + if err != nil { + return report, err + } + byID := make(map[string]Metadata, len(all)) + for _, session := range all { + byID[session.SessionID] = session + } + + candidates := map[string]bool{} + for _, session := range all { + updated, err := time.Parse(time.RFC3339, session.UpdatedAt) + if err != nil { + report.Kept = append(report.Kept, store.pruneEntry(session, PruneKeptUndated)) + continue + } + if !updated.Before(cutoff) { + continue + } + // Open elsewhere at planning time. Checked again under the lease when the + // session is removed; this pass is what lets its ancestors be kept too. + release, locked, err := store.HoldExclusive(session.SessionID) + if err != nil { + report.Kept = append(report.Kept, store.pruneEntry(session, "its lease could not be checked: "+err.Error())) + continue + } + if !locked { + report.Kept = append(report.Kept, store.pruneEntry(session, PruneKeptOpen)) + continue + } + release() + candidates[session.SessionID] = true + } + + // Every ancestor of a session that stays, stays. + for _, session := range all { + if candidates[session.SessionID] { + continue + } + for _, ancestor := range pruneAncestors(byID, session.SessionID) { + if candidates[ancestor] { + delete(candidates, ancestor) + report.Kept = append(report.Kept, store.pruneEntry(byID[ancestor], PruneKeptParent)) + } + } + } + + order := make([]Metadata, 0, len(candidates)) + for id := range candidates { + order = append(order, byID[id]) + } + depth := func(id string) int { return len(pruneAncestors(byID, id)) } + sort.Slice(order, func(left, right int) bool { + leftDepth, rightDepth := depth(order[left].SessionID), depth(order[right].SessionID) + if leftDepth != rightDepth { + return leftDepth > rightDepth + } + if order[left].UpdatedAt != order[right].UpdatedAt { + return order[left].UpdatedAt < order[right].UpdatedAt + } + return order[left].SessionID < order[right].SessionID + }) + if prunePlannedSeam != nil { + prunePlannedSeam() + } + + keep := map[string]bool{} + for _, session := range order { + if keep[session.SessionID] { + report.Kept = append(report.Kept, store.pruneEntry(session, PruneKeptParent)) + continue + } + entry := store.pruneEntry(session, "") + removed, keptReason, err := store.pruneSession(session.SessionID, cutoff, byID, options.DryRun) + switch { + case err != nil: + entry.Reason = err.Error() + report.Failed = append(report.Failed, entry) + case keptReason != "": + entry.Reason = keptReason + report.Kept = append(report.Kept, entry) + case removed: + report.Removed = append(report.Removed, entry) + } + if !removed { + for _, ancestor := range pruneAncestors(byID, session.SessionID) { + keep[ancestor] = true + } + } + } + return report, nil +} + +// pruneAncestors lists a session's ancestors, nearest first, stopping at a +// missing one or a cycle. +func pruneAncestors(byID map[string]Metadata, sessionID string) []string { + var ancestors []string + seen := map[string]bool{sessionID: true} + current, ok := byID[sessionID] + for ok && current.ParentSessionID != "" && !seen[current.ParentSessionID] { + parentID := current.ParentSessionID + seen[parentID] = true + ancestors = append(ancestors, parentID) + current, ok = byID[parentID] + } + return ancestors +} + +func (store *Store) pruneEntry(session Metadata, reason string) PruneEntry { + return PruneEntry{ + SessionID: session.SessionID, + Title: session.Title, + Kind: session.SessionKind, + UpdatedAt: session.UpdatedAt, + Bytes: store.sessionBytes(session.SessionID), + Reason: reason, + } +} + +// sessionBytes is what a session occupies on disk, without following links. +func (store *Store) sessionBytes(sessionID string) int64 { + var total int64 + _ = filepath.WalkDir(store.sessionPath(sessionID), func(_ string, entry fs.DirEntry, err error) error { + if err != nil || entry.IsDir() { + return nil + } + if info, err := entry.Info(); err == nil && info.Mode().IsRegular() { + total += info.Size() + } + return nil + }) + return total +} + +// childCreatedAfterPlan reports whether a session the plan did not see names +// sessionID as its parent. Only sessions missing from planned are read, so the +// cost is one directory listing plus whatever was created since the plan. +func (store *Store) childCreatedAfterPlan(sessionID string, planned map[string]Metadata) (bool, error) { + entries, err := os.ReadDir(store.RootDir) + if err != nil { + return false, err + } + for _, entry := range entries { + id := entry.Name() + if !entry.IsDir() || id == sessionID { + continue + } + if _, seen := planned[id]; seen { + continue + } + child, err := store.readMetadata(id) + if err != nil { + continue // not a session, or one without its metadata + } + if child.ParentSessionID == sessionID { + return true, nil + } + } + return false, nil +} + +// pruneSession removes one session that planning chose. It reports removed +// once the metadata is gone, even when leftovers could not be deleted (err says +// what), and a keptReason when the session turned out to be open, was written +// since the plan, or has a child the plan did not know about. planned is every +// session the plan saw. A dry run makes every one of those checks under the +// same locks and stops before removing anything; removed then says a real run +// would have gone on to remove the session. +func (store *Store) pruneSession(sessionID string, cutoff time.Time, planned map[string]Metadata, dryRun bool) (removed bool, keptReason string, err error) { + releaseLease, locked, err := store.HoldExclusive(sessionID) + if err != nil { + return false, "", fmt.Errorf("check the session's lease: %w", err) + } + if !locked { + return false, PruneKeptOpen, nil + } + leaseHeld := true + letLeaseGo := func() { + leaseHeld = false + releaseLease() + } + defer letLeaseGo() + release, err := store.lockSessionWithoutLease(sessionID) + if err != nil { + return false, "", err + } + released := false + defer func() { + if !released { + release() + } + }() + if pruneRemoveSeam != nil { + pruneRemoveSeam(sessionID) + } + + session, err := store.readMetadata(sessionID) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return false, "", fmt.Errorf("the session disappeared while pruning") + } + return false, "", err + } + updated, err := time.Parse(time.RFC3339, session.UpdatedAt) + if err != nil { + return false, PruneKeptUndated, nil + } + if !updated.Before(cutoff) { + return false, PruneKeptUpdated, nil + } + // A session forked or given a child after the plan was made, by a process that + // has exited since, is not in the plan, and nothing else would stop its parent + // going. Every way of creating a session under a parent holds that parent + // first (holdParent), so none can start while this lease is held exclusively, + // and one that finished has its metadata on disk. + child, err := store.childCreatedAfterPlan(sessionID, planned) + if err != nil { + return false, "", fmt.Errorf("look for sessions created under it: %w", err) + } + if child { + return false, PruneKeptParent, nil + } + if dryRun { + return true, "", nil + } + + // THE METADATA FIRST. From here the session no longer exists to List or Get. + if err := os.Remove(store.metadataPath(sessionID)); err != nil { + return false, "", fmt.Errorf("remove the session metadata: %w", err) + } + dir := store.sessionPath(sessionID) + entries, err := os.ReadDir(dir) + if err != nil { + return true, "", fmt.Errorf("list what is left of the session: %w", err) + } + for _, entry := range entries { + // session.lock is held right here; it goes after the release below. The + // lease file is held too, but it was opened for deletion (openLeaseFile). + if entry.Name() == filepath.Base(store.lockPath(sessionID)) { + continue + } + if err := os.RemoveAll(filepath.Join(dir, entry.Name())); err != nil { + return true, "", fmt.Errorf("remove %s: %w", entry.Name(), err) + } + } + release() + released = true + // A writer that takes session.lock in the gap only keeps an empty directory + // alive, and its own write then fails: the metadata is already gone. + if err := os.Remove(store.lockPath(sessionID)); err != nil && !errors.Is(err, fs.ErrNotExist) { + return true, "", fmt.Errorf("remove the session lock: %w", err) + } + // Let go of the lease before the directory. lease.lock went with the rest, but + // where a delete only takes effect once the last handle closes (Windows without + // POSIX delete semantics: older builds, or FAT and exFAT volumes), this handle + // would keep the directory from being empty. + letLeaseGo() + if pruneRemoveDirSeam != nil { + pruneRemoveDirSeam(sessionID, leaseHeld) + } + if err := os.Remove(dir); err != nil && !errors.Is(err, fs.ErrNotExist) { + return true, "", fmt.Errorf("remove the session directory: %w", err) + } + return true, "", nil +} diff --git a/internal/sessions/prune_race_test.go b/internal/sessions/prune_race_test.go new file mode 100644 index 000000000..0e80d4861 --- /dev/null +++ b/internal/sessions/prune_race_test.go @@ -0,0 +1,202 @@ +package sessions + +import ( + "errors" + "fmt" + "os" + "strings" + "testing" +) + +// A fork made after prune's plan, by a process that has exited since, is not in +// the plan and holds nothing. It still keeps its parent: once prune holds that +// parent exclusively it looks for children the plan did not know about. +func TestPruneKeepsAParentForkedAfterThePlan(t *testing.T) { + root := t.TempDir() + createFinishedSession(t, root, "parent", "2026-06-01T00:00:00Z", "") + + forked := false + prunePlannedSeam = func() { + other := NewStore(StoreOptions{RootDir: root, Now: fixedClock("2026-09-25T00:00:00Z")}) + if _, err := other.Fork("parent", ForkInput{SessionID: "late"}); err != nil { + t.Errorf("fork after the plan: %v", err) + return + } + forked = true + // The forking process exits, and its leases go with it. + other.Release("late") + other.Release("parent") + } + defer func() { prunePlannedSeam = nil }() + + report, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays}) + if err != nil { + t.Fatalf("Prune: %v", err) + } + if !forked { + t.Fatal("SETUP INVALID: the fork after the plan never happened") + } + if reason := keptReason(report, "parent"); reason != PruneKeptParent { + t.Errorf("parent kept for %q, want %q (removed %v)", reason, PruneKeptParent, pruneIDs(report.Removed)) + } + if lineage, err := pruneStore(root).Lineage("late"); err != nil || len(lineage) != 2 { + t.Fatalf("the late fork's lineage is broken: %d entries, %v", len(lineage), err) + } +} + +// A dry run decides what a real run would, including what only shows once a +// session is held for removal: a fork made after the plan, a write since the +// plan, and a process that opened the session since the plan. Each case runs +// both ways from the same start, and both reports must be the one wanted. +func TestPruneDryRunDecidesLikeARealRunAtRemoval(t *testing.T) { + cases := []struct { + name string + // create lays out the sessions. afterPlan changes them once the plan is + // made, and returns what to undo when Prune has finished. + create func(t *testing.T, root string) + afterPlan func(t *testing.T, root string) (undo func()) + want string + }{ + { + name: "a fork made after the plan", + create: func(t *testing.T, root string) { + createFinishedSession(t, root, "parent", "2026-06-01T00:00:00Z", "") + }, + afterPlan: func(t *testing.T, root string) func() { + other := NewStore(StoreOptions{RootDir: root, Now: fixedClock("2026-09-25T00:00:00Z")}) + if _, err := other.Fork("parent", ForkInput{SessionID: "late"}); err != nil { + t.Errorf("fork after the plan: %v", err) + } + other.Release("late") + other.Release("parent") + return func() {} + }, + want: "removed [] kept [parent: " + PruneKeptParent + "] failed []", + }, + { + name: "a write since the plan", + create: func(t *testing.T, root string) { + createFinishedSession(t, root, "parent", "2026-06-01T00:00:00Z", "") + createFinishedSession(t, root, "child", "2026-06-15T00:00:00Z", "parent") + }, + afterPlan: func(t *testing.T, root string) func() { + rewriteUpdatedAt(t, root, "child", "2026-09-25T23:00:00Z") + return func() {} + }, + want: "removed [] kept [child: " + PruneKeptUpdated + ", parent: " + PruneKeptParent + "] failed []", + }, + { + name: "a session opened since the plan", + create: func(t *testing.T, root string) { + createFinishedSession(t, root, "opened", "2026-06-01T00:00:00Z", "") + createFinishedSession(t, root, "idle", "2026-06-02T00:00:00Z", "") + }, + afterPlan: func(t *testing.T, root string) func() { + other := NewStore(StoreOptions{RootDir: root}) + if busy := other.hold("opened"); busy { + t.Error("SETUP INVALID: the session was busy when opened after the plan") + } + return func() { other.Release("opened") } + }, + want: "removed [idle] kept [opened: " + PruneKeptOpen + "] failed []", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + defer func() { prunePlannedSeam = nil }() + for _, dryRun := range []bool{false, true} { + root := t.TempDir() + tc.create(t, root) + before := sessionDirNames(t, root) + var undo func() + prunePlannedSeam = func() { undo = tc.afterPlan(t, root) } + report, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays, DryRun: dryRun}) + prunePlannedSeam = nil + if undo == nil { + t.Fatal("SETUP INVALID: nothing changed after the plan") + } + undo() + if err != nil { + t.Fatalf("dry run %v: Prune: %v", dryRun, err) + } + if got := pruneSummary(report); got != tc.want { + t.Errorf("dry run %v: the report is %s, want %s", dryRun, got, tc.want) + } + if !dryRun { + continue + } + for _, id := range before { + if _, err := os.Stat(pruneStore(root).metadataPath(id)); err != nil { + t.Errorf("the dry run removed the metadata of %s: %v", id, err) + } + } + } + }) + } +} + +// pruneSummary puts what a report decided on one line, in report order. +func pruneSummary(report PruneReport) string { + kept := []string{} + for _, entry := range report.Kept { + kept = append(kept, entry.SessionID+": "+entry.Reason) + } + return fmt.Sprintf("removed %v kept [%s] failed %v", pruneIDs(report.Removed), strings.Join(kept, ", "), pruneIDs(report.Failed)) +} + +// Prune removes the metadata first and unlinks lease.lock after it. A process +// that picked the session a moment earlier and only now takes its lease creates +// a fresh lease.lock in the directory prune is emptying, and locks it with +// nothing to contend with. Continuing the session, or creating one under it, is +// refused all the same, and prune still removes the directory. +func TestContinuingASessionPruneIsRemovingIsRefused(t *testing.T) { + root := t.TempDir() + createFinishedSession(t, root, "old", "2026-06-01T00:00:00Z", "") + other := NewStore(StoreOptions{RootDir: root}) + + reached := false + pruneRemoveDirSeam = func(id string, _ bool) { + if id != "old" { + return + } + reached = true + if _, err := os.Stat(other.metadataPath("old")); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("SETUP INVALID: the metadata is still there at the seam: %v", err) + } + if err := other.HoldToContinue("old"); !errors.Is(err, ErrPruning) || !strings.Contains(err.Error(), "was removed") { + t.Errorf("continue a session prune is removing: err = %v, want it refused as removed", err) + } + if events, err := readExecContextEvents(other, "old"); !errors.Is(err, ErrPruning) || events != nil { + t.Errorf("exec context read of a session prune is removing: %d events, err = %v, want it refused", len(events), err) + } + if _, err := other.Create(CreateInput{SessionID: "child", ParentSessionID: "old"}); !errors.Is(err, ErrPruning) { + t.Errorf("create a session under one prune is removing: err = %v, want it refused", err) + } + } + defer func() { pruneRemoveDirSeam = nil }() + + report, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays}) + if err != nil { + t.Fatalf("Prune: %v", err) + } + if !reached { + t.Fatal("SETUP INVALID: prune never reached the directory removal") + } + if strings.Join(pruneIDs(report.Removed), ",") != "old" || len(report.Failed) != 0 { + t.Errorf("removed %v, failed %v: the refused continuation must not keep prune from removing the directory", pruneIDs(report.Removed), pruneIDs(report.Failed)) + } + if sessionDirExists(t, root, "old") || sessionDirExists(t, root, "child") { + t.Errorf("left behind: old=%v child=%v", sessionDirExists(t, root, "old"), sessionDirExists(t, root, "child")) + } + + // Once prune has finished, the session the caller picked is simply gone, and + // continuing it is refused the same way. + if err := other.HoldToContinue("old"); !errors.Is(err, ErrPruning) { + t.Errorf("continue a session prune removed: err = %v, want it refused", err) + } + // A caller that picked nothing still reads a missing session as empty. + events, present, err := other.ReadRehydratedEventsWithPresence("never-existed") + if err != nil || present || len(events) != 0 { + t.Errorf("rehydrated read of a session that never existed = %d events, present=%v, err=%v; want empty", len(events), present, err) + } +} diff --git a/internal/sessions/prune_test.go b/internal/sessions/prune_test.go new file mode 100644 index 000000000..ce452e24b --- /dev/null +++ b/internal/sessions/prune_test.go @@ -0,0 +1,520 @@ +package sessions + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// pruneNow is "now" for every prune in these tests. +const pruneNow = "2026-09-26T00:00:00Z" + +const thirtyDays = 30 * 24 * time.Hour + +func pruneStore(root string) *Store { + return NewStore(StoreOptions{RootDir: root, Now: fixedClock(pruneNow)}) +} + +// createFinishedSession leaves a session the way a run that has ended leaves +// it: created and written at `at` by a Store that no longer holds it open. +func createFinishedSession(t *testing.T, root, id, at, parent string) { + t.Helper() + store := NewStore(StoreOptions{RootDir: root, Now: fixedClock(at)}) + var err error + if parent == "" { + _, err = store.Create(CreateInput{SessionID: id, Title: "title " + id}) + } else { + _, err = store.Fork(parent, ForkInput{SessionID: id, Title: "title " + id}) + } + if err != nil { + t.Fatalf("create %s: %v", id, err) + } + if _, err := store.AppendEvent(id, AppendEventInput{Type: EventMessage, Payload: map[string]string{"content": "hello from " + id}}); err != nil { + t.Fatalf("append to %s: %v", id, err) + } + store.Release(id) + if parent != "" { + store.Release(parent) + } +} + +func sessionDirExists(t *testing.T, root, id string) bool { + t.Helper() + _, err := os.Stat(filepath.Join(root, id)) + if err == nil { + return true + } + if errors.Is(err, os.ErrNotExist) { + return false + } + t.Fatalf("stat %s: %v", id, err) + return false +} + +func pruneIDs(entries []PruneEntry) []string { + ids := []string{} + for _, entry := range entries { + ids = append(ids, entry.SessionID) + } + return ids +} + +func keptReason(report PruneReport, id string) string { + for _, entry := range report.Kept { + if entry.SessionID == id { + return entry.Reason + } + } + return "" +} + +// rewriteUpdatedAt changes a session's recorded last update behind the store's +// back, the way a writer that takes no lease would. +func rewriteUpdatedAt(t *testing.T, root, id, updatedAt string) { + t.Helper() + path := filepath.Join(root, id, "metadata.json") + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s metadata: %v", id, err) + } + var fields map[string]any + if err := json.Unmarshal(data, &fields); err != nil { + t.Fatalf("decode %s metadata: %v", id, err) + } + fields["updatedAt"] = updatedAt + data, err = json.Marshal(fields) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, 0o600); err != nil { + t.Fatalf("write %s metadata: %v", id, err) + } +} + +func TestPruneRemovesOnlySessionsOlderThanTheCutoff(t *testing.T) { + root := t.TempDir() + createFinishedSession(t, root, "old-a", "2026-07-01T00:00:00Z", "") + createFinishedSession(t, root, "old-b", "2026-07-15T00:00:00Z", "") + createFinishedSession(t, root, "recent", "2026-09-25T12:00:00Z", "") + + // A checkpoint blob, which has to go with its session. + workspace := t.TempDir() + if err := os.WriteFile(filepath.Join(workspace, "a.txt"), []byte("before"), 0o644); err != nil { + t.Fatal(err) + } + old := NewStore(StoreOptions{RootDir: root, Now: fixedClock("2026-07-01T00:00:00Z")}) + if _, err := old.CaptureToolCheckpoint("old-a", workspace, "edit_file", []string{"a.txt"}); err != nil { + t.Fatalf("capture a checkpoint: %v", err) + } + old.Release("old-a") + if blobs, err := os.ReadDir(filepath.Join(root, "old-a", CheckpointsDir, "blobs")); err != nil || len(blobs) == 0 { + t.Fatalf("SETUP INVALID: no checkpoint blob to remove (%v)", err) + } + + report, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays}) + if err != nil { + t.Fatalf("Prune: %v", err) + } + if got := strings.Join(pruneIDs(report.Removed), ","); got != "old-a,old-b" { + t.Fatalf("removed %q, want old-a,old-b", got) + } + if len(report.Kept) != 0 || len(report.Failed) != 0 { + t.Fatalf("kept %v and failed %v, want neither", report.Kept, report.Failed) + } + for _, entry := range report.Removed { + if entry.Bytes <= 0 { + t.Errorf("%s was reported at %d bytes", entry.SessionID, entry.Bytes) + } + if sessionDirExists(t, root, entry.SessionID) { + t.Errorf("%s was reported removed and its directory is still there", entry.SessionID) + } + } + if got, err := pruneStore(root).Get("recent"); err != nil || got == nil { + t.Fatalf("the recent session is gone or unreadable: %v %v", got, err) + } +} + +func TestPruneRefusesACutoffShorterThanADay(t *testing.T) { + root := t.TempDir() + createFinishedSession(t, root, "old", "2026-07-01T00:00:00Z", "") + _, err := pruneStore(root).Prune(PruneOptions{OlderThan: 23 * time.Hour}) + if err == nil || !strings.Contains(err.Error(), "minimum") { + t.Fatalf("a 23h cutoff was not refused for being under the minimum: %v", err) + } + if !sessionDirExists(t, root, "old") { + t.Fatal("a refused prune removed a session") + } +} + +func TestPruneDryRunDecidesTheSameAndRemovesNothing(t *testing.T) { + root := t.TempDir() + createFinishedSession(t, root, "old-a", "2026-07-01T00:00:00Z", "") + createFinishedSession(t, root, "old-b", "2026-07-15T00:00:00Z", "") + + report, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays, DryRun: true}) + if err != nil { + t.Fatalf("Prune: %v", err) + } + if !report.DryRun || strings.Join(pruneIDs(report.Removed), ",") != "old-a,old-b" { + t.Fatalf("dry run report = %+v, want both sessions listed as would-be removals", report) + } + for _, id := range []string{"old-a", "old-b"} { + if !sessionDirExists(t, root, id) { + t.Errorf("a dry run removed %s", id) + } + } +} + +// A SESSION ANOTHER PROCESS HAS OPEN STAYS, HOWEVER OLD. The other Store stands +// in for the other process: lease locks conflict between handles, not only +// between processes. Each way of having a session open is covered: holding it +// outright, writing to it, and loading it to resume it. +func TestPruneLeavesASessionAnotherProcessHasOpen(t *testing.T) { + for _, open := range []struct { + name string + do func(t *testing.T, other *Store, id string) + }{ + {"held", func(t *testing.T, other *Store, id string) { other.Hold(id) }}, + {"written", func(t *testing.T, other *Store, id string) { + if _, err := other.AppendEvent(id, AppendEventInput{Type: EventMessage, Payload: map[string]string{"content": "still here"}}); err != nil { + t.Fatalf("append: %v", err) + } + }}, + {"resumed", func(t *testing.T, other *Store, id string) { + if _, err := other.ReadRehydratedEvents(id); err != nil { + t.Fatalf("resume: %v", err) + } + }}, + } { + t.Run(open.name, func(t *testing.T) { + root := t.TempDir() + createFinishedSession(t, root, "old", "2026-07-01T00:00:00Z", "") + // The other process's clock is as old as the session, so a write + // leaves it just as old and only the lease can keep it. + other := NewStore(StoreOptions{RootDir: root, Now: fixedClock("2026-07-01T00:00:00Z")}) + open.do(t, other, "old") + + // A dry run has to say so too, not list it as removable. + preview, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays, DryRun: true}) + if err != nil { + t.Fatalf("dry run: %v", err) + } + if reason := keptReason(preview, "old"); reason != PruneKeptOpen || len(preview.Removed) != 0 { + t.Fatalf("dry run kept reason %q and would remove %v, want it kept as open", reason, pruneIDs(preview.Removed)) + } + + report, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays}) + if err != nil { + t.Fatalf("Prune: %v", err) + } + if reason := keptReason(report, "old"); reason != PruneKeptOpen { + t.Fatalf("kept reason %q, want %q (report %+v)", reason, PruneKeptOpen, report) + } + if !sessionDirExists(t, root, "old") { + t.Fatal("a session another process had open was removed") + } + + // The lease was the only thing keeping it. + other.Release("old") + report, err = pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays}) + if err != nil { + t.Fatalf("Prune after release: %v", err) + } + if strings.Join(pruneIDs(report.Removed), ",") != "old" { + t.Fatalf("after the other process let go, removed %v", pruneIDs(report.Removed)) + } + }) + } +} + +// Lineage and Tree fail on a missing ancestor, so an ancestor of anything kept +// stays, however old. +func TestPruneKeepsEveryAncestorOfAKeptSession(t *testing.T) { + root := t.TempDir() + createFinishedSession(t, root, "grandparent", "2026-06-01T00:00:00Z", "") + createFinishedSession(t, root, "parent", "2026-06-15T00:00:00Z", "grandparent") + createFinishedSession(t, root, "child", "2026-09-25T00:00:00Z", "parent") + createFinishedSession(t, root, "unrelated", "2026-06-01T00:00:00Z", "") + + report, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays}) + if err != nil { + t.Fatalf("Prune: %v", err) + } + if got := strings.Join(pruneIDs(report.Removed), ","); got != "unrelated" { + t.Fatalf("removed %q, want only the unrelated session", got) + } + for _, id := range []string{"parent", "grandparent"} { + if reason := keptReason(report, id); reason != PruneKeptParent { + t.Errorf("%s kept for %q, want %q", id, reason, PruneKeptParent) + } + } + if lineage, err := pruneStore(root).Lineage("child"); err != nil || len(lineage) != 3 { + t.Fatalf("the kept child's lineage is broken: %d entries, %v", len(lineage), err) + } +} + +// Descendants go first, and one that turns out to have been written since the +// plan keeps its ancestors, which are old enough themselves. +func TestPruneKeepsTheAncestorsOfASessionWrittenWhilePruning(t *testing.T) { + root := t.TempDir() + createFinishedSession(t, root, "parent", "2026-06-01T00:00:00Z", "") + createFinishedSession(t, root, "child", "2026-06-15T00:00:00Z", "parent") + + var seen []string + pruneRemoveSeam = func(id string) { + seen = append(seen, id) + if id == "child" { + rewriteUpdatedAt(t, root, "child", "2026-09-25T23:00:00Z") + } + } + defer func() { pruneRemoveSeam = nil }() + + report, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays}) + if err != nil { + t.Fatalf("Prune: %v", err) + } + if strings.Join(seen, ",") != "child" { + t.Errorf("removal reached %v, want the child first and the parent never", seen) + } + if reason := keptReason(report, "child"); reason != PruneKeptUpdated { + t.Errorf("child kept for %q, want %q", reason, PruneKeptUpdated) + } + if reason := keptReason(report, "parent"); reason != PruneKeptParent { + t.Errorf("parent kept for %q, want %q", reason, PruneKeptParent) + } + if len(report.Removed) != 0 || !sessionDirExists(t, root, "parent") || !sessionDirExists(t, root, "child") { + t.Fatalf("something was removed: %+v", report) + } +} + +func TestPruneKeepsASessionWhoseLastUpdateCannotBeRead(t *testing.T) { + root := t.TempDir() + createFinishedSession(t, root, "undated", "2026-06-01T00:00:00Z", "") + rewriteUpdatedAt(t, root, "undated", "sometime last spring") + + preview, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays, DryRun: true}) + if err != nil { + t.Fatalf("dry run: %v", err) + } + if reason := keptReason(preview, "undated"); reason != PruneKeptUndated || len(preview.Removed) != 0 { + t.Fatalf("dry run kept reason %q and would remove %v", reason, pruneIDs(preview.Removed)) + } + + report, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays}) + if err != nil { + t.Fatalf("Prune: %v", err) + } + if reason := keptReason(report, "undated"); reason != PruneKeptUndated { + t.Fatalf("kept for %q, want %q", reason, PruneKeptUndated) + } + if !sessionDirExists(t, root, "undated") { + t.Fatal("a session with an unreadable update time was removed") + } +} + +// A session another process has just created, and not yet written to, is as +// open as one it has been writing for hours: a TUI creates its session before +// the first prompt. +func TestPruneLeavesASessionAnotherProcessJustCreated(t *testing.T) { + root := t.TempDir() + other := NewStore(StoreOptions{RootDir: root, Now: fixedClock("2026-07-01T00:00:00Z")}) + if _, err := other.Create(CreateInput{SessionID: "fresh", Title: "not written yet"}); err != nil { + t.Fatalf("create: %v", err) + } + defer other.Release("fresh") + + report, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays}) + if err != nil { + t.Fatalf("Prune: %v", err) + } + if reason := keptReason(report, "fresh"); reason != PruneKeptOpen { + t.Fatalf("kept reason %q, want %q (report %+v)", reason, PruneKeptOpen, report) + } + if !sessionDirExists(t, root, "fresh") { + t.Fatal("a session another process had just created was removed") + } +} + +// sessionDirNames lists the session directories under root. +func sessionDirNames(t *testing.T, root string) []string { + t.Helper() + entries, err := os.ReadDir(root) + if err != nil { + t.Fatal(err) + } + names := []string{} + for _, entry := range entries { + if entry.IsDir() { + names = append(names, entry.Name()) + } + } + return names +} + +// Every way of creating a session under a parent holds that parent, and is +// refused while prune holds it, rather than creating a session under one that is +// about to go: Fork, CreateChild, and the direct Create that exec +// --calling-session-id and spec implementations use. +func TestForkAndChildHoldTheParentTheyAreCreatedFrom(t *testing.T) { + for _, create := range []struct { + name string + do func(store *Store) error + }{ + {"fork", func(store *Store) error { + _, err := store.Fork("parent", ForkInput{SessionID: "new"}) + return err + }}, + {"child", func(store *Store) error { + _, err := store.CreateChild("parent", ChildInput{SessionID: "new"}) + return err + }}, + {"create with a parent", func(store *Store) error { + _, err := store.Create(CreateInput{SessionID: "new", ParentSessionID: "parent"}) + return err + }}, + {"exec calling session", func(store *Store) error { + _, err := PrepareExec(PrepareExecOptions{Store: store, SessionID: "new", CallingSessionID: "parent"}) + return err + }}, + {"spec implementation", func(store *Store) error { + _, _, err := store.EnsureSpecImplementation(EnsureSpecImplementationInput{SpecID: "spec", SpecSourceSessionID: "parent", Prompt: "build it"}) + return err + }}, + } { + t.Run(create.name, func(t *testing.T) { + root := t.TempDir() + createFinishedSession(t, root, "parent", "2026-06-01T00:00:00Z", "") + other := NewStore(StoreOptions{RootDir: root, Now: fixedClock("2026-06-01T00:00:00Z")}) + before := strings.Join(sessionDirNames(t, root), ",") + + release, locked, err := pruneStore(root).HoldExclusive("parent") + if err != nil || !locked { + t.Fatalf("take the parent's lease the way prune does: locked=%v, %v", locked, err) + } + err = create.do(other) + release() + if err == nil || !strings.Contains(err.Error(), "locked by zero sessions prune") { + t.Errorf("%s from a parent prune holds: err = %v, want it refused", create.name, err) + } + if after := strings.Join(sessionDirNames(t, root), ","); after != before { + t.Errorf("the refused %s created a session anyway: %s, was %s", create.name, after, before) + } + + if err := create.do(other); err != nil { + t.Fatalf("%s once prune let go: %v", create.name, err) + } + report, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays, DryRun: true}) + if err != nil { + t.Fatalf("dry run: %v", err) + } + if reason := keptReason(report, "parent"); reason != PruneKeptOpen { + t.Errorf("parent kept for %q, want %q: the %s's process holds it", reason, PruneKeptOpen, create.name) + } + }) + } +} + +// Where a delete only takes effect once the last handle closes, the lease Prune +// holds would keep the session directory from being empty, so it lets go first. +func TestPruneLetsGoOfTheLeaseBeforeRemovingTheDirectory(t *testing.T) { + root := t.TempDir() + createFinishedSession(t, root, "old", "2026-06-01T00:00:00Z", "") + + var reached []string + pruneRemoveDirSeam = func(id string, leaseHeld bool) { + reached = append(reached, id) + if leaseHeld { + t.Errorf("%s: the directory is removed while prune still holds the lease", id) + } + } + defer func() { pruneRemoveDirSeam = nil }() + + report, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays}) + if err != nil { + t.Fatalf("Prune: %v", err) + } + if strings.Join(reached, ",") != "old" || strings.Join(pruneIDs(report.Removed), ",") != "old" || sessionDirExists(t, root, "old") { + t.Fatalf("reached %v and removed %v, want old reached and removed", reached, pruneIDs(report.Removed)) + } +} + +// Loading a session to continue it is refused while prune holds it, instead of +// read without the lease, and the exec path does not fall back to the raw log on +// that refusal. +func TestResumeIsRefusedWhilePruneHoldsTheSession(t *testing.T) { + root := t.TempDir() + createFinishedSession(t, root, "old", "2026-06-01T00:00:00Z", "") + other := NewStore(StoreOptions{RootDir: root}) + + release, locked, err := pruneStore(root).HoldExclusive("old") + if err != nil || !locked { + t.Fatalf("take the lease the way prune does: locked=%v, %v", locked, err) + } + _, _, readErr := other.ReadRehydratedEventsWithPresence("old") + events, execErr := readExecContextEvents(other, "old") + release() + if !errors.Is(readErr, ErrPruning) { + t.Errorf("rehydrated read while prune holds the session: err = %v, want ErrPruning", readErr) + } + if !errors.Is(execErr, ErrPruning) || events != nil { + t.Errorf("exec context read while prune holds the session: %d events, err = %v, want it refused, not read from the raw log", len(events), execErr) + } + + // Once prune lets go, the read succeeds and holds the session. + if _, err := other.ReadRehydratedEvents("old"); err != nil { + t.Fatalf("read once prune let go: %v", err) + } + report, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays, DryRun: true}) + if err != nil { + t.Fatalf("dry run: %v", err) + } + if reason := keptReason(report, "old"); reason != PruneKeptOpen { + t.Errorf("old kept for %q, want %q", reason, PruneKeptOpen) + } +} + +// A lease that cannot be taken at all is not a free one: prune keeps the +// session. That is why a process whose own hold fails the same way carries on +// with the session instead of being refused. Prune cannot remove it either, and +// refusing would stop resume and fork wherever locking does not work, for people +// who never prune. +func TestPruneKeepsASessionWhoseLeaseCannotBeChecked(t *testing.T) { + root := t.TempDir() + createFinishedSession(t, root, "old", "2026-06-01T00:00:00Z", "") + lease := filepath.Join(root, "old", leaseFileName) + if err := os.Remove(lease); err != nil && !errors.Is(err, os.ErrNotExist) { + t.Fatalf("remove the lease file: %v", err) + } + // A directory where the lease file goes cannot be opened as one. + if err := os.Mkdir(lease, 0o700); err != nil { + t.Fatalf("SETUP INVALID: %v", err) + } + if _, locked, err := pruneStore(root).HoldExclusive("old"); err == nil || locked { + t.Fatalf("SETUP INVALID: the lease could still be taken: locked=%v, %v", locked, err) + } + + for _, dryRun := range []bool{true, false} { + report, err := pruneStore(root).Prune(PruneOptions{OlderThan: thirtyDays, DryRun: dryRun}) + if err != nil { + t.Fatalf("Prune (dry run %v): %v", dryRun, err) + } + if reason := keptReason(report, "old"); !strings.HasPrefix(reason, "its lease could not be checked") || len(report.Removed) != 0 || len(report.Failed) != 0 { + t.Fatalf("dry run %v: kept for %q, removed %v, failed %v, want it kept because its lease could not be checked", dryRun, reason, pruneIDs(report.Removed), pruneIDs(report.Failed)) + } + } + if !sessionDirExists(t, root, "old") { + t.Fatal("a session whose lease could not be checked was removed") + } + + other := NewStore(StoreOptions{RootDir: root}) + if _, err := other.ReadRehydratedEvents("old"); err != nil { + t.Fatalf("resume a session whose lease cannot be taken: %v", err) + } + if _, err := other.Fork("old", ForkInput{SessionID: "fork"}); err != nil { + t.Fatalf("fork a session whose lease cannot be taken: %v", err) + } +} diff --git a/internal/sessions/replay.go b/internal/sessions/replay.go index d849e3f36..d17d8dd24 100644 --- a/internal/sessions/replay.go +++ b/internal/sessions/replay.go @@ -233,7 +233,16 @@ func (store *Store) ReadRehydratedEvents(sessionID string) ([]Event, error) { // ReadRehydratedEventsWithPresence carries the underlying event-log presence // through compaction projection without changing ReadRehydratedEvents' existing // empty-on-missing contract. +// +// It is how a session is loaded to be continued (the TUI's resume, `exec +// --resume` and --fork, ACP's session/load and session/resume), so it holds the +// session open, and it fails with ErrPruning while prune holds it. Callers that +// fall back to ReadEvents when rehydration fails must not fall back on that +// error. See holdOrRefuse. func (store *Store) ReadRehydratedEventsWithPresence(sessionID string) ([]Event, bool, error) { + if err := store.holdOrRefuse(sessionID); err != nil { + return nil, false, err + } events, present, err := store.ReadEventsWithPresence(sessionID) if err != nil { return nil, present, err diff --git a/internal/sessions/store.go b/internal/sessions/store.go index a811779b3..cab1faa5d 100644 --- a/internal/sessions/store.go +++ b/internal/sessions/store.go @@ -233,6 +233,10 @@ type Store struct { // is accepted deliberately rather than risk an unsafe eviction. sessionLocks map[string]*sync.Mutex idCounter atomic.Uint64 + + // leases are the sessions this Store holds open. See Hold. + leasesMu sync.Mutex + leases map[string]*os.File } var sessionIDPattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_-]{0,127}$`) @@ -279,6 +283,14 @@ func (store *Store) Create(input CreateInput) (Metadata, error) { if input.Depth < 0 { return Metadata{}, fmt.Errorf("invalid zero session depth %d", input.Depth) } + // A session created under a parent holds that parent, the way Fork and + // CreateChild do, and is refused while prune holds or is removing it: exec + // --calling-session-id and spec implementations create their children here. + if parent := strings.TrimSpace(input.ParentSessionID); parent != "" { + if err := store.holdParent(parent); err != nil { + return Metadata{}, err + } + } timestamp := store.timestamp() session := Metadata{ @@ -332,6 +344,7 @@ func (store *Store) Create(input CreateInput) (Metadata, error) { if err := file.Close(); err != nil { return Metadata{}, fmt.Errorf("close zero session events file: %w", err) } + store.Hold(sessionID) return session, nil } @@ -429,6 +442,9 @@ func (store *Store) Fork(parentSessionID string, input ForkInput) (Metadata, err if !ValidSessionID(parentSessionID) { return Metadata{}, fmt.Errorf("invalid zero session id %q", parentSessionID) } + if err := store.holdParent(parentSessionID); err != nil { + return Metadata{}, err + } parent, err := store.Get(parentSessionID) if err != nil { return Metadata{}, err @@ -897,6 +913,14 @@ func (store *Store) sessionLock(sessionID string) *sync.Mutex { // reverse order. The OS lock is best-effort: if it cannot be acquired (e.g. an // unsupported platform) the in-memory mutex still applies. func (store *Store) lockSession(sessionID string) (func(), error) { + // A process that writes a session has it open. See Hold. + store.Hold(sessionID) + return store.lockSessionWithoutLease(sessionID) +} + +// lockSessionWithoutLease is lockSession for prune, which must not mark as open +// the session it is about to remove. +func (store *Store) lockSessionWithoutLease(sessionID string) (func(), error) { mu := store.sessionLock(sessionID) mu.Lock() release, err := store.acquireFileLock(sessionID) diff --git a/internal/tui/resume_pruning_test.go b/internal/tui/resume_pruning_test.go new file mode 100644 index 000000000..fcfc60eb2 --- /dev/null +++ b/internal/tui/resume_pruning_test.go @@ -0,0 +1,70 @@ +package tui + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/Gitlawb/zero/internal/sessions" +) + +// A resume that finds zero sessions prune holding the session is refused, not +// read from the raw log without the lease. +func TestResumeEventsIsRefusedWhilePruneHoldsTheSession(t *testing.T) { + root := t.TempDir() + creator := sessions.NewStore(sessions.StoreOptions{RootDir: root}) + meta, err := creator.Create(sessions.CreateInput{Title: "resume me"}) + if err != nil { + t.Fatalf("create: %v", err) + } + if _, err := creator.AppendEvent(meta.SessionID, sessions.AppendEventInput{Type: sessions.EventMessage, Payload: map[string]string{"content": "hello"}}); err != nil { + t.Fatalf("append: %v", err) + } + creator.Release(meta.SessionID) + + release, locked, err := sessions.NewStore(sessions.StoreOptions{RootDir: root}).HoldExclusive(meta.SessionID) + if err != nil || !locked { + t.Fatalf("take the lease the way prune does: locked=%v, %v", locked, err) + } + defer release() + + m := model{sessionStore: sessions.NewStore(sessions.StoreOptions{RootDir: root})} + events, err := m.resumeEvents(meta.SessionID) + if !errors.Is(err, sessions.ErrPruning) || events != nil { + t.Fatalf("resume while prune holds the session: %d events, err = %v, want it refused", len(events), err) + } +} + +// A session picked for /resume that prune removes before it is read is refused +// too, not resumed as an empty conversation: part way through the removal, with +// the directory still there and its metadata gone, and once it is gone. +func TestResumeEventsIsRefusedOnceThePickedSessionIsRemoved(t *testing.T) { + root := t.TempDir() + creator := sessions.NewStore(sessions.StoreOptions{RootDir: root}) + meta, err := creator.Create(sessions.CreateInput{Title: "resume me"}) + if err != nil { + t.Fatalf("create: %v", err) + } + if _, err := creator.AppendEvent(meta.SessionID, sessions.AppendEventInput{Type: sessions.EventMessage, Payload: map[string]string{"content": "hello"}}); err != nil { + t.Fatalf("append: %v", err) + } + creator.Release(meta.SessionID) + dir := filepath.Join(root, meta.SessionID) + + if err := os.Remove(filepath.Join(dir, sessions.MetadataFile)); err != nil { + t.Fatal(err) + } + m := model{sessionStore: sessions.NewStore(sessions.StoreOptions{RootDir: root})} + if events, err := m.resumeEvents(meta.SessionID); !errors.Is(err, sessions.ErrPruning) || events != nil { + t.Errorf("resume while prune is removing the session: %d events, err = %v, want it refused", len(events), err) + } + + if err := os.RemoveAll(dir); err != nil { + t.Fatal(err) + } + m = model{sessionStore: sessions.NewStore(sessions.StoreOptions{RootDir: root})} + if events, err := m.resumeEvents(meta.SessionID); !errors.Is(err, sessions.ErrPruning) || events != nil { + t.Errorf("resume after prune removed the session: %d events, err = %v, want it refused", len(events), err) + } +} diff --git a/internal/tui/session.go b/internal/tui/session.go index a64e43c75..95a7c8319 100644 --- a/internal/tui/session.go +++ b/internal/tui/session.go @@ -310,10 +310,20 @@ func (m model) resolveResumeSession(args string) (*sessions.Metadata, error) { // the CLI's `zero exec --resume` (readExecContextEvents) and the in-TUI /compact // reload. Falls back to the raw log if rehydration fails. func (m model) resumeEvents(sessionID string) ([]sessions.Event, error) { + // Picked from its metadata a moment ago. Make sure it is still there, and + // held, before resuming it: see sessions.HoldToContinue. + if err := m.sessionStore.HoldToContinue(sessionID); err != nil { + return nil, err + } events, err := m.sessionStore.ReadRehydratedEvents(sessionID) if err == nil { return events, nil } + if errors.Is(err, sessions.ErrPruning) { + // Not a rehydration failure: the raw read would resume the session without + // holding it while zero sessions prune may be removing it. + return nil, err + } raw, rawErr := m.sessionStore.ReadEvents(sessionID) if rawErr != nil { // Surface the raw-read failure (the actual fallback error), not the earlier