diff --git a/AGENTS.md b/AGENTS.md index 41dc3be..d608bf3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -48,6 +48,7 @@ cmd/odek/ skill_promote.go `odek skill promote` — clear NeedsReview on a tainted skill schedule.go `odek schedule` command and scheduler wiring memory_cmd.go `odek memory` command + cleanup.go `odek cleanup [--dry-run]` one-shot storage sweep + janitor wiring (telegram/serve/schedule daemon) parallel.go Parallelism helpers toolctx.go Tool-call context plumbing security_report_validation_test.go Regression bar for every documented mitigation @@ -60,6 +61,7 @@ internal/ auth/ Interactive approval system memory/ MemoryManager (facts, buffer, episodes, merge, scan). EpisodeProvenance — tainted episodes never auto-replayed. session/ Session store (CRUD, trim, cleanup, compact JSON). AuditStore + divergence heuristic. + maintenance/ Storage-maintenance janitor (session/audit/plan retention, log rotation, media sweep, skill skip-list GC). Config: `maintenance` section (operator-only). skills/ Skill system (types, loader, triggers, self-improve, curator, import, cache). SkillProvenance gate. config/ Config file loading, env vars, secrets.env, priority merge telegram/ Telegram bot: bot.go, poller.go, handler.go, commands.go, session.go, health.go, plan.go, media_path.go @@ -91,6 +93,7 @@ ReAct cycle: observe → think → act → repeat. - **Interaction modes** — engaging (narrated), enhance (persistent), verbose (raw), off. - Max 300 iterations by default. - **Post-response async processing** — skill learning, episode extraction, and per-turn extended-memory extraction run in background goroutines tracked by `MemoryManager.RunBackground`; `Agent.Close` drains them via `WaitForBackground` (bounded, ~15s) so the work survives CLI exit without hanging `odek run`. +- **Storage maintenance janitor** — `maintenance.Start` (internal/maintenance) runs a periodic sweep of `~/.odek` (expired sessions/audit/plans, oversized-log rotation, media sweep, skill skip-list GC) inside `odek telegram`, `odek serve`, and `odek schedule daemon` when `maintenance.enabled` is set; `odek cleanup [--dry-run]` runs the same sweep on demand. Session files are also trimmed at write time (oldest groups first, system message kept) when they would exceed `MaxSessionFileBytes`, so a session can never grow past the load cap. Operator-only config — project `./odek.json` cannot set it. See docs/MAINTENANCE.md. - **Artifact-aware file search** — `search_files` and `multi_grep` skip build/artifact directories (`node_modules`, `vendor`, `.git`, `__pycache__`, `.venv`, etc.) automatically, reducing noise and speeding scans. - **Semantic session search** — the `session_search` tool uses go-vector RandomProjections + k-NN for semantic similarity search through session content, with a two-tier pipeline: vector index (fast, ~1ms) → deepSearch fallback (exhaustive, slower). - **Security-first defaults** — the latest hardening closes the high/medium/low findings tracked in `sec_findings.md`: default `non_interactive` is `deny`, project-level `odek.json` cannot redirect backends or hijack delivery, project-level sandbox knobs require explicit operator approval, `~/.odek` trust anchors are protected, WebSocket upgrades require a per-instance CSRF token, and all untrusted content is wrapped before reaching the model. See Security Architecture below for the full list. diff --git a/cmd/odek/cleanup.go b/cmd/odek/cleanup.go new file mode 100644 index 0000000..23c47db --- /dev/null +++ b/cmd/odek/cleanup.go @@ -0,0 +1,257 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" + "time" + + "github.com/BackendStack21/odek/internal/config" + "github.com/BackendStack21/odek/internal/maintenance" + "github.com/BackendStack21/odek/internal/session" +) + +// cleanupCmd implements `odek cleanup [--dry-run]`: a one-shot, operator- +// invoked storage sweep over ~/.odek (expired sessions, audit records, plans, +// skill skip entries, oversized logs). It runs the same maintenance.Sweep the +// background janitor uses in long-lived processes (telegram, serve, schedule +// daemon). Like `odek session cleanup`, this deletes data without a +// confirmation prompt — it is a local, operator-run command. +func cleanupCmd(args []string) error { + dryRun := false + for _, a := range args { + switch a { + case "--dry-run": + dryRun = true + case "--help", "-h": + fmt.Println(`Usage: odek cleanup [--dry-run] + +Remove expired odek storage from ~/.odek per the [maintenance] config +section: old sessions, audit records, plans, and skill skip entries, and +rotate oversized logs. + + --dry-run Show what would be removed without removing anything.`) + return nil + default: + return fmt.Errorf("unknown flag %q for cleanup", a) + } + } + + resolved := config.LoadConfig(config.CLIFlags{}) + cfg := maintenanceConfig(resolved) + home := expandHome("~/.odek") + + if dryRun { + printCleanupDryRun(home, cfg) + return nil + } + + report, err := maintenance.Sweep(context.Background(), home, cfg) + if err != nil { + return fmt.Errorf("cleanup: %w", err) + } + printCleanupReport(report) + return nil +} + +// maintenanceConfig returns the resolved maintenance section as a +// maintenance.Config. resolved.Maintenance already carries the fully +// defaulted type; this helper is the single mapping point if the resolved +// shape ever diverges. +func maintenanceConfig(resolved config.ResolvedConfig) maintenance.Config { + return resolved.Maintenance +} + +// startStorageMaintenance starts the background storage janitor when the +// resolved maintenance config enables it. Long-lived processes (telegram bot, +// web UI server, schedule daemon) call this at startup; the janitor stops +// when ctx is cancelled. +func startStorageMaintenance(ctx context.Context, resolved config.ResolvedConfig) { + cfg := maintenanceConfig(resolved) + if !cfg.Enabled { + return + } + maintenance.Start(ctx, expandHome("~/.odek"), cfg) + fmt.Fprintf(os.Stderr, "odek: storage maintenance enabled (interval %dm)\n", cfg.IntervalMinutes) +} + +// printCleanupReport prints the human-readable result of a sweep, or a quiet +// success line when there was nothing to do. +func printCleanupReport(r maintenance.Report) { + if r.SessionsRemoved == 0 && r.AuditRemoved == 0 && r.PlansRemoved == 0 && + r.SkipsRemoved == 0 && r.MediaFreedBytes == 0 && len(r.LogsRotated) == 0 { + fmt.Println("Storage is clean — nothing to remove.") + return + } + fmt.Println("Cleanup complete:") + fmt.Printf(" sessions removed: %d\n", r.SessionsRemoved) + fmt.Printf(" audit records removed: %d\n", r.AuditRemoved) + fmt.Printf(" plans removed: %d\n", r.PlansRemoved) + fmt.Printf(" skip entries removed: %d\n", r.SkipsRemoved) + fmt.Printf(" media freed: %s\n", humanBytes(r.MediaFreedBytes)) + for _, p := range r.LogsRotated { + fmt.Printf(" log rotated: %s\n", p) + } +} + +// humanBytes formats a byte count for human consumption (e.g. "12.3 MB"). +func humanBytes(n int64) string { + const unit = 1024 + if n < unit { + return fmt.Sprintf("%d B", n) + } + div, exp := int64(unit), 0 + for v := n / unit; v >= unit; v /= unit { + div *= unit + exp++ + } + return fmt.Sprintf("%.1f %cB", float64(n)/float64(div), "KMGTPE"[exp]) +} + +// ── Dry run ──────────────────────────────────────────────────────────── +// +// maintenance.Sweep has no dry-run mode, so the CLI builds the same candidate +// list locally for display only. Media cleanup is not previewed — its +// retention policy lives inside the maintenance package. + +// cleanupCandidates lists what a sweep WOULD remove, per category. +type cleanupCandidates struct { + sessions []string + audit []string + plans []string + skips int + logs []string +} + +// collectCleanupCandidates enumerates expired files under home without +// removing anything. +func collectCleanupCandidates(home string, cfg maintenance.Config) cleanupCandidates { + now := time.Now() + var c cleanupCandidates + + if cfg.SessionsMaxAgeDays > 0 { + c.sessions = sessionCandidates(home, now.AddDate(0, 0, -cfg.SessionsMaxAgeDays)) + } + if cfg.AuditMaxAgeDays > 0 { + c.audit = filesOlderThan(filepath.Join(home, "sessions", "audit"), now.AddDate(0, 0, -cfg.AuditMaxAgeDays), false) + } + if cfg.PlansMaxAgeDays > 0 { + // Plans may be nested per chat (plans/chat/), so walk recursively. + c.plans = filesOlderThan(filepath.Join(home, "plans"), now.AddDate(0, 0, -cfg.PlansMaxAgeDays), true) + } + if cfg.SkillsSkipMaxAgeDays > 0 { + c.skips = staleSkipEntries(filepath.Join(home, "skills", ".skipped.json"), now.AddDate(0, 0, -cfg.SkillsSkipMaxAgeDays)) + } + if cfg.LogMaxMB > 0 { + for _, name := range []string{"schedule.log", "telegram.log"} { + p := filepath.Join(home, name) + if info, err := os.Stat(p); err == nil && info.Size() > cfg.LogMaxMB*1024*1024 { + c.logs = append(c.logs, p) + } + } + } + return c +} + +// sessionCandidates lists session files whose UpdatedAt is before cutoff, +// using the session store's own listing so the dry-run preview matches what +// Store.Cleanup (and therefore the sweep) would delete. Unreadable sessions +// are skipped, mirroring Cleanup. +func sessionCandidates(home string, cutoff time.Time) []string { + store, err := session.NewStoreWithDir(filepath.Join(home, "sessions")) + if err != nil { + return nil + } + sessions, err := store.List(0) + if err != nil { + return nil + } + var out []string + for _, s := range sessions { + if s.UpdatedAt.Before(cutoff) { + out = append(out, store.Path(s.ID)) + } + } + return out +} + +// filesOlderThan returns the regular files under dir whose modification time +// is before cutoff. Session index/metadata files are excluded. Missing +// directories yield an empty list. +func filesOlderThan(dir string, cutoff time.Time, recursive bool) []string { + var out []string + walk := func(path string, d fs.DirEntry, err error) error { + if err != nil { + return nil // unreadable entries are not candidates + } + if d.IsDir() { + if path != dir && !recursive { + return fs.SkipDir + } + return nil + } + if d.Type()&fs.ModeSymlink != 0 { + return nil // never follow or report symlinks + } + name := d.Name() + if name == "index.json" || !strings.HasSuffix(name, ".json") && !strings.HasSuffix(name, ".md") { + return nil + } + info, err := d.Info() + if err != nil { + return nil + } + if info.ModTime().Before(cutoff) { + out = append(out, path) + } + return nil + } + _ = filepath.WalkDir(dir, walk) // missing dir → no candidates + return out +} + +// staleSkipEntries counts entries in the skills .skipped.json file whose +// skipped_at timestamp is before cutoff. An unreadable or malformed file +// counts as zero — the sweep itself decides what to do with it. +func staleSkipEntries(path string, cutoff time.Time) int { + data, err := os.ReadFile(path) + if err != nil { + return 0 + } + var file struct { + Skipped map[string]struct { + SkippedAt time.Time `json:"skipped_at"` + } `json:"skipped"` + } + if err := json.Unmarshal(data, &file); err != nil { + return 0 + } + n := 0 + for _, e := range file.Skipped { + if e.SkippedAt.Before(cutoff) { + n++ + } + } + return n +} + +// printCleanupDryRun reports the candidate list without removing anything. +func printCleanupDryRun(home string, cfg maintenance.Config) { + c := collectCleanupCandidates(home, cfg) + if len(c.sessions) == 0 && len(c.audit) == 0 && len(c.plans) == 0 && c.skips == 0 && len(c.logs) == 0 { + fmt.Println("Dry run: storage is clean — nothing would be removed.") + return + } + fmt.Println("Dry run — nothing removed. Would remove:") + fmt.Printf(" sessions: %d\n", len(c.sessions)) + fmt.Printf(" audit records: %d\n", len(c.audit)) + fmt.Printf(" plans: %d\n", len(c.plans)) + fmt.Printf(" skip entries: %d\n", c.skips) + for _, p := range c.logs { + fmt.Printf(" log rotated: %s\n", p) + } +} diff --git a/cmd/odek/cleanup_edges_test.go b/cmd/odek/cleanup_edges_test.go new file mode 100644 index 0000000..49f4486 --- /dev/null +++ b/cmd/odek/cleanup_edges_test.go @@ -0,0 +1,240 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/BackendStack21/odek/internal/maintenance" +) + +// writeCleanupFile writes content to path (creating parents) and backdates +// its modtime by age. +func writeCleanupFile(t *testing.T, path string, content []byte, age time.Duration) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, content, 0600); err != nil { + t.Fatal(err) + } + mod := time.Now().Add(-age) + if err := os.Chtimes(path, mod, mod); err != nil { + t.Fatal(err) + } +} + +func TestCleanupCmd_Help(t *testing.T) { + if err := cleanupCmd([]string{"--help"}); err != nil { + t.Errorf("cleanupCmd(--help) error: %v", err) + } +} + +func TestCleanupCmd_UnknownFlag(t *testing.T) { + if err := cleanupCmd([]string{"--bogus"}); err == nil || !strings.Contains(err.Error(), "unknown flag") { + t.Errorf("cleanupCmd(--bogus) error = %v, want an unknown flag error", err) + } +} + +// TestCleanupCmd_SweepError forces the maintenance sweep to fail (the +// sessions path is a regular file) and verifies the command reports it. +func TestCleanupCmd_SweepError(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + odekHome := filepath.Join(home, ".odek") + if err := os.MkdirAll(odekHome, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(odekHome, "sessions"), []byte("x"), 0600); err != nil { + t.Fatal(err) + } + if err := cleanupCmd(nil); err == nil || !strings.Contains(err.Error(), "cleanup:") { + t.Errorf("cleanupCmd() error = %v, want a cleanup: error", err) + } +} + +func TestPrintCleanupReport(t *testing.T) { + // Nothing removed → quiet success line. + printCleanupReport(maintenance.Report{}) + // Every category populated → full breakdown incl. rotated-log listing. + printCleanupReport(maintenance.Report{ + SessionsRemoved: 2, + AuditRemoved: 1, + PlansRemoved: 3, + SkipsRemoved: 1, + MediaFreedBytes: 2048, + LogsRotated: []string{filepath.Join("/tmp", "telegram.log")}, + }) +} + +func TestHumanBytes(t *testing.T) { + tests := []struct { + n int64 + want string + }{ + {0, "0 B"}, + {512, "512 B"}, + {1023, "1023 B"}, + {1024, "1.0 KB"}, + {2048, "2.0 KB"}, + {5 << 20, "5.0 MB"}, + {3 << 30, "3.0 GB"}, + {2 << 40, "2.0 TB"}, + } + for _, tc := range tests { + if got := humanBytes(tc.n); got != tc.want { + t.Errorf("humanBytes(%d) = %q, want %q", tc.n, got, tc.want) + } + } +} + +// TestCollectCleanupCandidates_Logs covers the oversized-log candidate branch. +func TestCollectCleanupCandidates_Logs(t *testing.T) { + home := t.TempDir() + big := make([]byte, 2<<20) // 2 MiB > 1 MB limit + writeCleanupFile(t, filepath.Join(home, "schedule.log"), big, time.Hour) + + c := collectCleanupCandidates(home, maintenance.Config{LogMaxMB: 1}) + if len(c.logs) != 1 { + t.Errorf("logs candidates = %d, want 1", len(c.logs)) + } +} + +// TestSessionCandidates_StoreError covers the NewStoreWithDir failure branch: +// the sessions path exists as a regular file. +func TestSessionCandidates_StoreError(t *testing.T) { + home := t.TempDir() + if err := os.WriteFile(filepath.Join(home, "sessions"), []byte("x"), 0600); err != nil { + t.Fatal(err) + } + if got := sessionCandidates(home, time.Now()); got != nil { + t.Errorf("sessionCandidates = %v, want nil", got) + } +} + +// TestSessionCandidates_ListError covers the store.List failure branch via an +// unreadable sessions directory. +func TestSessionCandidates_ListError(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root bypasses directory permission checks") + } + home := t.TempDir() + sessionsDir := filepath.Join(home, "sessions") + if err := os.MkdirAll(sessionsDir, 0755); err != nil { + t.Fatal(err) + } + if err := os.Chmod(sessionsDir, 0000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(sessionsDir, 0755) }) + + if got := sessionCandidates(home, time.Now()); got != nil { + t.Errorf("sessionCandidates = %v, want nil", got) + } +} + +// TestFilesOlderThan_NonRecursive covers the SkipDir branch for nested +// directories when recursive is false. +func TestFilesOlderThan_NonRecursive(t *testing.T) { + dir := t.TempDir() + writeCleanupFile(t, filepath.Join(dir, "chat1", "old.json"), []byte(`{}`), 48*time.Hour) + + if got := filesOlderThan(dir, time.Now(), false); len(got) != 0 { + t.Errorf("non-recursive candidates = %v, want none (nested dir skipped)", got) + } + if got := filesOlderThan(dir, time.Now(), true); len(got) != 1 { + t.Errorf("recursive candidates = %v, want 1", got) + } +} + +// TestFilesOlderThan_Symlink covers the symlink skip branch. +func TestFilesOlderThan_Symlink(t *testing.T) { + dir := t.TempDir() + writeCleanupFile(t, filepath.Join(dir, "old.json"), []byte(`{}`), 48*time.Hour) + if err := os.Symlink(filepath.Join(dir, "old.json"), filepath.Join(dir, "link.json")); err != nil { + t.Fatal(err) + } + + got := filesOlderThan(dir, time.Now(), false) + if len(got) != 1 || filepath.Base(got[0]) != "old.json" { + t.Errorf("candidates = %v, want only old.json (symlink skipped)", got) + } +} + +// TestFilesOlderThan_NameFilter covers the index.json / non-json-md skip branch. +func TestFilesOlderThan_NameFilter(t *testing.T) { + dir := t.TempDir() + writeCleanupFile(t, filepath.Join(dir, "index.json"), []byte(`{}`), 48*time.Hour) + writeCleanupFile(t, filepath.Join(dir, "notes.txt"), []byte("x"), 48*time.Hour) + writeCleanupFile(t, filepath.Join(dir, "old.json"), []byte(`{}`), 48*time.Hour) + + got := filesOlderThan(dir, time.Now(), false) + if len(got) != 1 || filepath.Base(got[0]) != "old.json" { + t.Errorf("candidates = %v, want only old.json", got) + } +} + +// TestFilesOlderThan_UnreadableDir covers the walk-error branch via an +// unreadable subdirectory. +func TestFilesOlderThan_UnreadableDir(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root bypasses directory permission checks") + } + dir := t.TempDir() + writeCleanupFile(t, filepath.Join(dir, "chat1", "old.json"), []byte(`{}`), 48*time.Hour) + if err := os.Chmod(filepath.Join(dir, "chat1"), 0000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(filepath.Join(dir, "chat1"), 0755) }) + + if got := filesOlderThan(dir, time.Now(), true); len(got) != 0 { + t.Errorf("candidates = %v, want none (unreadable dir skipped)", got) + } +} + +// TestFilesOlderThan_InfoError covers the Info() failure branch by removing +// the search bit from the directory while keeping it readable. +func TestFilesOlderThan_InfoError(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root bypasses directory permission checks") + } + dir := t.TempDir() + writeCleanupFile(t, filepath.Join(dir, "old.json"), []byte(`{}`), 48*time.Hour) + if err := os.Chmod(dir, 0400); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(dir, 0755) }) + + if got := filesOlderThan(dir, time.Now(), false); len(got) != 0 { + t.Errorf("candidates = %v, want none (unreadable entries skipped)", got) + } +} + +func TestStaleSkipEntries_Errors(t *testing.T) { + // Missing file → zero candidates. + if got := staleSkipEntries(filepath.Join(t.TempDir(), ".skipped.json"), time.Now()); got != 0 { + t.Errorf("staleSkipEntries(missing) = %d, want 0", got) + } + // Malformed JSON → zero candidates. + dir := t.TempDir() + bad := filepath.Join(dir, ".skipped.json") + if err := os.WriteFile(bad, []byte("{not json"), 0600); err != nil { + t.Fatal(err) + } + if got := staleSkipEntries(bad, time.Now()); got != 0 { + t.Errorf("staleSkipEntries(malformed) = %d, want 0", got) + } +} + +func TestPrintCleanupDryRun(t *testing.T) { + // Empty home → quiet "nothing would be removed" line. + printCleanupDryRun(t.TempDir(), maintenance.DefaultConfig()) + + // Oversized log → candidate summary incl. the rotated-log listing. + home := t.TempDir() + big := make([]byte, 2<<20) + writeCleanupFile(t, filepath.Join(home, "telegram.log"), big, time.Hour) + printCleanupDryRun(home, maintenance.Config{LogMaxMB: 1}) +} diff --git a/cmd/odek/cleanup_test.go b/cmd/odek/cleanup_test.go new file mode 100644 index 0000000..e6c1f8c --- /dev/null +++ b/cmd/odek/cleanup_test.go @@ -0,0 +1,215 @@ +package main + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + "time" + + "github.com/BackendStack21/odek/internal/config" + "github.com/BackendStack21/odek/internal/maintenance" +) + +// seedCleanupHome creates a temp HOME with an ~/.odek tree containing old and +// new files in each category the janitor manages. It returns the home path. +func seedCleanupHome(t *testing.T) (home, odekHome string) { + t.Helper() + home = t.TempDir() + t.Setenv("HOME", home) + odekHome = filepath.Join(home, ".odek") + + old := time.Now().AddDate(0, 0, -120) // older than every default retention + writeFile := func(rel string, mod time.Time) string { + p := filepath.Join(odekHome, rel) + if err := os.MkdirAll(filepath.Dir(p), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(`{"id":"x"}`), 0600); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(p, mod, mod); err != nil { + t.Fatal(err) + } + return p + } + // Sessions are swept by Store.Cleanup, which reads each file's embedded + // updated_at (and requires the embedded ID to match the filename). + writeSession := func(id string, updated time.Time) string { + p := filepath.Join(odekHome, "sessions", id+".json") + if err := os.MkdirAll(filepath.Dir(p), 0755); err != nil { + t.Fatal(err) + } + body, err := json.Marshal(map[string]any{ + "id": id, + "updated_at": updated.UTC().Format(time.RFC3339), + "messages": []any{}, + }) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, body, 0600); err != nil { + t.Fatal(err) + } + return p + } + + // Old entries (must be swept) and new entries (must survive). + writeSession("20250101-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", old) + writeSession("29990101-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", time.Now()) + writeFile("sessions/audit/20250101-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.json", old) + writeFile("sessions/audit/29990101-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.json", time.Now()) + writeFile("plans/chat1/old-plan.md", old) + writeFile("plans/chat1/new-plan.md", time.Now()) + + // Skill skips: one stale, one fresh. + skillsDir := filepath.Join(odekHome, "skills") + if err := os.MkdirAll(skillsDir, 0755); err != nil { + t.Fatal(err) + } + skips := map[string]any{ + "skipped": map[string]any{ + "old-skill": map[string]any{"skipped_at": old.Format(time.RFC3339), "times_skipped": 3}, + "new-skill": map[string]any{"skipped_at": time.Now().Format(time.RFC3339), "times_skipped": 1}, + }, + } + data, err := json.Marshal(skips) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(skillsDir, ".skipped.json"), data, 0600); err != nil { + t.Fatal(err) + } + return home, odekHome +} + +// TestCleanupCmd_Sweep removes only expired entries and keeps fresh ones. +func TestCleanupCmd_Sweep(t *testing.T) { + _, odekHome := seedCleanupHome(t) + + if err := cleanupCmd(nil); err != nil { + t.Fatalf("cleanupCmd() error: %v", err) + } + + for _, rel := range []string{ + "sessions/20250101-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.json", + "sessions/audit/20250101-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.json", + "plans/chat1/old-plan.md", + } { + if _, err := os.Stat(filepath.Join(odekHome, rel)); !os.IsNotExist(err) { + t.Errorf("expected %s to be removed, stat err = %v", rel, err) + } + } + for _, rel := range []string{ + "sessions/29990101-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.json", + "sessions/audit/29990101-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.json", + "plans/chat1/new-plan.md", + } { + if _, err := os.Stat(filepath.Join(odekHome, rel)); err != nil { + t.Errorf("expected %s to survive: %v", rel, err) + } + } +} + +// TestCleanupCmd_DryRun reports candidates but removes nothing. +func TestCleanupCmd_DryRun(t *testing.T) { + _, odekHome := seedCleanupHome(t) + + if err := cleanupCmd([]string{"--dry-run"}); err != nil { + t.Fatalf("cleanupCmd(--dry-run) error: %v", err) + } + + for _, rel := range []string{ + "sessions/20250101-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.json", + "sessions/audit/20250101-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.json", + "plans/chat1/old-plan.md", + "plans/chat1/new-plan.md", + } { + if _, err := os.Stat(filepath.Join(odekHome, rel)); err != nil { + t.Errorf("dry-run must not remove %s: %v", rel, err) + } + } +} + +// TestCleanupDryRun_Candidates exercises the local candidate enumeration used +// by --dry-run against the seeded tree. +func TestCleanupDryRun_Candidates(t *testing.T) { + _, odekHome := seedCleanupHome(t) + + // Default-shaped config: 30d sessions, 14d audit, 30d plans, 90d skips. + cfg := maintenanceConfig(config.ResolvedConfig{Maintenance: maintenance.Config{ + SessionsMaxAgeDays: 30, + AuditMaxAgeDays: 14, + PlansMaxAgeDays: 30, + SkillsSkipMaxAgeDays: 90, + }}) + c := collectCleanupCandidates(odekHome, cfg) + if len(c.sessions) != 1 { + t.Errorf("sessions candidates = %d, want 1", len(c.sessions)) + } + if len(c.audit) != 1 { + t.Errorf("audit candidates = %d, want 1", len(c.audit)) + } + if len(c.plans) != 1 { + t.Errorf("plans candidates = %d, want 1", len(c.plans)) + } + if c.skips != 1 { + t.Errorf("skip candidates = %d, want 1", c.skips) + } +} + +// TestMaintenanceConfigMapping pins the resolved-config → maintenance.Config +// field mapping so a shape change in config.ResolvedConfig.Maintenance fails +// here first. +func TestMaintenanceConfigMapping(t *testing.T) { + resolved := config.ResolvedConfig{Maintenance: maintenance.Config{ + Enabled: true, + IntervalMinutes: 60, + SessionsMaxAgeDays: 30, + AuditMaxAgeDays: 14, + LogMaxMB: 50, + PlansMaxAgeDays: 30, + SkillsSkipMaxAgeDays: 90, + }} + cfg := maintenanceConfig(resolved) + if !cfg.Enabled || cfg.IntervalMinutes != 60 || cfg.SessionsMaxAgeDays != 30 || + cfg.AuditMaxAgeDays != 14 || cfg.LogMaxMB != 50 || cfg.PlansMaxAgeDays != 30 || + cfg.SkillsSkipMaxAgeDays != 90 { + t.Errorf("maintenanceConfig mapping wrong: %+v", cfg) + } +} + +// TestStartStorageMaintenance_Disabled is a no-op when the config disables the +// janitor — it must return immediately without starting anything. +func TestStartStorageMaintenance_Disabled(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + resolved := config.ResolvedConfig{Maintenance: maintenance.Config{Enabled: false}} + done := make(chan struct{}) + go func() { + startStorageMaintenance(t.Context(), resolved) + close(done) + }() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("startStorageMaintenance did not return for disabled config") + } +} + +// TestStartStorageMaintenance_Enabled verifies the janitor starts and stops +// cleanly with the context. Behavioural sweep assertions live in the +// cleanupCmd tests; this only proves the wiring does not hang or panic. +func TestStartStorageMaintenance_Enabled(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + resolved := config.ResolvedConfig{Maintenance: maintenance.Config{ + Enabled: true, + IntervalMinutes: 60, + }} + ctx, cancel := context.WithCancel(t.Context()) + startStorageMaintenance(ctx, resolved) + cancel() + // Give the janitor goroutine a moment to observe cancellation; there is + // no join handle, so this is a smoke check only. + time.Sleep(50 * time.Millisecond) +} diff --git a/cmd/odek/dispatch.go b/cmd/odek/dispatch.go index 58a6af2..6ece722 100644 --- a/cmd/odek/dispatch.go +++ b/cmd/odek/dispatch.go @@ -55,6 +55,8 @@ func dispatch(args []string) int { return cliExit(scheduleCmd(rest)) case "memory": return cliExit(memoryCmd(rest)) + case "cleanup": + return cliExit(cleanupCmd(rest)) default: fmt.Fprintf(os.Stderr, "odek: unknown command %q\n", cmd) printUsage() diff --git a/cmd/odek/main.go b/cmd/odek/main.go index 28ba241..a8964bb 100644 --- a/cmd/odek/main.go +++ b/cmd/odek/main.go @@ -822,6 +822,7 @@ func printUsage() { odek telegram odek schedule odek memory > + odek cleanup [--dry-run] odek version Commands: @@ -851,6 +852,8 @@ Commands: list: show episodes excluded from recall (untrusted) promote : approve one so it can be recalled. Human-gated on purpose — not available to the agent. + cleanup One-shot storage sweep of ~/.odek (sessions, audit, + plans, skill skips, log rotation). --dry-run previews. init Create a config file (default: ./odek.json) version Print version and exit diff --git a/cmd/odek/schedule.go b/cmd/odek/schedule.go index 476f753..fc80fbc 100644 --- a/cmd/odek/schedule.go +++ b/cmd/odek/schedule.go @@ -326,6 +326,10 @@ func scheduleDaemon(_ []string) error { ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer cancel() + // Start the storage-maintenance janitor (expired sessions, audit records, + // plans, skill skips, log rotation), tied to the daemon's lifetime. + startStorageMaintenance(ctx, resolved) + jobs, _ := st.List() enabled := 0 for _, j := range jobs { diff --git a/cmd/odek/serve.go b/cmd/odek/serve.go index 6ff499e..f21eb4a 100644 --- a/cmd/odek/serve.go +++ b/cmd/odek/serve.go @@ -358,6 +358,13 @@ func serveCmd(args []string) error { openInBrowser(tokenURL) } + // Start the storage-maintenance janitor (expired sessions, audit records, + // plans, skill skips, log rotation) for the life of the server. The + // context is cancelled when serveOnListener returns on shutdown. + maintCtx, maintCancel := context.WithCancel(context.Background()) + defer maintCancel() + startStorageMaintenance(maintCtx, resolved) + return serveOnListener(listener, mux) } diff --git a/cmd/odek/telegram.go b/cmd/odek/telegram.go index 91a5c6b..9f4cad6 100644 --- a/cmd/odek/telegram.go +++ b/cmd/odek/telegram.go @@ -913,6 +913,10 @@ func telegramCmd(args []string) error { stopScheduler := startSchedulerForBot(ctx, bot, resolved, systemMessage, handlerLog, scheduleStore, sessionManager) defer stopScheduler() + // 16c. Start the storage-maintenance janitor (expired sessions, audit + // records, plans, skill skips, log rotation), tied to the bot's lifetime. + startStorageMaintenance(ctx, resolved) + // 17. Process updates until the channel is closed (ctx cancelled). for upd := range updates { handler.HandleUpdate(upd) diff --git a/docs/CLI.md b/docs/CLI.md index e850d54..1a25595 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -13,7 +13,8 @@ | `odek session show [id]` | Show session details (default: latest) | | `odek session delete ` | Delete a session | | `odek session trim ` | Keep only the `n` most recent messages | -| `odek session cleanup ` | Delete sessions older than N days | +| `odek session cleanup ` | Delete sessions older than N days (see also: automatic storage maintenance below) | +| `odek cleanup [--dry-run]` | One-shot storage sweep of `~/.odek`: expired sessions, audit records, plans, skill skip entries, and oversized-log rotation, per the `[maintenance]` config. `--dry-run` previews without deleting. The same sweep runs automatically in the Telegram bot, `odek serve`, and `odek schedule daemon`. See [MAINTENANCE.md](MAINTENANCE.md) | | `odek skill list` | List all available skills | | `odek skill view ` | View a skill's full content | | `odek skill delete ` | Delete a skill | @@ -389,6 +390,12 @@ odek session cleanup 30 # Wipe all sessions odek session cleanup 0 +# Sweep all expired storage (sessions, audit, plans, skips, log rotation) +odek cleanup + +# Preview the sweep without deleting anything +odek cleanup --dry-run + # OpenAI odek run --model gpt-4o --base-url https://api.openai.com/v1 "Explain this code" diff --git a/docs/CONFIG.md b/docs/CONFIG.md index fcf7a27..c888f4d 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -650,7 +650,7 @@ The `telegram` section configures the Telegram bot integration and the `--delive | `poll_interval` | — | 1 | Seconds between poll cycles | | `poll_timeout` | — | 30 | Long-poll timeout (1-60 seconds) | | `max_msg_length` | — | 4096 | Max characters per message | -| `session_ttl_hours` | — | 24 | Hours before inactive session expires | +| `session_ttl_hours` | — | 24 | Hours an inactive chat's session stays in the in-memory cache before being reloaded from disk. This is cache-only — on-disk session expiry is `maintenance.sessions_max_age_days` (see [MAINTENANCE.md](MAINTENANCE.md)) | | `max_download_size` | `ODEK_TELEGRAM_MAX_DOWNLOAD_SIZE` | 5242880 (5 MiB) | Per-file byte cap for Telegram voice/photo/document downloads. Set to `-1` to disable. | | `media_quota_per_chat` | `ODEK_TELEGRAM_MEDIA_QUOTA_PER_CHAT` | 0 (disabled) | Total bytes of downloaded media allowed per chat. `0` disables the quota. | | `log_level` | — | info | Log level: debug, info, warn, error | @@ -740,6 +740,46 @@ Project-level `odek.json` cannot set `schedules.dangerous`; configure it via `~/ Full guide: [docs/SCHEDULES.md](SCHEDULES.md). +## Storage maintenance + +Configures the background storage janitor (`internal/maintenance`). It runs a +sweep over `~/.odek` every `interval_minutes`: expiring old sessions and +audit records, rotating oversized logs, deleting stale Telegram plans and +downloaded media, and garbage-collecting expired skill skip-list entries. +Every field has an `ODEK_MAINTENANCE_*` environment override. + +```json +{ + "maintenance": { + "enabled": true, + "interval_minutes": 60, + "sessions_max_age_days": 30, + "audit_max_age_days": 14, + "log_max_mb": 50, + "plans_max_age_days": 30, + "skills_skip_max_age_days": 90 + } +} +``` + +| Field | Env | Default | Description | +|---|---|---|---| +| `enabled` | `ODEK_MAINTENANCE_ENABLED` | `true` | Run the janitor. Set false to disable all storage maintenance. | +| `interval_minutes` | `ODEK_MAINTENANCE_INTERVAL_MINUTES` | `60` | Minutes between sweeps. The first sweep runs after one interval, never at startup. | +| `sessions_max_age_days` | `ODEK_MAINTENANCE_SESSIONS_MAX_AGE_DAYS` | `30` | Delete sessions (and their index/vector-index entries) older than this. `0` = keep forever. | +| `audit_max_age_days` | `ODEK_MAINTENANCE_AUDIT_MAX_AGE_DAYS` | `14` | Delete `~/.odek/sessions/audit/*.json` records older than this. `0` = keep forever. | +| `log_max_mb` | `ODEK_MAINTENANCE_LOG_MAX_MB` | `50` | Rotate `~/.odek/telegram.log` and `~/.odek/schedule.log` larger than this: current log becomes `.1` (one backup generation) and a fresh empty log is started. `0` = no rotation. | +| `plans_max_age_days` | `ODEK_MAINTENANCE_PLANS_MAX_AGE_DAYS` | `30` | Delete Telegram plan files (`~/.odek/plans/**/*.md`) older than this; emptied chat directories are removed. `0` = keep forever. | +| `skills_skip_max_age_days` | `ODEK_MAINTENANCE_SKILLS_SKIP_MAX_AGE_DAYS` | `90` | Remove skill skip-list entries (`~/.odek/skills/.skipped.json`) older than this. `0` = keep forever. | + +Downloaded Telegram media (`~/.odek/media/`, including per-chat `chat/` +subdirectories) is always swept after 1 hour; that policy is not configurable. + +The `maintenance` section is **operator-only**: it governs deletion of user +data, so the project-level `./odek.json` cannot set it (a `maintenance` +section there is ignored with a stderr warning). Configure it via +`~/.odek/config.json` or the `ODEK_MAINTENANCE_*` environment variables. + ## Tool configuration Control which tools are exposed to the LLM. Use this to deploy locked-down diff --git a/docs/MAINTENANCE.md b/docs/MAINTENANCE.md new file mode 100644 index 0000000..4261d80 --- /dev/null +++ b/docs/MAINTENANCE.md @@ -0,0 +1,144 @@ +# Storage Maintenance + +odek accumulates local state under `~/.odek/` — session transcripts, prompt- +injection audit records, plans, skill skip lists, and log files. The storage +**janitor** keeps that growth bounded: a background sweep that removes expired +entries and rotates oversized logs, plus an `odek cleanup` command for one-shot, +operator-invoked runs. + +```bash +# Sweep expired storage once, right now +odek cleanup + +# See what a sweep WOULD remove, without deleting anything +odek cleanup --dry-run +``` + +--- + +## Where the janitor runs + +The background janitor starts automatically in every long-lived odek process +(when `maintenance.enabled` is true): + +| Process | Notes | +|---|---| +| **`odek telegram`** | Starts with the bot, stops on shutdown. | +| **`odek serve`** | Starts with the Web UI server. | +| **`odek schedule daemon`** | Starts with the scheduler daemon. | + +Each prints `odek: storage maintenance enabled (interval 60m)` at startup. The +janitor sweeps on a fixed interval and stops with the process. Short-lived +commands (`odek run`, `odek repl`, …) do not run the janitor — use +`odek cleanup` for those workflows. + +## What is cleaned + +| Category | Location | Retention key | Default | +|---|---|---|---| +| Sessions | `~/.odek/sessions/*.json` (by `updated_at`) | `sessions_max_age_days` | 30 days | +| Audit records | `~/.odek/sessions/audit/*.json` (by mtime) | `audit_max_age_days` | 14 days | +| Plans | `~/.odek/plans/**/*.md` (by mtime) | `plans_max_age_days` | 30 days | +| Skill skip entries | `~/.odek/skills/.skipped.json` | `skills_skip_max_age_days` | 90 days | +| Telegram media | `~/.odek/media/` (by mtime) | fixed: 1 hour | freed bytes reported | +| Logs | `~/.odek/telegram.log`, `~/.odek/schedule.log` | `log_max_mb` | 50 MB (rotated) | + +Age for sessions is measured from the session's `updated_at`; for audit +records, plans, and media from the file's modification time. Oversized logs +are **rotated**, not deleted — the current log is renamed to `.1` +(one backup generation) and a fresh log is started. The sweep report includes +the rotated paths. Downloaded Telegram media is transient and expires after a +fixed 1 hour. + +## What is NEVER touched + +The janitor only expires the categories above. It never touches: + +- **Memory** — atoms, facts, episodes, buffers (`~/.odek/memory/`) +- **Skill files** — `SKILL.md` definitions (only stale `.skipped.json` + entries age out) +- **Schedules** — `schedules.json`, `schedule-state.json` +- **Trust anchors** — `config.json`, `secrets.env`, `IDENTITY.md`, + approval stores, lock files, and everything else under `~/.odek/` that is + not in the "What is cleaned" table + +## Configuration + +The `[maintenance]` section (all keys optional — defaults shown): + +```json +{ + "maintenance": { + "enabled": true, + "interval_minutes": 60, + "sessions_max_age_days": 30, + "audit_max_age_days": 14, + "log_max_mb": 50, + "plans_max_age_days": 30, + "skills_skip_max_age_days": 90 + } +} +``` + +| Key | Default | Description | +|---|---|---| +| `enabled` | `true` | Run the background janitor in long-lived processes | +| `interval_minutes` | `60` | Minutes between automatic sweeps | +| `sessions_max_age_days` | `30` | Delete sessions older than this | +| `audit_max_age_days` | `14` | Delete prompt-injection audit records older than this | +| `log_max_mb` | `50` | Rotate logs larger than this | +| `plans_max_age_days` | `30` | Delete plans older than this | +| `skills_skip_max_age_days` | `90` | Drop skill skip entries older than this | + +The maintenance config is **operator-only**: like `base_url`, `api_key`, and +the `dangerous` section, it is honored from `~/.odek/config.json` (and process +environment) but **ignored from a project-level `./odek.json`**, so a checked- +out repository cannot disable the janitor or relax its own retention. + +## `odek cleanup` + +`odek cleanup` runs one sweep immediately, using the same resolved config as +the background janitor, and prints a per-category report: + +``` +$ odek cleanup +Cleanup complete: + sessions removed: 12 + audit records removed: 34 + plans removed: 2 + skip entries removed: 5 + media freed: 48.2 MB + log rotated: /home/you/.odek/schedule.log +``` + +When there is nothing to do it prints a single quiet line: + +``` +Storage is clean — nothing to remove. +``` + +`odek cleanup --dry-run` removes nothing and reports what a sweep would +remove: + +``` +$ odek cleanup --dry-run +Dry run — nothing removed. Would remove: + sessions: 12 + audit records: 34 + plans: 2 + skip entries: 5 + log rotated: /home/you/.odek/schedule.log +``` + +Like `odek session cleanup`, the command deletes data without a confirmation +prompt — it is a local, operator-invoked command. Use `--dry-run` first if +you want to inspect the candidate list. + +## Related + +- [CLI.md](CLI.md) — command reference +- [SCHEDULES.md](SCHEDULES.md) — the schedule daemon (hosts the janitor) +- [CONFIG.md](CONFIG.md) — full configuration reference +- `internal/session` — session files are also capped at 32 MiB **at write + time**: an oversized transcript is trimmed (oldest turns first, keeping the + system message and the most recent turns) so it never becomes unloadable. diff --git a/docs/SCHEDULES.md b/docs/SCHEDULES.md index 837596e..b7e274e 100644 --- a/docs/SCHEDULES.md +++ b/docs/SCHEDULES.md @@ -39,6 +39,10 @@ running, just without scheduling); if the bot holds it, a standalone bot's embedded scheduler with `schedules.enabled = false` if you prefer to run the daemon separately.) +Both also host the storage-maintenance janitor, which expires old sessions, +audit records, and plans and rotates oversized logs — see +[MAINTENANCE.md](MAINTENANCE.md). + --- ## Managing jobs diff --git a/internal/config/loader.go b/internal/config/loader.go index b5b5119..3da1003 100644 --- a/internal/config/loader.go +++ b/internal/config/loader.go @@ -26,6 +26,7 @@ import ( "github.com/BackendStack21/odek/internal/danger" "github.com/BackendStack21/odek/internal/embedding" "github.com/BackendStack21/odek/internal/guard" + "github.com/BackendStack21/odek/internal/maintenance" "github.com/BackendStack21/odek/internal/mcpclient" "github.com/BackendStack21/odek/internal/memory" "github.com/BackendStack21/odek/internal/memory/extended" @@ -101,11 +102,11 @@ type CLIFlags struct { MemoryExtendedFollowUpAnticipationEnabled *bool // nil = not set // Guard subsystem CLI overrides. - GuardProvider string // "" = not set - GuardURL string // "" = not set - GuardBatchURL string // "" = not set - GuardLongURL string // "" = not set - GuardSocketPath string // "" = not set + GuardProvider string // "" = not set + GuardURL string // "" = not set + GuardBatchURL string // "" = not set + GuardLongURL string // "" = not set + GuardSocketPath string // "" = not set GuardThreshold float64 // 0 = not set GuardTimeoutSeconds int // 0 = not set GuardFallbackToLocal *bool // nil = not set @@ -202,6 +203,21 @@ type ToolConfig struct { Disabled []string `json:"disabled,omitempty"` } +// MaintenanceConfig is the file-level "maintenance" section. Pointer fields +// distinguish "not set" (inherit the default) from an explicit 0, which is +// meaningful for the retention knobs (0 = keep forever / disable). +// Operator-controlled: rejected from project-level ./odek.json because it +// governs DELETION of user data. +type MaintenanceConfig struct { + Enabled *bool `json:"enabled,omitempty"` + IntervalMinutes *int `json:"interval_minutes,omitempty"` + SessionsMaxAgeDays *int `json:"sessions_max_age_days,omitempty"` + AuditMaxAgeDays *int `json:"audit_max_age_days,omitempty"` + LogMaxMB *int64 `json:"log_max_mb,omitempty"` + PlansMaxAgeDays *int `json:"plans_max_age_days,omitempty"` + SkillsSkipMaxAgeDays *int `json:"skills_skip_max_age_days,omitempty"` +} + // ToolsConfig is the "tools" section of odek.json. It is intentionally a // pointer in FileConfig so "not set" can be distinguished from an explicit // empty list. @@ -303,6 +319,11 @@ type FileConfig struct { // Schedules configures the native in-process task scheduler. Schedules *SchedulesConfig `json:"schedules,omitempty"` + // Maintenance configures the storage-maintenance janitor (retention and + // deletion of sessions, audit records, plans, logs, skip-list entries). + // Operator-controlled: rejected from project-level ./odek.json. + Maintenance *MaintenanceConfig `json:"maintenance,omitempty"` + // Tools controls which tools are exposed to the LLM. // Project-level ./odek.json may only disable tools, not enable them. Tools *ToolsConfig `json:"tools,omitempty"` @@ -464,6 +485,11 @@ type ResolvedConfig struct { // Default: enabled=true, max_concurrent=2, timezone="UTC", catchup=false. Schedules ScheduleConfig + // Maintenance is the resolved storage-maintenance config. + // Default: maintenance.DefaultConfig() (enabled, 60min tick, sessions 30d, + // audit 14d, logs 50MB, plans 30d, skip-list 90d). + Maintenance maintenance.Config + // Tools is the resolved tool-list configuration. // Empty Enabled/Disabled means "no restriction" for that direction. Tools ToolConfig @@ -666,6 +692,35 @@ func envInt(key string) int { return n } +// envIntPtr parses a ODEK_* env var as an integer. Returns nil if unset or +// unparseable, so an explicit 0 (meaningful for retention knobs) stays +// distinguishable from "not set". +func envIntPtr(key string) *int { + v := os.Getenv("ODEK_" + key) + if v == "" { + return nil + } + n, err := strconv.Atoi(v) + if err != nil { + return nil + } + return &n +} + +// envInt64Ptr parses a ODEK_* env var as an int64. Returns nil if unset or +// unparseable, like envIntPtr. +func envInt64Ptr(key string) *int64 { + v := os.Getenv("ODEK_" + key) + if v == "" { + return nil + } + n, err := strconv.ParseInt(v, 10, 64) + if err != nil { + return nil + } + return &n +} + // envFloat parses a ODEK_* env var as a float64. Returns 0 if unset/unparseable. func envFloat(key string) float64 { v := os.Getenv("ODEK_" + key) @@ -737,6 +792,46 @@ func ensureGuard(cfg *guard.Config) *guard.Config { return cfg } +// ensureMaintenance returns a non-nil *MaintenanceConfig, allocating one if needed. +func ensureMaintenance(cfg *MaintenanceConfig) *MaintenanceConfig { + if cfg == nil { + return &MaintenanceConfig{} + } + return cfg +} + +// resolveMaintenance merges the file-level maintenance section over the +// package defaults. Unset (nil) fields inherit the default; explicit 0 keeps +// its meaning (keep forever / disable). +func resolveMaintenance(cfg *MaintenanceConfig) maintenance.Config { + def := maintenance.DefaultConfig() + if cfg == nil { + return def + } + if cfg.Enabled != nil { + def.Enabled = *cfg.Enabled + } + if cfg.IntervalMinutes != nil { + def.IntervalMinutes = *cfg.IntervalMinutes + } + if cfg.SessionsMaxAgeDays != nil { + def.SessionsMaxAgeDays = *cfg.SessionsMaxAgeDays + } + if cfg.AuditMaxAgeDays != nil { + def.AuditMaxAgeDays = *cfg.AuditMaxAgeDays + } + if cfg.LogMaxMB != nil { + def.LogMaxMB = *cfg.LogMaxMB + } + if cfg.PlansMaxAgeDays != nil { + def.PlansMaxAgeDays = *cfg.PlansMaxAgeDays + } + if cfg.SkillsSkipMaxAgeDays != nil { + def.SkillsSkipMaxAgeDays = *cfg.SkillsSkipMaxAgeDays + } + return def +} + // envScheduleDangerousConfig parses ODEK_SCHEDULES_DANGEROUS_* env vars into a // DangerousConfig. Returns nil if none are set. func envScheduleDangerousConfig(prefix string) *danger.DangerousConfig { @@ -857,6 +952,12 @@ func LoadConfig(cli CLIFlags) ResolvedConfig { fmt.Fprintf(os.Stderr, "odek: WARNING: ignoring memory from project config (%s); set it via ~/.odek/config.json\n", ProjectConfigPath()) project.Memory = nil } + // The maintenance section governs DELETION of user data (sessions, audit + // records, plans, logs). A malicious repo must not be able to set it. + if project.Maintenance != nil { + fmt.Fprintf(os.Stderr, "odek: WARNING: ignoring maintenance from project config (%s); set it via ~/.odek/config.json or ODEK_MAINTENANCE_*\n", ProjectConfigPath()) + project.Maintenance = nil + } if project.Guard != nil { fmt.Fprintf(os.Stderr, "odek: WARNING: ignoring guard from project config (%s); set it via ~/.odek/config.json, ODEK_GUARD_*, or the CLI\n", ProjectConfigPath()) project.Guard = nil @@ -1245,6 +1346,38 @@ func LoadConfig(cli CLIFlags) ResolvedConfig { } } + // Maintenance env overrides (ODEK_MAINTENANCE_*). Explicit 0 is meaningful + // for the retention knobs (0 = keep forever / disable), so they parse via + // the pointer helpers rather than envInt. + if v := envBool("MAINTENANCE_ENABLED"); v != nil { + cfg.Maintenance = ensureMaintenance(cfg.Maintenance) + cfg.Maintenance.Enabled = v + } + if v := envIntPtr("MAINTENANCE_INTERVAL_MINUTES"); v != nil { + cfg.Maintenance = ensureMaintenance(cfg.Maintenance) + cfg.Maintenance.IntervalMinutes = v + } + if v := envIntPtr("MAINTENANCE_SESSIONS_MAX_AGE_DAYS"); v != nil { + cfg.Maintenance = ensureMaintenance(cfg.Maintenance) + cfg.Maintenance.SessionsMaxAgeDays = v + } + if v := envIntPtr("MAINTENANCE_AUDIT_MAX_AGE_DAYS"); v != nil { + cfg.Maintenance = ensureMaintenance(cfg.Maintenance) + cfg.Maintenance.AuditMaxAgeDays = v + } + if v := envInt64Ptr("MAINTENANCE_LOG_MAX_MB"); v != nil { + cfg.Maintenance = ensureMaintenance(cfg.Maintenance) + cfg.Maintenance.LogMaxMB = v + } + if v := envIntPtr("MAINTENANCE_PLANS_MAX_AGE_DAYS"); v != nil { + cfg.Maintenance = ensureMaintenance(cfg.Maintenance) + cfg.Maintenance.PlansMaxAgeDays = v + } + if v := envIntPtr("MAINTENANCE_SKILLS_SKIP_MAX_AGE_DAYS"); v != nil { + cfg.Maintenance = ensureMaintenance(cfg.Maintenance) + cfg.Maintenance.SkillsSkipMaxAgeDays = v + } + // Telegram env overrides: merge env vars on top of file config. baseTelegram := telegram.DefaultConfig() if cfg.Telegram != nil { @@ -1489,29 +1622,30 @@ func LoadConfig(cli CLIFlags) ResolvedConfig { MaxIter: cfg.MaxIter, System: cfg.System, - SandboxImage: cfg.SandboxImage, // empty = resolve at call site (Dockerfile.odek or alpine:latest) - SandboxNetwork: ifZero(cfg.SandboxNetwork, DefaultSandboxNetwork), - SandboxMemory: cfg.SandboxMemory, - SandboxCPUs: cfg.SandboxCPUs, - SandboxUser: cfg.SandboxUser, - SandboxEnv: cfg.SandboxEnv, - SandboxVolumes: cfg.SandboxVolumes, - Skills: resolveSkills(cfg.Skills), - Dangerous: resolveDangerous(cfg.Dangerous, true), - Memory: resolveMemory(cfg.Memory), - Guard: resolveGuard(cfg.Guard), - Embedding: cfg.Embedding, + SandboxImage: cfg.SandboxImage, // empty = resolve at call site (Dockerfile.odek or alpine:latest) + SandboxNetwork: ifZero(cfg.SandboxNetwork, DefaultSandboxNetwork), + SandboxMemory: cfg.SandboxMemory, + SandboxCPUs: cfg.SandboxCPUs, + SandboxUser: cfg.SandboxUser, + SandboxEnv: cfg.SandboxEnv, + SandboxVolumes: cfg.SandboxVolumes, + Skills: resolveSkills(cfg.Skills), + Dangerous: resolveDangerous(cfg.Dangerous, true), + Memory: resolveMemory(cfg.Memory), + Guard: resolveGuard(cfg.Guard), + Embedding: cfg.Embedding, MCPServers: cfg.MCPServers, ProjectMCPServerNames: projectMCPNames, ProjectSandboxOverride: projectSandboxOverride, Telegram: resolveTelegram(cfg.Telegram), - Transcription: resolveTranscription(cfg.Transcription), - Vision: resolveVision(cfg.Vision), - WebSearch: resolveWebSearch(cfg.WebSearch), - Schedules: resolveSchedules(cfg.Schedules), - Tools: resolveTools(cfg.Tools), - InteractionMode: ifZero(cfg.InteractionMode, "engaging"), - ToolProgress: ifZero(cfg.ToolProgress, "all"), + Transcription: resolveTranscription(cfg.Transcription), + Vision: resolveVision(cfg.Vision), + WebSearch: resolveWebSearch(cfg.WebSearch), + Schedules: resolveSchedules(cfg.Schedules), + Maintenance: resolveMaintenance(cfg.Maintenance), + Tools: resolveTools(cfg.Tools), + InteractionMode: ifZero(cfg.InteractionMode, "engaging"), + ToolProgress: ifZero(cfg.ToolProgress, "all"), } // Every subsystem inherits the shared top-level embedding default unless it @@ -2114,6 +2248,9 @@ func overlayFile(base, override FileConfig) FileConfig { if override.Memory != nil { base.Memory = override.Memory } + if override.Maintenance != nil { + base.Maintenance = override.Maintenance + } if override.Guard != nil { base.Guard = override.Guard } diff --git a/internal/config/maintenance_test.go b/internal/config/maintenance_test.go new file mode 100644 index 0000000..856a20c --- /dev/null +++ b/internal/config/maintenance_test.go @@ -0,0 +1,200 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/BackendStack21/odek/internal/maintenance" +) + +func TestLoadConfig_MaintenanceDefaults(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + cfg := LoadConfig(CLIFlags{}) + if cfg.Maintenance != maintenance.DefaultConfig() { + t.Errorf("Maintenance = %+v, want defaults %+v", cfg.Maintenance, maintenance.DefaultConfig()) + } +} + +func TestLoadConfig_MaintenanceGlobalFile(t *testing.T) { + dir := t.TempDir() + t.Setenv("HOME", dir) + t.Chdir(dir) + + globalDir := filepath.Join(dir, ".odek") + os.MkdirAll(globalDir, 0755) + if err := os.WriteFile(filepath.Join(globalDir, "config.json"), []byte(`{ + "maintenance": { + "enabled": false, + "interval_minutes": 15, + "sessions_max_age_days": 90, + "audit_max_age_days": 7, + "log_max_mb": 100, + "plans_max_age_days": 60, + "skills_skip_max_age_days": 30 + } + }`), 0644); err != nil { + t.Fatal(err) + } + + cfg := LoadConfig(CLIFlags{}) + want := maintenance.Config{ + Enabled: false, + IntervalMinutes: 15, + SessionsMaxAgeDays: 90, + AuditMaxAgeDays: 7, + LogMaxMB: 100, + PlansMaxAgeDays: 60, + SkillsSkipMaxAgeDays: 30, + } + if cfg.Maintenance != want { + t.Errorf("Maintenance = %+v, want %+v", cfg.Maintenance, want) + } +} + +// TestLoadConfig_MaintenanceExplicitZero verifies that an explicit 0 in the +// file config means "keep forever / disable", not "inherit the default". +func TestLoadConfig_MaintenanceExplicitZero(t *testing.T) { + dir := t.TempDir() + t.Setenv("HOME", dir) + t.Chdir(dir) + + globalDir := filepath.Join(dir, ".odek") + os.MkdirAll(globalDir, 0755) + if err := os.WriteFile(filepath.Join(globalDir, "config.json"), []byte(`{ + "maintenance": {"sessions_max_age_days": 0} + }`), 0644); err != nil { + t.Fatal(err) + } + + cfg := LoadConfig(CLIFlags{}) + if cfg.Maintenance.SessionsMaxAgeDays != 0 { + t.Errorf("SessionsMaxAgeDays = %d, want 0 (explicit keep-forever)", cfg.Maintenance.SessionsMaxAgeDays) + } + // Unset fields still inherit defaults. + if cfg.Maintenance.AuditMaxAgeDays != 14 { + t.Errorf("AuditMaxAgeDays = %d, want default 14", cfg.Maintenance.AuditMaxAgeDays) + } +} + +// TestLoadConfig_MaintenanceProjectIgnored verifies that the project-level +// ./odek.json cannot configure deletion of user data. +func TestLoadConfig_MaintenanceProjectIgnored(t *testing.T) { + dir := t.TempDir() + t.Setenv("HOME", dir) + t.Chdir(dir) + + globalDir := filepath.Join(dir, ".odek") + os.MkdirAll(globalDir, 0755) + if err := os.WriteFile(filepath.Join(globalDir, "config.json"), []byte(`{ + "maintenance": {"sessions_max_age_days": 90} + }`), 0644); err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(dir, "odek.json"), []byte(`{ + "maintenance": {"enabled": true, "sessions_max_age_days": 1, "audit_max_age_days": 1} + }`), 0644); err != nil { + t.Fatal(err) + } + + cfg := LoadConfig(CLIFlags{}) + if cfg.Maintenance.SessionsMaxAgeDays != 90 { + t.Errorf("SessionsMaxAgeDays = %d, want 90 (project maintenance must be ignored)", cfg.Maintenance.SessionsMaxAgeDays) + } + if cfg.Maintenance.AuditMaxAgeDays != 14 { + t.Errorf("AuditMaxAgeDays = %d, want default 14 (project maintenance must be ignored)", cfg.Maintenance.AuditMaxAgeDays) + } +} + +// TestLoadConfig_MaintenanceProjectIgnored_EnvStillOverrides verifies that +// ODEK_MAINTENANCE_* env vars still apply when a project config tries (and +// fails) to set the same section. +func TestLoadConfig_MaintenanceProjectIgnored_EnvStillOverrides(t *testing.T) { + dir := t.TempDir() + t.Setenv("HOME", dir) + t.Chdir(dir) + + if err := os.WriteFile(filepath.Join(dir, "odek.json"), []byte(`{ + "maintenance": {"sessions_max_age_days": 1} + }`), 0644); err != nil { + t.Fatal(err) + } + t.Setenv("ODEK_MAINTENANCE_SESSIONS_MAX_AGE_DAYS", "45") + + cfg := LoadConfig(CLIFlags{}) + if cfg.Maintenance.SessionsMaxAgeDays != 45 { + t.Errorf("SessionsMaxAgeDays = %d, want 45 (env override)", cfg.Maintenance.SessionsMaxAgeDays) + } +} + +func TestLoadConfig_MaintenanceEnvVars(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("ODEK_MAINTENANCE_ENABLED", "false") + t.Setenv("ODEK_MAINTENANCE_INTERVAL_MINUTES", "5") + t.Setenv("ODEK_MAINTENANCE_SESSIONS_MAX_AGE_DAYS", "7") + t.Setenv("ODEK_MAINTENANCE_AUDIT_MAX_AGE_DAYS", "3") + t.Setenv("ODEK_MAINTENANCE_LOG_MAX_MB", "10") + t.Setenv("ODEK_MAINTENANCE_PLANS_MAX_AGE_DAYS", "15") + t.Setenv("ODEK_MAINTENANCE_SKILLS_SKIP_MAX_AGE_DAYS", "20") + + cfg := LoadConfig(CLIFlags{}) + want := maintenance.Config{ + Enabled: false, + IntervalMinutes: 5, + SessionsMaxAgeDays: 7, + AuditMaxAgeDays: 3, + LogMaxMB: 10, + PlansMaxAgeDays: 15, + SkillsSkipMaxAgeDays: 20, + } + if cfg.Maintenance != want { + t.Errorf("Maintenance = %+v, want %+v", cfg.Maintenance, want) + } +} + +// TestLoadConfig_MaintenanceEnvExplicitZero verifies that an env var set to 0 +// is honored (keep forever), not treated as unset. +func TestLoadConfig_MaintenanceEnvExplicitZero(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("ODEK_MAINTENANCE_AUDIT_MAX_AGE_DAYS", "0") + + cfg := LoadConfig(CLIFlags{}) + if cfg.Maintenance.AuditMaxAgeDays != 0 { + t.Errorf("AuditMaxAgeDays = %d, want 0 (explicit env zero)", cfg.Maintenance.AuditMaxAgeDays) + } +} + +// TestEnvIntPtr covers the unset / valid / unparseable branches of the +// ODEK_* integer env helper. +func TestEnvIntPtr(t *testing.T) { + os.Unsetenv("ODEK_TEST_ENV_INT_PTR") + if got := envIntPtr("TEST_ENV_INT_PTR"); got != nil { + t.Errorf("envIntPtr(unset) = %v, want nil", *got) + } + t.Setenv("ODEK_TEST_ENV_INT_PTR", "42") + if got := envIntPtr("TEST_ENV_INT_PTR"); got == nil || *got != 42 { + t.Errorf("envIntPtr(42) = %v, want 42", got) + } + t.Setenv("ODEK_TEST_ENV_INT_PTR", "notanint") + if got := envIntPtr("TEST_ENV_INT_PTR"); got != nil { + t.Errorf("envIntPtr(notanint) = %v, want nil", *got) + } +} + +// TestEnvInt64Ptr covers the unset / valid / unparseable branches of the +// ODEK_* int64 env helper. +func TestEnvInt64Ptr(t *testing.T) { + os.Unsetenv("ODEK_TEST_ENV_INT64_PTR") + if got := envInt64Ptr("TEST_ENV_INT64_PTR"); got != nil { + t.Errorf("envInt64Ptr(unset) = %v, want nil", *got) + } + t.Setenv("ODEK_TEST_ENV_INT64_PTR", "9876543210") + if got := envInt64Ptr("TEST_ENV_INT64_PTR"); got == nil || *got != 9876543210 { + t.Errorf("envInt64Ptr(9876543210) = %v, want 9876543210", got) + } + t.Setenv("ODEK_TEST_ENV_INT64_PTR", "12x") + if got := envInt64Ptr("TEST_ENV_INT64_PTR"); got != nil { + t.Errorf("envInt64Ptr(12x) = %v, want nil", *got) + } +} diff --git a/internal/maintenance/maintenance.go b/internal/maintenance/maintenance.go new file mode 100644 index 0000000..cef73c7 --- /dev/null +++ b/internal/maintenance/maintenance.go @@ -0,0 +1,353 @@ +// Package maintenance provides periodic storage hygiene for the odek home +// directory (~/.odek): session retention, audit-record retention, log +// rotation, Telegram plan/media cleanup, and skill skip-list garbage +// collection. +// +// The janitor is safe to run concurrently with a live agent: every pass is +// idempotent, individual steps are independent (one failing step does not +// abort the rest), and deletions are based on file modification times. +package maintenance + +import ( + "context" + "fmt" + "io/fs" + "os" + "path/filepath" + "time" + + "github.com/BackendStack21/odek/internal/session" + "github.com/BackendStack21/odek/internal/skills" +) + +// mediaMaxAge is the fixed retention for downloaded Telegram media. The +// per-turn telegram.CleanupMedia already uses one hour; the maintenance +// sweep extends the same policy to the per-chat subdirectories it skips. +const mediaMaxAge = time.Hour + +// Config controls the storage-maintenance janitor. +type Config struct { + Enabled bool + IntervalMinutes int // janitor tick; default 60 + SessionsMaxAgeDays int // delete sessions older than this; default 30; 0 = keep forever + AuditMaxAgeDays int // delete audit records older than this; default 14; 0 = keep + LogMaxMB int64 // rotate telegram/schedule logs larger than this; default 50; 0 = no rotation + PlansMaxAgeDays int // delete telegram plans older than this; default 30; 0 = keep + SkillsSkipMaxAgeDays int // GC skill skip-list entries older than this; default 90; 0 = keep +} + +// DefaultConfig returns the out-of-the-box maintenance policy. +func DefaultConfig() Config { + return Config{ + Enabled: true, + IntervalMinutes: 60, + SessionsMaxAgeDays: 30, + AuditMaxAgeDays: 14, + LogMaxMB: 50, + PlansMaxAgeDays: 30, + SkillsSkipMaxAgeDays: 90, + } +} + +// Report summarises what one Sweep pass removed. +type Report struct { + SessionsRemoved int + AuditRemoved int + PlansRemoved int + SkipsRemoved int + MediaFreedBytes int64 + LogsRotated []string +} + +// Sweep runs one full maintenance pass over the odek home dir (e.g. ~/.odek). +// Idempotent, safe to call concurrently with a running agent. +// +// Every step is independent: a failing step records the first error and the +// remaining steps still run, so one corrupt subtree cannot block the others. +func Sweep(ctx context.Context, home string, cfg Config) (Report, error) { + var rep Report + var firstErr error + fail := func(err error) { + if err != nil && firstErr == nil { + firstErr = err + } + } + + if cfg.SessionsMaxAgeDays > 0 { + if err := ctx.Err(); err != nil { + return rep, err + } + n, err := sweepSessions(home, cfg.SessionsMaxAgeDays) + rep.SessionsRemoved = n + fail(err) + } + + if cfg.AuditMaxAgeDays > 0 { + if err := ctx.Err(); err != nil { + return rep, err + } + n, err := sweepAudit(home, cfg.AuditMaxAgeDays) + rep.AuditRemoved = n + fail(err) + } + + if cfg.LogMaxMB > 0 { + if err := ctx.Err(); err != nil { + return rep, err + } + rotated, err := rotateLogs(home, cfg.LogMaxMB) + rep.LogsRotated = rotated + fail(err) + } + + if cfg.PlansMaxAgeDays > 0 { + if err := ctx.Err(); err != nil { + return rep, err + } + n, err := sweepPlans(home, cfg.PlansMaxAgeDays) + rep.PlansRemoved = n + fail(err) + } + + if err := ctx.Err(); err != nil { + return rep, err + } + freed, err := sweepMedia(home) + rep.MediaFreedBytes = freed + fail(err) + + if cfg.SkillsSkipMaxAgeDays > 0 { + if err := ctx.Err(); err != nil { + return rep, err + } + n, err := gcSkipList(home, cfg.SkillsSkipMaxAgeDays) + rep.SkipsRemoved = n + fail(err) + } + + return rep, firstErr +} + +// Start runs Sweep on an interval until ctx is cancelled. It launches a +// background janitor goroutine and returns immediately. The first sweep runs +// after one interval (not immediately, so process startup isn't slowed). +// A disabled config or non-positive interval is a no-op / falls back to the +// default interval respectively. +func Start(ctx context.Context, home string, cfg Config) { + if !cfg.Enabled { + return + } + interval := time.Duration(cfg.IntervalMinutes) * time.Minute + if interval <= 0 { + interval = time.Duration(DefaultConfig().IntervalMinutes) * time.Minute + } + go func() { + ticker := time.NewTicker(interval) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + if _, err := Sweep(ctx, home, cfg); err != nil && ctx.Err() == nil { + fmt.Fprintf(os.Stderr, "odek: maintenance sweep: %v\n", err) + } + } + } + }() +} + +// daysAgo returns the cutoff time for a day-based retention policy. Duration +// arithmetic (instead of AddDate) avoids DST-sensitive behaviour where a +// "day" isn't always 24 hours. +func daysAgo(days int) time.Time { + return time.Now().Add(-time.Duration(days) * 24 * time.Hour) +} + +// sweepSessions deletes sessions older than maxAgeDays via the session +// store's own Cleanup, which also scrubs index.json and the vector index. +func sweepSessions(home string, maxAgeDays int) (int, error) { + store, err := session.NewStoreWithDir(filepath.Join(home, "sessions")) + if err != nil { + return 0, err + } + return store.Cleanup(daysAgo(maxAgeDays)) +} + +// sweepAudit deletes audit records (/sessions/audit/*.json) whose +// modtime is older than maxAgeDays. Contents are never parsed. +func sweepAudit(home string, maxAgeDays int) (int, error) { + dir := filepath.Join(home, "sessions", "audit") + entries, err := os.ReadDir(dir) + if err != nil { + if os.IsNotExist(err) { + return 0, nil + } + return 0, fmt.Errorf("maintenance: read audit dir: %w", err) + } + cutoff := daysAgo(maxAgeDays) + var removed int + for _, e := range entries { + if e.IsDir() || filepath.Ext(e.Name()) != ".json" { + continue + } + info, err := e.Info() + if err != nil { + continue // skip unreadable entries + } + if info.ModTime().Before(cutoff) { + if err := os.Remove(filepath.Join(dir, e.Name())); err != nil { + continue // one bad file shouldn't block the sweep + } + removed++ + } + } + return removed, nil +} + +// rotateLogs rotates /telegram.log (when it exists) and +// /schedule.log when they exceed maxMB: the current log is renamed to +// .1 (replacing any previous generation) and a fresh empty log is +// created. One backup generation only. Returns the rotated log paths. +func rotateLogs(home string, maxMB int64) ([]string, error) { + limit := maxMB << 20 + var rotated []string + for _, name := range []string{"telegram.log", "schedule.log"} { + path := filepath.Join(home, name) + info, err := os.Stat(path) + if err != nil { + if os.IsNotExist(err) { + continue + } + return rotated, fmt.Errorf("maintenance: stat %s: %w", name, err) + } + if info.Size() <= limit { + continue + } + // os.Rename replaces an existing .1 on POSIX filesystems. + if err := os.Rename(path, path+".1"); err != nil { + return rotated, fmt.Errorf("maintenance: rotate %s: %w", name, err) + } + // Recreate an empty log with the same restrictive permissions the + // appenders use, so they keep working on a fresh file. + f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600) + if err != nil { + return rotated, fmt.Errorf("maintenance: truncate %s: %w", name, err) + } + if err := f.Close(); err != nil { + return rotated, fmt.Errorf("maintenance: truncate %s: %w", name, err) + } + rotated = append(rotated, path) + } + return rotated, nil +} + +// sweepPlans deletes Telegram plan files (/plans/**/*.md) older than +// maxAgeDays and removes chat directories left empty afterwards. +func sweepPlans(home string, maxAgeDays int) (int, error) { + root := filepath.Join(home, "plans") + if _, err := os.Stat(root); err != nil { + if os.IsNotExist(err) { + return 0, nil + } + return 0, fmt.Errorf("maintenance: stat plans dir: %w", err) + } + cutoff := daysAgo(maxAgeDays) + var removed int + var dirs []string + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return nil // skip unreadable entries, keep sweeping + } + if d.IsDir() { + if path != root { + dirs = append(dirs, path) + } + return nil + } + if filepath.Ext(d.Name()) != ".md" { + return nil + } + info, err := d.Info() + if err != nil { + return nil + } + if info.ModTime().Before(cutoff) { + if err := os.Remove(path); err == nil { + removed++ + } + } + return nil + }) + if err != nil { + return removed, fmt.Errorf("maintenance: walk plans dir: %w", err) + } + // Remove chat directories emptied by the sweep. os.Remove fails on + // non-empty directories, so this is safe for dirs that still hold plans. + // Deepest first so nested empties collapse in one pass. + for i := len(dirs) - 1; i >= 0; i-- { + _ = os.Remove(dirs[i]) + } + return removed, nil +} + +// sweepMedia deletes downloaded Telegram media files older than mediaMaxAge, +// including the per-chat chat/ subdirectories that the per-turn +// telegram.CleanupMedia skips. Subdirectories themselves are never removed. +// Returns the total number of bytes freed. +func sweepMedia(home string) (int64, error) { + root := filepath.Join(home, "media") + if _, err := os.Stat(root); err != nil { + if os.IsNotExist(err) { + return 0, nil + } + return 0, fmt.Errorf("maintenance: stat media dir: %w", err) + } + cutoff := time.Now().Add(-mediaMaxAge) + var freed int64 + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil || d.IsDir() { + return nil // never remove directories; skip unreadable entries + } + info, err := d.Info() + if err != nil { + return nil + } + if info.ModTime().Before(cutoff) { + if err := os.Remove(path); err == nil { + freed += info.Size() + } + } + return nil + }) + if err != nil { + return freed, fmt.Errorf("maintenance: walk media dir: %w", err) + } + return freed, nil +} + +// gcSkipList removes skill skip-list entries (/skills/.skipped.json) +// whose last skip is older than maxAgeDays. ShouldSkip already treats such +// entries as expired; this just stops the file from growing forever. +func gcSkipList(home string, maxAgeDays int) (int, error) { + dir := filepath.Join(home, "skills") + sl := skills.LoadSkipList(dir) + if len(sl.Skipped) == 0 { + return 0, nil + } + cutoff := daysAgo(maxAgeDays).UTC() + var removed int + for name, e := range sl.Skipped { + if e.SkippedAt.Before(cutoff) { + delete(sl.Skipped, name) + removed++ + } + } + if removed == 0 { + return 0, nil + } + if err := sl.Save(dir); err != nil { + return removed, fmt.Errorf("maintenance: save skip list: %w", err) + } + return removed, nil +} diff --git a/internal/maintenance/maintenance_errors_test.go b/internal/maintenance/maintenance_errors_test.go new file mode 100644 index 0000000..2eba05d --- /dev/null +++ b/internal/maintenance/maintenance_errors_test.go @@ -0,0 +1,437 @@ +// Unreachable-by-design branches (left uncovered intentionally): +// - maintenance.go:237-239 (rotateLogs f.Close error): Close on a freshly +// created O_WRONLY file cannot fail on a local filesystem (only EBADF, +// which cannot occur here since the fd was just opened successfully). +// - maintenance.go:282-284 and 323-325 (sweepPlans/sweepMedia WalkDir error +// returns): dead defensive code — filepath.WalkDir only returns a non-nil +// error when the walk callback returns one, and both callbacks always +// return nil, so the error return can never execute. +package maintenance + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// writeSkipList seeds /skills/.skipped.json with one entry per name. +func writeSkipList(t *testing.T, home string, entries map[string]time.Time) { + t.Helper() + skipped := make(map[string]any, len(entries)) + for name, at := range entries { + skipped[name] = map[string]any{"skipped_at": at, "heuristic": "h", "times_skipped": 1} + } + data, err := json.Marshal(map[string]any{"skipped": skipped}) + if err != nil { + t.Fatal(err) + } + writeFileAt(t, filepath.Join(home, "skills", ".skipped.json"), data, time.Now()) +} + +// skipIfRoot skips permission-based tests when running as root, since root +// bypasses directory permission checks (mirrors the pattern used in +// cmd/odek/batch_patch_audit_test.go). +func skipIfRoot(t *testing.T) { + t.Helper() + if os.Geteuid() == 0 { + t.Skip("root bypasses directory permission checks") + } +} + +// ── Sweep orchestration ──────────────────────────────────────────────── + +// TestSweepStepFailureDoesNotBlockOthers forces the sessions step to fail +// (its directory is a regular file) and verifies the error is reported while +// the remaining steps still run. +func TestSweepStepFailureDoesNotBlockOthers(t *testing.T) { + home := t.TempDir() + // /sessions is a regular file, so NewStoreWithDir cannot MkdirAll it. + if err := os.WriteFile(filepath.Join(home, "sessions"), []byte("x"), 0644); err != nil { + t.Fatal(err) + } + // A stale media file proves the later media step still ran. + writeFileAt(t, filepath.Join(home, "media", "voice_chat1_x.ogg"), []byte("aaaa"), time.Now().Add(-2*time.Hour)) + + cfg := DefaultConfig() + cfg.AuditMaxAgeDays = 0 + cfg.LogMaxMB = 0 + cfg.PlansMaxAgeDays = 0 + cfg.SkillsSkipMaxAgeDays = 0 + + rep, err := Sweep(context.Background(), home, cfg) + if err == nil { + t.Fatal("Sweep should report the sessions step failure") + } + if rep.MediaFreedBytes != 4 { + t.Errorf("MediaFreedBytes = %d, want 4 (later steps must still run)", rep.MediaFreedBytes) + } +} + +// failAfterContext lets the first failAfter Err() calls pass and then reports +// context.Canceled, so tests can stop Sweep at a specific per-step gate. +type failAfterContext struct { + context.Context + failAfter int + calls int +} + +func (c *failAfterContext) Err() error { + c.calls++ + if c.calls > c.failAfter { + return context.Canceled + } + return nil +} + +// TestSweepContextCancelledAtEachGate cancels the context at every per-step +// gate past the first (gate 1 is covered by TestSweepContextCancelled). +func TestSweepContextCancelledAtEachGate(t *testing.T) { + for failAfter := 1; failAfter <= 5; failAfter++ { + ctx := &failAfterContext{Context: context.Background(), failAfter: failAfter} + if _, err := Sweep(ctx, t.TempDir(), DefaultConfig()); err == nil { + t.Errorf("failAfter=%d: Sweep should return the context error", failAfter) + } + } +} + +// ── Start ────────────────────────────────────────────────────────────── + +// TestStartDefaultIntervalFallback covers a non-positive interval falling +// back to the default 60-minute tick. +func TestStartDefaultIntervalFallback(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cfg := DefaultConfig() + cfg.IntervalMinutes = 0 // must fall back to DefaultConfig().IntervalMinutes + Start(ctx, t.TempDir(), cfg) + cancel() // janitor goroutine must exit +} + +// TestStartRunsSweepOnTick waits for the janitor's first tick (the smallest +// configurable interval is 1 minute) and verifies a failing sweep is reported +// on stderr. Slow by necessity — skipped in -short mode. +func TestStartRunsSweepOnTick(t *testing.T) { + if testing.Short() { + t.Skip("requires waiting for the 1-minute janitor tick") + } + home := t.TempDir() + // Force the sweep to fail so the janitor reports it on stderr. + if err := os.WriteFile(filepath.Join(home, "sessions"), []byte("x"), 0644); err != nil { + t.Fatal(err) + } + + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + old := os.Stderr + os.Stderr = w + defer func() { os.Stderr = old }() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + cfg := DefaultConfig() + cfg.IntervalMinutes = 1 // smallest expressible tick + Start(ctx, home, cfg) + + got := make(chan string, 1) + go func() { + buf := make([]byte, 4096) + n, _ := r.Read(buf) + got <- string(buf[:n]) + }() + select { + case msg := <-got: + if !strings.Contains(msg, "maintenance sweep") { + t.Errorf("stderr = %q, want a maintenance sweep error report", msg) + } + case <-time.After(75 * time.Second): + t.Error("janitor did not run a sweep within 75s of Start") + } + cancel() + os.Stderr = old + w.Close() + r.Close() +} + +// ── sweepSessions / sweepAudit ───────────────────────────────────────── + +// TestSweepAuditReadDirError covers a non-NotExist ReadDir failure: the audit +// path exists but is a regular file. +func TestSweepAuditReadDirError(t *testing.T) { + home := t.TempDir() + if err := os.MkdirAll(filepath.Join(home, "sessions"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(home, "sessions", "audit"), []byte("x"), 0644); err != nil { + t.Fatal(err) + } + if _, err := sweepAudit(home, 30); err == nil || !strings.Contains(err.Error(), "read audit dir") { + t.Errorf("sweepAudit error = %v, want a read audit dir error", err) + } +} + +// TestSweepAuditSkipsNonJSONEntries covers the directory/non-.json skip branch. +func TestSweepAuditSkipsNonJSONEntries(t *testing.T) { + home := t.TempDir() + auditDir := filepath.Join(home, "sessions", "audit") + old := time.Now().Add(-30 * 24 * time.Hour) + writeFileAt(t, filepath.Join(auditDir, "old.json"), []byte(`{}`), old) + writeFileAt(t, filepath.Join(auditDir, "notes.txt"), []byte("keep"), old) // non-json untouched + if err := os.MkdirAll(filepath.Join(auditDir, "nested"), 0755); err != nil { + t.Fatal(err) + } + + removed, err := sweepAudit(home, 14) + if err != nil { + t.Fatal(err) + } + if removed != 1 { + t.Errorf("removed = %d, want 1 (only the old .json)", removed) + } + if _, err := os.Stat(filepath.Join(auditDir, "notes.txt")); err != nil { + t.Error("non-json file should have been kept") + } + if _, err := os.Stat(filepath.Join(auditDir, "nested")); err != nil { + t.Error("nested directory should have been kept") + } +} + +// TestSweepAuditInfoError makes an entry's Info() call fail by removing the +// search (execute) bit from the audit directory while keeping it readable. +func TestSweepAuditInfoError(t *testing.T) { + skipIfRoot(t) + home := t.TempDir() + auditDir := filepath.Join(home, "sessions", "audit") + writeFileAt(t, filepath.Join(auditDir, "old.json"), []byte(`{}`), time.Now().Add(-30*24*time.Hour)) + if err := os.Chmod(auditDir, 0400); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(auditDir, 0755) }) + + removed, err := sweepAudit(home, 14) + if err != nil { + t.Fatal(err) + } + if removed != 0 { + t.Errorf("removed = %d, want 0 (unreadable entries are skipped)", removed) + } +} + +// TestSweepAuditRemoveError makes os.Remove fail by removing the write bit +// from the audit directory; the sweep must skip the file, not fail. +func TestSweepAuditRemoveError(t *testing.T) { + skipIfRoot(t) + home := t.TempDir() + auditDir := filepath.Join(home, "sessions", "audit") + writeFileAt(t, filepath.Join(auditDir, "old.json"), []byte(`{}`), time.Now().Add(-30*24*time.Hour)) + if err := os.Chmod(auditDir, 0500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(auditDir, 0755) }) + + removed, err := sweepAudit(home, 14) + if err != nil { + t.Fatal(err) + } + if removed != 0 { + t.Errorf("removed = %d, want 0 (unremovable file is skipped)", removed) + } +} + +// ── rotateLogs ───────────────────────────────────────────────────────── + +// TestRotateLogsStatError covers a non-NotExist Stat failure via a symlink +// loop at the log path. +func TestRotateLogsStatError(t *testing.T) { + home := t.TempDir() + if err := os.Symlink("telegram.log", filepath.Join(home, "telegram.log")); err != nil { + t.Fatal(err) + } + if _, err := rotateLogs(home, 1); err == nil || !strings.Contains(err.Error(), "stat telegram.log") { + t.Errorf("rotateLogs error = %v, want a stat error", err) + } +} + +// TestRotateLogsRenameError makes the rename fail by removing the write bit +// from the home directory. +func TestRotateLogsRenameError(t *testing.T) { + skipIfRoot(t) + home := t.TempDir() + big := make([]byte, 2<<20) + writeFileAt(t, filepath.Join(home, "telegram.log"), big, time.Now()) + if err := os.Chmod(home, 0500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(home, 0755) }) + + if _, err := rotateLogs(home, 1); err == nil || !strings.Contains(err.Error(), "rotate telegram.log") { + t.Errorf("rotateLogs error = %v, want a rotate error", err) + } +} + +// TestRotateLogsRecreateError exhausts the process file-descriptor budget so +// the rename succeeds but recreating the fresh log fails. +func TestRotateLogsRecreateError(t *testing.T) { + home := t.TempDir() + big := make([]byte, 2<<20) + writeFileAt(t, filepath.Join(home, "telegram.log"), big, time.Now()) + + var fds []*os.File + defer func() { + for _, f := range fds { + f.Close() + } + }() + for { + f, err := os.Open(os.DevNull) + if err != nil { + break // budget exhausted (EMFILE) + } + fds = append(fds, f) + } + + _, err := rotateLogs(home, 1) + if err == nil || !strings.Contains(err.Error(), "truncate telegram.log") { + t.Errorf("rotateLogs error = %v, want a truncate error", err) + } + // The rename already happened, so the backup generation must exist. + if _, statErr := os.Stat(filepath.Join(home, "telegram.log.1")); statErr != nil { + t.Errorf("backup telegram.log.1 missing after rename: %v", statErr) + } +} + +// ── sweepPlans / sweepMedia ──────────────────────────────────────────── + +// TestSweepPlansStatError covers a non-NotExist Stat failure via a symlink +// loop at the plans root. +func TestSweepPlansStatError(t *testing.T) { + home := t.TempDir() + if err := os.Symlink("plans", filepath.Join(home, "plans")); err != nil { + t.Fatal(err) + } + if _, err := sweepPlans(home, 30); err == nil || !strings.Contains(err.Error(), "stat plans dir") { + t.Errorf("sweepPlans error = %v, want a stat plans dir error", err) + } +} + +// TestSweepPlansSkipsUnreadableDir makes the walk callback receive an error +// for an unreadable chat directory; the sweep must skip it and continue. +func TestSweepPlansSkipsUnreadableDir(t *testing.T) { + skipIfRoot(t) + home := t.TempDir() + writeFileAt(t, filepath.Join(home, "plans", "chat1", "old.md"), []byte("plan"), time.Now().Add(-60*24*time.Hour)) + if err := os.Chmod(filepath.Join(home, "plans", "chat1"), 0000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(filepath.Join(home, "plans", "chat1"), 0755) }) + + removed, err := sweepPlans(home, 30) + if err != nil { + t.Fatal(err) + } + if removed != 0 { + t.Errorf("removed = %d, want 0 (unreadable dir skipped)", removed) + } +} + +// TestSweepPlansInfoError makes an entry's Info() call fail by removing the +// search bit from the plans root while keeping it readable. +func TestSweepPlansInfoError(t *testing.T) { + skipIfRoot(t) + home := t.TempDir() + plansDir := filepath.Join(home, "plans") + writeFileAt(t, filepath.Join(plansDir, "old.md"), []byte("plan"), time.Now().Add(-60*24*time.Hour)) + if err := os.Chmod(plansDir, 0400); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(plansDir, 0755) }) + + removed, err := sweepPlans(home, 30) + if err != nil { + t.Fatal(err) + } + if removed != 0 { + t.Errorf("removed = %d, want 0 (unreadable entries are skipped)", removed) + } +} + +// TestSweepMediaStatError covers a non-NotExist Stat failure via a symlink +// loop at the media root. +func TestSweepMediaStatError(t *testing.T) { + home := t.TempDir() + if err := os.Symlink("media", filepath.Join(home, "media")); err != nil { + t.Fatal(err) + } + if _, err := sweepMedia(home); err == nil || !strings.Contains(err.Error(), "stat media dir") { + t.Errorf("sweepMedia error = %v, want a stat media dir error", err) + } +} + +// TestSweepMediaInfoError makes an entry's Info() call fail by removing the +// search bit from the media root while keeping it readable. +func TestSweepMediaInfoError(t *testing.T) { + skipIfRoot(t) + home := t.TempDir() + mediaDir := filepath.Join(home, "media") + writeFileAt(t, filepath.Join(mediaDir, "voice_chat1_x.ogg"), []byte("aaaa"), time.Now().Add(-2*time.Hour)) + if err := os.Chmod(mediaDir, 0400); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(mediaDir, 0755) }) + + freed, err := sweepMedia(home) + if err != nil { + t.Fatal(err) + } + if freed != 0 { + t.Errorf("freed = %d, want 0 (unreadable entries are skipped)", freed) + } +} + +// ── gcSkipList ───────────────────────────────────────────────────────── + +// TestGCSkipListKeepsFreshEntries covers the removed == 0 early return with a +// non-empty skip list. +func TestGCSkipListKeepsFreshEntries(t *testing.T) { + home := t.TempDir() + writeSkipList(t, home, map[string]time.Time{"fresh-skill": time.Now().UTC()}) + + removed, err := gcSkipList(home, 90) + if err != nil { + t.Fatal(err) + } + if removed != 0 { + t.Errorf("removed = %d, want 0 (no expired entries)", removed) + } +} + +// TestGCSkipListSaveError makes the skip-list rewrite fail by removing the +// write bit from the skills directory. +func TestGCSkipListSaveError(t *testing.T) { + skipIfRoot(t) + home := t.TempDir() + writeSkipList(t, home, map[string]time.Time{"old-skill": time.Now().Add(-120 * 24 * time.Hour).UTC()}) + skillsDir := filepath.Join(home, "skills") + // WriteFile truncates the existing .skipped.json in place, so the file + // itself must be read-only (dir write permission alone is not checked + // when opening an existing file). + if err := os.Chmod(filepath.Join(skillsDir, ".skipped.json"), 0400); err != nil { + t.Fatal(err) + } + if err := os.Chmod(skillsDir, 0500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(skillsDir, 0755) }) + + removed, err := gcSkipList(home, 90) + if err == nil || !strings.Contains(err.Error(), "save skip list") { + t.Errorf("gcSkipList error = %v, want a save skip list error", err) + } + if removed != 1 { + t.Errorf("removed = %d, want 1 (counted before the save failed)", removed) + } +} diff --git a/internal/maintenance/maintenance_test.go b/internal/maintenance/maintenance_test.go new file mode 100644 index 0000000..1e61de6 --- /dev/null +++ b/internal/maintenance/maintenance_test.go @@ -0,0 +1,454 @@ +package maintenance + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + "time" +) + +// writeFileAt writes content to path (creating parents) and sets its modtime. +func writeFileAt(t *testing.T, path string, content []byte, mod time.Time) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, content, 0644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(path, mod, mod); err != nil { + t.Fatal(err) + } +} + +// writeSessionFixture writes a minimal session JSON file with the given +// UpdatedAt so the store's no-index Cleanup fallback picks it up. +func writeSessionFixture(t *testing.T, home, id string, updatedAt time.Time) { + t.Helper() + sess := map[string]any{ + "id": id, + "created_at": updatedAt, + "updated_at": updatedAt, + "model": "test-model", + "turns": 1, + "task": "fixture", + "messages": []any{}, + } + data, err := json.Marshal(sess) + if err != nil { + t.Fatal(err) + } + writeFileAt(t, filepath.Join(home, "sessions", id+".json"), data, updatedAt) +} + +func TestDefaultConfig(t *testing.T) { + cfg := DefaultConfig() + want := Config{ + Enabled: true, + IntervalMinutes: 60, + SessionsMaxAgeDays: 30, + AuditMaxAgeDays: 14, + LogMaxMB: 50, + PlansMaxAgeDays: 30, + SkillsSkipMaxAgeDays: 90, + } + if cfg != want { + t.Errorf("DefaultConfig() = %+v, want %+v", cfg, want) + } +} + +func TestSweepSessions(t *testing.T) { + old := time.Now().Add(-60 * 24 * time.Hour) + recent := time.Now().Add(-time.Hour) + + tests := []struct { + name string + maxAgeDays int + wantRemoved int + wantKept []string + }{ + {"old session removed, recent kept", 30, 1, []string{"20260201-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"}}, + {"zero keeps everything", 0, 0, []string{"20200101-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "20260201-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + home := t.TempDir() + writeSessionFixture(t, home, "20200101-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", old) + writeSessionFixture(t, home, "20260201-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", recent) + + cfg := DefaultConfig() + cfg.SessionsMaxAgeDays = tc.maxAgeDays + // Isolate the session step from the others. + cfg.AuditMaxAgeDays = 0 + cfg.LogMaxMB = 0 + cfg.PlansMaxAgeDays = 0 + cfg.SkillsSkipMaxAgeDays = 0 + + rep, err := Sweep(context.Background(), home, cfg) + if err != nil { + t.Fatal(err) + } + if rep.SessionsRemoved != tc.wantRemoved { + t.Errorf("SessionsRemoved = %d, want %d", rep.SessionsRemoved, tc.wantRemoved) + } + for _, id := range tc.wantKept { + if _, err := os.Stat(filepath.Join(home, "sessions", id+".json")); err != nil { + t.Errorf("session %s should have been kept: %v", id, err) + } + } + }) + } +} + +func TestSweepSessionsIdempotent(t *testing.T) { + home := t.TempDir() + writeSessionFixture(t, home, "20200101-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", time.Now().Add(-60*24*time.Hour)) + + cfg := DefaultConfig() + cfg.AuditMaxAgeDays = 0 + cfg.LogMaxMB = 0 + cfg.PlansMaxAgeDays = 0 + cfg.SkillsSkipMaxAgeDays = 0 + + rep1, err := Sweep(context.Background(), home, cfg) + if err != nil { + t.Fatal(err) + } + if rep1.SessionsRemoved != 1 { + t.Fatalf("first sweep removed %d, want 1", rep1.SessionsRemoved) + } + rep2, err := Sweep(context.Background(), home, cfg) + if err != nil { + t.Fatal(err) + } + if rep2.SessionsRemoved != 0 { + t.Errorf("second sweep removed %d, want 0 (idempotent)", rep2.SessionsRemoved) + } +} + +func TestSweepAudit(t *testing.T) { + home := t.TempDir() + auditDir := filepath.Join(home, "sessions", "audit") + writeFileAt(t, filepath.Join(auditDir, "old.json"), []byte(`{}`), time.Now().Add(-30*24*time.Hour)) + writeFileAt(t, filepath.Join(auditDir, "new.json"), []byte(`{}`), time.Now()) + + cfg := DefaultConfig() + cfg.SessionsMaxAgeDays = 0 + cfg.LogMaxMB = 0 + cfg.PlansMaxAgeDays = 0 + cfg.SkillsSkipMaxAgeDays = 0 + + rep, err := Sweep(context.Background(), home, cfg) + if err != nil { + t.Fatal(err) + } + if rep.AuditRemoved != 1 { + t.Errorf("AuditRemoved = %d, want 1", rep.AuditRemoved) + } + if _, err := os.Stat(filepath.Join(auditDir, "old.json")); !os.IsNotExist(err) { + t.Error("old audit record should have been deleted") + } + if _, err := os.Stat(filepath.Join(auditDir, "new.json")); err != nil { + t.Error("recent audit record should have been kept") + } +} + +func TestSweepAuditDisabledAndMissing(t *testing.T) { + home := t.TempDir() + auditDir := filepath.Join(home, "sessions", "audit") + writeFileAt(t, filepath.Join(auditDir, "old.json"), []byte(`{}`), time.Now().Add(-30*24*time.Hour)) + + cfg := DefaultConfig() + cfg.AuditMaxAgeDays = 0 // keep forever + cfg.SessionsMaxAgeDays = 0 + cfg.LogMaxMB = 0 + cfg.PlansMaxAgeDays = 0 + cfg.SkillsSkipMaxAgeDays = 0 + + rep, err := Sweep(context.Background(), home, cfg) + if err != nil { + t.Fatal(err) + } + if rep.AuditRemoved != 0 { + t.Errorf("AuditRemoved = %d, want 0 (disabled)", rep.AuditRemoved) + } + + // Missing audit dir entirely: no error, no removals. + rep, err = Sweep(context.Background(), t.TempDir(), DefaultConfig()) + if err != nil { + t.Fatal(err) + } + if rep.AuditRemoved != 0 { + t.Errorf("AuditRemoved = %d, want 0 (missing dir)", rep.AuditRemoved) + } +} + +func TestRotateLogs(t *testing.T) { + big := make([]byte, 2<<20) // 2 MiB + for i := range big { + big[i] = 'x' + } + + tests := []struct { + name string + logMaxMB int64 + telegramLog []byte // nil = file absent + scheduleLog []byte + staleBackup bool // pre-create telegram.log.1 to verify replacement + wantRotated int + }{ + {"oversized telegram.log rotates", 1, big, nil, false, 1}, + {"small logs untouched", 50, []byte("small"), []byte("small"), false, 0}, + {"existing .1 replaced", 1, big, nil, true, 1}, + {"both logs rotate", 1, big, big, false, 2}, + {"zero disables rotation", 0, big, big, false, 0}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + home := t.TempDir() + now := time.Now() + if tc.telegramLog != nil { + writeFileAt(t, filepath.Join(home, "telegram.log"), tc.telegramLog, now) + } + if tc.scheduleLog != nil { + writeFileAt(t, filepath.Join(home, "schedule.log"), tc.scheduleLog, now) + } + if tc.staleBackup { + writeFileAt(t, filepath.Join(home, "telegram.log.1"), []byte("stale"), now) + } + + cfg := DefaultConfig() + cfg.LogMaxMB = tc.logMaxMB + cfg.SessionsMaxAgeDays = 0 + cfg.AuditMaxAgeDays = 0 + cfg.PlansMaxAgeDays = 0 + cfg.SkillsSkipMaxAgeDays = 0 + + rep, err := Sweep(context.Background(), home, cfg) + if err != nil { + t.Fatal(err) + } + if len(rep.LogsRotated) != tc.wantRotated { + t.Fatalf("LogsRotated = %v, want %d entries", rep.LogsRotated, tc.wantRotated) + } + for _, path := range rep.LogsRotated { + info, err := os.Stat(path) + if err != nil { + t.Fatalf("rotated log %s missing: %v", path, err) + } + if info.Size() != 0 { + t.Errorf("rotated log %s not truncated: size %d", path, info.Size()) + } + backup, err := os.ReadFile(path + ".1") + if err != nil { + t.Fatalf("backup %s.1 missing: %v", path, err) + } + if len(backup) != len(big) { + t.Errorf("backup %s.1 size = %d, want %d", path, len(backup), len(big)) + } + } + if tc.staleBackup { + backup, _ := os.ReadFile(filepath.Join(home, "telegram.log.1")) + if string(backup) == "stale" { + t.Error("stale .1 backup should have been replaced") + } + } + }) + } +} + +func TestSweepPlans(t *testing.T) { + home := t.TempDir() + old := time.Now().Add(-60 * 24 * time.Hour) + recent := time.Now() + writeFileAt(t, filepath.Join(home, "plans", "chat1", "old.md"), []byte("plan"), old) + writeFileAt(t, filepath.Join(home, "plans", "chat2", "new.md"), []byte("plan"), recent) + writeFileAt(t, filepath.Join(home, "plans", "chat2", "notes.txt"), []byte("keep"), old) // non-md untouched + + cfg := DefaultConfig() + cfg.SessionsMaxAgeDays = 0 + cfg.AuditMaxAgeDays = 0 + cfg.LogMaxMB = 0 + cfg.SkillsSkipMaxAgeDays = 0 + + rep, err := Sweep(context.Background(), home, cfg) + if err != nil { + t.Fatal(err) + } + if rep.PlansRemoved != 1 { + t.Errorf("PlansRemoved = %d, want 1", rep.PlansRemoved) + } + if _, err := os.Stat(filepath.Join(home, "plans", "chat1")); !os.IsNotExist(err) { + t.Error("emptied chat dir should have been removed") + } + if _, err := os.Stat(filepath.Join(home, "plans", "chat2", "new.md")); err != nil { + t.Error("recent plan should have been kept") + } + if _, err := os.Stat(filepath.Join(home, "plans", "chat2", "notes.txt")); err != nil { + t.Error("non-markdown file should have been kept") + } +} + +func TestSweepPlansDisabled(t *testing.T) { + home := t.TempDir() + writeFileAt(t, filepath.Join(home, "plans", "chat1", "old.md"), []byte("plan"), time.Now().Add(-60*24*time.Hour)) + + cfg := DefaultConfig() + cfg.PlansMaxAgeDays = 0 + cfg.SessionsMaxAgeDays = 0 + cfg.AuditMaxAgeDays = 0 + cfg.LogMaxMB = 0 + cfg.SkillsSkipMaxAgeDays = 0 + + rep, err := Sweep(context.Background(), home, cfg) + if err != nil { + t.Fatal(err) + } + if rep.PlansRemoved != 0 { + t.Errorf("PlansRemoved = %d, want 0 (disabled)", rep.PlansRemoved) + } +} + +func TestSweepMedia(t *testing.T) { + home := t.TempDir() + old := time.Now().Add(-2 * time.Hour) + recent := time.Now() + writeFileAt(t, filepath.Join(home, "media", "voice_chat1_x.ogg"), []byte("aaaa"), old) + writeFileAt(t, filepath.Join(home, "media", "chat1", "photo.jpg"), []byte("bb"), old) + writeFileAt(t, filepath.Join(home, "media", "chat1", "recent.jpg"), []byte("cc"), recent) + + cfg := DefaultConfig() + cfg.SessionsMaxAgeDays = 0 + cfg.AuditMaxAgeDays = 0 + cfg.LogMaxMB = 0 + cfg.PlansMaxAgeDays = 0 + cfg.SkillsSkipMaxAgeDays = 0 + + rep, err := Sweep(context.Background(), home, cfg) + if err != nil { + t.Fatal(err) + } + if rep.MediaFreedBytes != 6 { + t.Errorf("MediaFreedBytes = %d, want 6", rep.MediaFreedBytes) + } + // The per-chat subdirectory itself must survive. + info, err := os.Stat(filepath.Join(home, "media", "chat1")) + if err != nil || !info.IsDir() { + t.Error("chat subdirectory should never be removed") + } + if _, err := os.Stat(filepath.Join(home, "media", "chat1", "recent.jpg")); err != nil { + t.Error("recent media should have been kept") + } +} + +func TestGCSkipList(t *testing.T) { + old := time.Now().Add(-120 * 24 * time.Hour).UTC() + recent := time.Now().UTC() + + tests := []struct { + name string + maxAgeDays int + wantRemoved int + wantKept []string + }{ + {"expired entries removed", 90, 1, []string{"new-skill"}}, + {"zero keeps everything", 0, 0, []string{"old-skill", "new-skill"}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + home := t.TempDir() + skipPath := filepath.Join(home, "skills", ".skipped.json") + data, err := json.Marshal(map[string]any{ + "skipped": map[string]any{ + "old-skill": map[string]any{"skipped_at": old, "heuristic": "h", "times_skipped": 3}, + "new-skill": map[string]any{"skipped_at": recent, "heuristic": "h", "times_skipped": 1}, + }, + }) + if err != nil { + t.Fatal(err) + } + writeFileAt(t, skipPath, data, recent) + + cfg := DefaultConfig() + cfg.SkillsSkipMaxAgeDays = tc.maxAgeDays + cfg.SessionsMaxAgeDays = 0 + cfg.AuditMaxAgeDays = 0 + cfg.LogMaxMB = 0 + cfg.PlansMaxAgeDays = 0 + + rep, err := Sweep(context.Background(), home, cfg) + if err != nil { + t.Fatal(err) + } + if rep.SkipsRemoved != tc.wantRemoved { + t.Errorf("SkipsRemoved = %d, want %d", rep.SkipsRemoved, tc.wantRemoved) + } + + raw, err := os.ReadFile(skipPath) + if err != nil { + t.Fatal(err) + } + var parsed struct { + Skipped map[string]json.RawMessage `json:"skipped"` + } + if err := json.Unmarshal(raw, &parsed); err != nil { + t.Fatal(err) + } + if len(parsed.Skipped) != len(tc.wantKept) { + t.Errorf("skip list has %d entries, want %d", len(parsed.Skipped), len(tc.wantKept)) + } + for _, name := range tc.wantKept { + if _, ok := parsed.Skipped[name]; !ok { + t.Errorf("entry %q should have been kept", name) + } + } + }) + } +} + +func TestGCSkipListMissingFile(t *testing.T) { + rep, err := Sweep(context.Background(), t.TempDir(), DefaultConfig()) + if err != nil { + t.Fatal(err) + } + if rep.SkipsRemoved != 0 { + t.Errorf("SkipsRemoved = %d, want 0 (missing file)", rep.SkipsRemoved) + } +} + +func TestSweepEmptyHome(t *testing.T) { + rep, err := Sweep(context.Background(), t.TempDir(), DefaultConfig()) + if err != nil { + t.Fatal(err) + } + if rep.SessionsRemoved != 0 || rep.AuditRemoved != 0 || rep.PlansRemoved != 0 || + rep.SkipsRemoved != 0 || rep.MediaFreedBytes != 0 || len(rep.LogsRotated) != 0 { + t.Errorf("empty home should produce a zero report, got %+v", rep) + } +} + +func TestSweepContextCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err := Sweep(ctx, t.TempDir(), DefaultConfig()) + if err == nil { + t.Error("Sweep with cancelled context should return an error") + } +} + +func TestStartDisabled(t *testing.T) { + cfg := DefaultConfig() + cfg.Enabled = false + // Must return immediately without launching a sweep. + Start(context.Background(), t.TempDir(), cfg) +} + +func TestStartStopsOnCancel(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cfg := DefaultConfig() + cfg.IntervalMinutes = 1 + Start(ctx, t.TempDir(), cfg) + cancel() // janitor goroutine must exit (verified by -race leak detectors) +} diff --git a/internal/session/session.go b/internal/session/session.go index c5bfad5..bb08e93 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -35,7 +35,9 @@ import ( // MaxSessionFileBytes caps the on-disk size of a session file that Load will // read into memory. This prevents a tampered or corrupted multi-gigabyte // session file from causing an OOM when any caller loads it. -const MaxSessionFileBytes = 32 * 1024 * 1024 // 32 MiB +// It is a var, not a const, so tests can temporarily shrink it instead of +// building multi-MiB fixtures — production code should treat it as fixed. +var MaxSessionFileBytes = 32 * 1024 * 1024 // 32 MiB // ── Types ────────────────────────────────────────────────────────────── @@ -52,6 +54,14 @@ type Session struct { Sandbox bool `json:"sandbox"` // was sandboxed — auto-apply on resume Messages []llm.Message `json:"messages"` // full conversation history Buffer []string `json:"buffer,omitempty"` // last N turn summaries (memory tier 2) + + // RedactBoundary records how many leading messages have already been + // secret-redacted by a previous save. Redacting the full transcript on + // every save is O(history) per write — O(n²) over a session's life — + // and dominates save time on long sessions (20+ regexes over tens of + // MB). Sessions are append-only, so only messages at or beyond the + // boundary need scanning. Old files default to 0 (= redact all once). + RedactBoundary int `json:"redact_boundary,omitempty"` } // ── Store ────────────────────────────────────────────────────────────── @@ -62,6 +72,11 @@ type Store struct { dir string // e.g. /home/user/.odek/sessions/ mu sync.Mutex + // trimWarned records session IDs for which the write-path size-cap trim + // warning has already been emitted, so the warning fires once per session + // per process instead of on every Append of an oversized session. + trimWarned map[string]struct{} + // Vec is the optional semantic search index. When non-nil, every // Save/Delete/Cleanup call updates the vector index automatically. // Call InitVectorIndex() to initialize. @@ -75,7 +90,14 @@ func NewStore() (*Store, error) { if err != nil { return nil, fmt.Errorf("session: home dir: %w", err) } - dir := filepath.Join(home, ".odek", "sessions") + return NewStoreWithDir(filepath.Join(home, ".odek", "sessions")) +} + +// NewStoreWithDir creates a session store rooted at the given directory. +// The directory is created if it doesn't exist. Used by subsystems (e.g. +// storage maintenance) that operate on an explicit home directory rather +// than the current user's default. +func NewStoreWithDir(dir string) (*Store, error) { if err := os.MkdirAll(dir, 0755); err != nil { return nil, fmt.Errorf("session: create dir: %w", err) } @@ -343,13 +365,22 @@ func (s *Store) saveLocked(sess *Session) error { return fmt.Errorf("session: refusing unsafe save: %w", err) } - // Redact secrets from all messages and the task label before writing to - // disk. This is defense-in-depth: the loop engine already redacts tool - // outputs, but this catches any secrets that slipped through - // (e.g. LLM hallucinations, direct API usage, or the first user prompt - // stored as the session title). + // Redact secrets before writing to disk. This is defense-in-depth: the + // loop engine already redacts tool outputs, but this catches any secrets + // that slipped through (e.g. LLM hallucinations, direct API usage, or + // the first user prompt stored as the session title). Sessions are + // append-only, so only messages at or beyond sess.RedactBoundary are + // scanned — messages already redacted by a previous save are not + // re-scanned (see the field comment for the O(n²) rationale). sess.Task = redact.RedactSecrets(sess.Task) - for i := range sess.Messages { + boundary := sess.RedactBoundary + if boundary < 0 { + boundary = 0 + } + if boundary > len(sess.Messages) { + boundary = len(sess.Messages) + } + for i := boundary; i < len(sess.Messages); i++ { sess.Messages[i].Content = redact.RedactSecrets(sess.Messages[i].Content) sess.Messages[i].ReasoningContent = redact.RedactSecrets(sess.Messages[i].ReasoningContent) } @@ -359,6 +390,35 @@ func (s *Store) saveLocked(sess *Session) error { return fmt.Errorf("session: marshal: %w", err) } + // Write-path size cap: MaxSessionFileBytes is enforced at Load, so a + // session allowed to grow past it on disk would become unloadable. Trim + // the oldest message groups (keeping the system message at index 0 and + // the most recent turns, mirroring the loop's trim semantics) until the + // serialized form fits. + if len(data) > MaxSessionFileBytes { + // The trim's own marshaled form is discarded: the final marshal + // below recomputes it after RedactBoundary is updated. + if _, err = s.trimToFileCapLocked(sess, data); err != nil { + return err + } + if s.trimWarned == nil { + s.trimWarned = make(map[string]struct{}) + } + if _, ok := s.trimWarned[sess.ID]; !ok { + s.trimWarned[sess.ID] = struct{}{} + fmt.Fprintf(os.Stderr, "odek: warning: session %s exceeded %d bytes on write — oldest messages trimmed to stay within the load cap\n", sess.ID, MaxSessionFileBytes) + } + } + // Every surviving message is now redacted: those before the boundary by + // earlier saves, the rest just now — and trimming only removes messages, + // so the boundary is simply the surviving count. Set it before the final + // marshal so it is actually persisted. + sess.RedactBoundary = len(sess.Messages) + data, err = json.Marshal(sess) + if err != nil { + return fmt.Errorf("session: marshal: %w", err) + } + if err := fsatomic.WriteFile(s.path(sess.ID), data, 0600); err != nil { return fmt.Errorf("session: write: %w", err) } @@ -377,6 +437,60 @@ func (s *Store) saveLocked(sess *Session) error { return nil } +// trimToFileCapLocked drops the oldest message groups from sess until its +// serialized form fits within MaxSessionFileBytes, returning the trimmed +// JSON. Caller must hold s.mu. +// +// Group semantics mirror the loop's context trimming: the system message at +// index 0 is always kept, and an assistant tool_calls message is dropped +// together with its following tool-result messages so a stored transcript +// never contains orphaned tool messages (which strict providers reject). +// The turn count is recounted to match the surviving messages. If nothing +// droppable remains (a degenerate case, e.g. a single oversized system +// message), the session is written as-is — failing the save would lose data. +func (s *Store) trimToFileCapLocked(sess *Session, data []byte) ([]byte, error) { + for len(data) > MaxSessionFileBytes { + start := 0 + if len(sess.Messages) > 0 && sess.Messages[0].Role == "system" { + start = 1 // keep system + } + if start >= len(sess.Messages) { + break // nothing left to drop + } + // Drop enough oldest groups in ONE pass to get back under the cap. + // Dropping a single group per re-marshal is O(n²) for long + // transcripts (thousands of messages × full-session marshal) and + // effectively hangs on oversized fixtures. Each pass drops at least + // one group, so the outer loop still terminates; groups are + // marshaled once each to size them exactly. + excess := len(data) - MaxSessionFileBytes + freed := 0 + dropEnd := start + for dropEnd < len(sess.Messages) && freed <= excess { + groupEnd := dropEnd + 1 + if sess.Messages[dropEnd].Role == "assistant" && len(sess.Messages[dropEnd].ToolCalls) > 0 { + for groupEnd < len(sess.Messages) && sess.Messages[groupEnd].Role == "tool" { + groupEnd++ + } + } + groupJSON, err := json.Marshal(sess.Messages[dropEnd:groupEnd]) + if err != nil { + return nil, fmt.Errorf("session: marshal trim candidate: %w", err) + } + freed += len(groupJSON) + dropEnd = groupEnd + } + sess.Messages = append(sess.Messages[:start], sess.Messages[dropEnd:]...) + sess.Turns = countUserTurns(sess.Messages) + var err error + data, err = json.Marshal(sess) + if err != nil { + return nil, fmt.Errorf("session: marshal after trim: %w", err) + } + } + return data, nil +} + // Load reads a session from disk by ID. Returns an error if the file // doesn't exist or can't be parsed. func (s *Store) Load(id string) (*Session, error) { @@ -387,7 +501,7 @@ func (s *Store) Load(id string) (*Session, error) { if err != nil { return nil, fmt.Errorf("session: load %q: %w", id, err) } - if info.Size() > MaxSessionFileBytes { + if info.Size() > int64(MaxSessionFileBytes) { return nil, fmt.Errorf("session: load %q: file too large (%d bytes, max %d)", id, info.Size(), MaxSessionFileBytes) } data, err := os.ReadFile(s.path(id)) diff --git a/internal/session/session_savecap_test.go b/internal/session/session_savecap_test.go new file mode 100644 index 0000000..2754265 --- /dev/null +++ b/internal/session/session_savecap_test.go @@ -0,0 +1,166 @@ +package session + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/BackendStack21/odek/internal/llm" +) + +// Tests for the write-path size-cap wiring in saveLocked and for +// trimToFileCapLocked edge cases that do not require multi-MiB fixtures (the +// trimmer is called directly with synthetic oversized payloads). +// +// Unreachable-by-design branches (left uncovered intentionally): +// - session.go:370-372, 448-450, 458-460 (json.Marshal error returns) and +// 381-383 (the trim-error propagation in saveLocked): every field of +// Session and llm.Message is a concrete JSON-marshalable type (strings, +// ints, bools, time.Time, nested structs), so json.Marshal cannot fail +// for these values; the error branches are dead defensive code and the +// trim-error branch therefore cannot fire either. + +// TestSave_IndexWriteError covers the saveIndexLocked error path in +// saveLocked: the session file write succeeds, but the atomic index rename +// fails because a directory sits at the index path. +func TestSave_IndexWriteError(t *testing.T) { + dir := t.TempDir() + store, err := NewStoreWithDir(dir) + if err != nil { + t.Fatalf("NewStoreWithDir() error: %v", err) + } + if err := os.Mkdir(filepath.Join(dir, "index.json"), 0755); err != nil { + t.Fatal(err) + } + + sess := &Session{ + ID: "20260101-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + Messages: []llm.Message{{Role: "user", Content: "hi"}}, + } + if err := store.Save(sess); err == nil || !strings.Contains(err.Error(), "write index") { + t.Errorf("Save() error = %v, want a write index error", err) + } +} + +// withFileCap temporarily shrinks MaxSessionFileBytes so cap-trimming tests +// can use small fixtures instead of multi-MiB transcripts (which made the +// session suite time out in CI). +func withFileCap(t *testing.T, n int) { + t.Helper() + orig := MaxSessionFileBytes + MaxSessionFileBytes = n + t.Cleanup(func() { MaxSessionFileBytes = orig }) +} + +// TestTrimToFileCap_NothingDroppable covers the degenerate break branch: a +// single system message with nothing left to drop is returned as-is. +func TestTrimToFileCap_NothingDroppable(t *testing.T) { + withFileCap(t, 64<<10) + store, err := NewStoreWithDir(t.TempDir()) + if err != nil { + t.Fatalf("NewStoreWithDir() error: %v", err) + } + sess := &Session{ + ID: "20260101-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + Messages: []llm.Message{{Role: "system", Content: "you are odek"}}, + } + oversized := make([]byte, MaxSessionFileBytes+1) + data, err := store.trimToFileCapLocked(sess, oversized) + if err != nil { + t.Fatalf("trimToFileCapLocked() error: %v", err) + } + if len(data) != len(oversized) { + t.Errorf("len(data) = %d, want %d (nothing droppable — returned as-is)", len(data), len(oversized)) + } + if len(sess.Messages) != 1 || sess.Messages[0].Role != "system" { + t.Errorf("system message should have been kept: %+v", sess.Messages) + } +} + +// TestTrimToFileCap_DropsToolGroups exercises the group-sizing pass directly: +// an assistant tool_calls message is dropped together with its tool results, +// and the trim ends once the re-marshaled session fits. +func TestTrimToFileCap_DropsToolGroups(t *testing.T) { + withFileCap(t, 64<<10) + store, err := NewStoreWithDir(t.TempDir()) + if err != nil { + t.Fatalf("NewStoreWithDir() error: %v", err) + } + call := llm.Message{Role: "assistant", Content: "calling tools"} + call.ToolCalls = append(call.ToolCalls, llm.ToolCall{ID: "c1", Type: "function"}) + sess := &Session{ + ID: "20260101-cccccccccccccccccccccccccccccccc", + Messages: []llm.Message{ + {Role: "system", Content: "you are odek"}, + call, + {Role: "tool", Name: "shell", Content: "result 1"}, + {Role: "tool", Name: "shell", Content: "result 2"}, + {Role: "user", Content: "next question"}, + }, + } + // Synthetic oversized payload: the excess is 1 byte, so one pass drops + // exactly the first group (the assistant tool_calls message together + // with its tool results) and the re-marshal fits. + oversized := make([]byte, MaxSessionFileBytes+1) + data, err := store.trimToFileCapLocked(sess, oversized) + if err != nil { + t.Fatalf("trimToFileCapLocked() error: %v", err) + } + if len(data) > MaxSessionFileBytes { + t.Errorf("len(data) = %d, exceeds cap %d", len(data), MaxSessionFileBytes) + } + if len(sess.Messages) != 2 || sess.Messages[0].Role != "system" || sess.Messages[1].Role != "user" { + t.Errorf("system + trailing user message should survive: %+v", sess.Messages) + } + for _, m := range sess.Messages { + if m.Role == "tool" { + t.Error("trimmed transcript must not contain orphaned tool messages") + } + } +} + +// TestSave_IncrementalRedaction verifies secrets are redacted only in +// messages at or beyond the persisted RedactBoundary: the first save redacts +// everything, later saves scan only newly appended messages. +func TestSave_IncrementalRedaction(t *testing.T) { + store, err := NewStoreWithDir(t.TempDir()) + if err != nil { + t.Fatalf("NewStoreWithDir() error: %v", err) + } + secret1 := "sk-" + strings.Repeat("a1", 20) + secret2 := "sk-" + strings.Repeat("b2", 20) + + sess, err := store.Create([]llm.Message{ + {Role: "system", Content: "you are odek"}, + {Role: "user", Content: "here is my key " + secret1}, + }, "test", "redact boundary") + if err != nil { + t.Fatalf("Create() error: %v", err) + } + loaded, err := store.Load(sess.ID) + if err != nil { + t.Fatalf("Load() error: %v", err) + } + if !strings.Contains(loaded.Messages[1].Content, "[REDACTED]") { + t.Errorf("secret in first save not redacted: %q", loaded.Messages[1].Content) + } + if loaded.RedactBoundary != len(loaded.Messages) { + t.Errorf("RedactBoundary = %d, want %d after first save", loaded.RedactBoundary, len(loaded.Messages)) + } + + loaded.Messages = append(loaded.Messages, llm.Message{Role: "assistant", Content: "try " + secret2}) + if err := store.Save(loaded); err != nil { + t.Fatalf("Save() error: %v", err) + } + reloaded, err := store.Load(sess.ID) + if err != nil { + t.Fatalf("Load() after second save: %v", err) + } + if !strings.Contains(reloaded.Messages[2].Content, "[REDACTED]") { + t.Errorf("secret in appended message not redacted: %q", reloaded.Messages[2].Content) + } + if reloaded.RedactBoundary != len(reloaded.Messages) { + t.Errorf("RedactBoundary = %d, want %d after second save", reloaded.RedactBoundary, len(reloaded.Messages)) + } +} diff --git a/internal/session/session_test.go b/internal/session/session_test.go index 128e127..a1bfe05 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -1282,3 +1282,134 @@ func TestBuildConversationText(t *testing.T) { t.Error("tool messages should be excluded") } } + +// ── Write-path size cap ──────────────────────────────────────────────── + +// TestSave_TrimsOversizedSession forces a session past MaxSessionFileBytes and +// verifies the write path trims the oldest messages so the file stays under +// the cap and remains Load-able. The system message at index 0 and the +// trailing messages must survive; the oldest turns are dropped. +func TestSave_TrimsOversizedSession(t *testing.T) { + store := newTestStore(t) + withFileCap(t, 64<<10) + + // 20 KiB per message × 4 messages ≈ 80 KiB serialized — past the 64 KiB + // test cap. Few large messages keep the trim loop's re-marshal cycles + // cheap; the small cap (see withFileCap) keeps this fast in CI. + big := strings.Repeat("x", 20<<10) + msgs := []llm.Message{{Role: "system", Content: "you are odek"}} + for i := 0; i < 4; i++ { + role := "user" + if i%2 == 1 { + role = "assistant" + } + msgs = append(msgs, llm.Message{Role: role, Content: big}) + } + // Distinct trailing messages we expect to survive the trim. + msgs = append(msgs, + llm.Message{Role: "user", Content: "final question"}, + llm.Message{Role: "assistant", Content: "final answer"}, + ) + + sess, err := store.Create(msgs, "test", "oversized session") + if err != nil { + t.Fatalf("Create() error: %v", err) + } + + info, err := os.Stat(store.Path(sess.ID)) + if err != nil { + t.Fatalf("stat session file: %v", err) + } + if info.Size() > int64(MaxSessionFileBytes) { + t.Errorf("session file size = %d, exceeds MaxSessionFileBytes %d", info.Size(), MaxSessionFileBytes) + } + + loaded, err := store.Load(sess.ID) + if err != nil { + t.Fatalf("Load() after trim: %v", err) + } + if loaded.Messages[0].Role != "system" || loaded.Messages[0].Content != "you are odek" { + t.Errorf("system message not preserved at index 0: %+v", loaded.Messages[0]) + } + last := loaded.Messages[len(loaded.Messages)-1] + if last.Role != "assistant" || last.Content != "final answer" { + t.Errorf("trailing message not preserved: role=%q content=%q", last.Role, last.Content) + } + if len(loaded.Messages) >= len(msgs) { + t.Errorf("expected oldest messages to be dropped, still have %d of %d", len(loaded.Messages), len(msgs)) + } + if got, want := loaded.Turns, countUserTurns(loaded.Messages); got != want { + t.Errorf("Turns = %d, want %d (recounted after trim)", got, want) + } + + // A second save of the same session must not warn again (once-per-session + // warning) and must not error — covered implicitly by Save succeeding. + if err := store.Save(loaded); err != nil { + t.Fatalf("second Save() error: %v", err) + } +} + +// TestSave_TrimKeepsToolGroupsIntact verifies the write-path trim drops an +// assistant tool_calls message together with its tool results, so a stored +// transcript never starts with an orphaned tool message. +func TestSave_TrimKeepsToolGroupsIntact(t *testing.T) { + store := newTestStore(t) + withFileCap(t, 64<<10) + + // 6 × 12 KiB ≈ 72 KiB — past the 64 KiB test cap. + big := strings.Repeat("y", 12<<10) + msgs := []llm.Message{{Role: "system", Content: "you are odek"}} + for i := 0; i < 3; i++ { + call := llm.Message{Role: "assistant", Content: big} + call.ToolCalls = append(call.ToolCalls, llm.ToolCall{ID: "c1", Type: "function"}) + call.ToolCalls[0].Function.Name = "shell" + msgs = append(msgs, + call, + llm.Message{Role: "tool", Name: "shell", ToolCallID: "c1", Content: big}, + ) + } + msgs = append(msgs, llm.Message{Role: "user", Content: "latest task"}) + + sess, err := store.Create(msgs, "test", "tool-group trim") + if err != nil { + t.Fatalf("Create() error: %v", err) + } + loaded, err := store.Load(sess.ID) + if err != nil { + t.Fatalf("Load() after trim: %v", err) + } + // Every surviving tool message must be preceded by an assistant message + // carrying tool calls. + for i, m := range loaded.Messages { + if m.Role == "tool" { + if i == 0 || loaded.Messages[i-1].Role == "system" || (loaded.Messages[i-1].Role == "assistant" && len(loaded.Messages[i-1].ToolCalls) == 0) { + t.Errorf("orphaned tool message at index %d", i) + } + } + } +} + +// TestSave_SmallSessionUntouched verifies sessions well under the cap are +// written verbatim — no trimming, no warning bookkeeping. +func TestSave_SmallSessionUntouched(t *testing.T) { + store := newTestStore(t) + msgs := []llm.Message{ + {Role: "system", Content: "you are odek"}, + {Role: "user", Content: "hello"}, + {Role: "assistant", Content: "hi there"}, + } + sess, err := store.Create(msgs, "test", "small session") + if err != nil { + t.Fatalf("Create() error: %v", err) + } + loaded, err := store.Load(sess.ID) + if err != nil { + t.Fatalf("Load() error: %v", err) + } + if len(loaded.Messages) != len(msgs) { + t.Errorf("small session trimmed: got %d messages, want %d", len(loaded.Messages), len(msgs)) + } + if len(store.trimWarned) != 0 { + t.Errorf("trim warning recorded for a small session: %v", store.trimWarned) + } +}