diff --git a/internal/cmd/helpcmd/command.go b/internal/cmd/helpcmd/command.go index 38fd9e5..8f45d32 100644 --- a/internal/cmd/helpcmd/command.go +++ b/internal/cmd/helpcmd/command.go @@ -161,8 +161,9 @@ func buildManifest() manifest { {Command: "retask sandbox session update", Description: "Partial update a session", Flags: []string{"--name", "--seed-nrn", "--seed-prompt"}, Example: "retask sandbox session update --name \"My Session\""}, {Command: "retask sandbox session stop", Description: "Stop a session", Example: "retask sandbox session stop "}, {Command: "retask sandbox session delete", Description: "Delete a session", Example: "retask sandbox session delete "}, - {Command: "retask sandbox connect", Description: "Connect this machine as a Private VM sandbox (long-running). Logs go to the TUI (stderr when headless) and to retask.log in the current folder, which rotates into retask.log.1 ... retask.log.N", Flags: []string{"--mode", "--auto-open", "--no-auto-respond", "--session-buffer", "--log-file", "--no-log-file", "--log-max-size", "--log-backups", "--no-log-path"}, Example: "retask sandbox connect "}, + {Command: "retask sandbox connect", Description: "Connect this machine as a Private VM sandbox (long-running). Logs go to the TUI (stderr when headless) and to retask.log in the current folder, which rotates into retask.log.1 ... retask.log.N. Session folders are created in the current directory and recorded in sandbox_.json. Stopping a session, the sandbox, or this command leaves folders on disk; --retention deletes those older than its window (checked hourly), and \"off\" disables it. Live sessions are never deleted", Flags: []string{"--mode", "--auto-open", "--no-auto-respond", "--retention", "--session-buffer", "--log-file", "--no-log-file", "--log-max-size", "--log-backups", "--no-log-path"}, Example: "retask sandbox connect --retention 30d"}, {Command: "retask sandbox attach", Description: "Attach terminal to a running local session", Example: "retask sandbox attach "}, + {Command: "retask sandbox cleanup", Description: "Delete session folders left behind by stopped sessions, in the current directory. Only folders recorded in a sandbox_.json session log are considered; anything else is left alone. With no argument every session log in the directory is swept; pass a sandbox id to narrow it. --older-than 0 deletes everything and prompts first unless --yes", Flags: []string{"--older-than", "--dry-run", "--yes"}, Example: "retask sandbox cleanup --older-than 7d"}, {Command: "retask agent list", Description: "List agents", Flags: []string{"--role"}, Example: "retask agent list --role ROLE_TASK_PROCESSOR"}, {Command: "retask agent get", Description: "Get an agent by ID", Example: "retask agent get "}, {Command: "retask agent create", Description: "Create an agent", Flags: []string{"--name", "--role", "--description", "--sandbox-template-id"}, Example: "retask agent create --name 'Task Bot' --role ROLE_TASK_PROCESSOR"}, diff --git a/internal/cmd/sandbox/cleanup.go b/internal/cmd/sandbox/cleanup.go new file mode 100644 index 0000000..3b3f83d --- /dev/null +++ b/internal/cmd/sandbox/cleanup.go @@ -0,0 +1,162 @@ +// internal/cmd/sandbox/cleanup.go +package sandbox + +import ( + "bufio" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/spf13/cobra" + + "github.com/nwebxyz/retask-cli/internal/flags" +) + +func newCleanupCommand(gf *flags.Global) *cobra.Command { + var olderThan string + var dryRun bool + var yes bool + + cmd := &cobra.Command{ + Use: "cleanup [sandbox-id]", + Short: "Delete old session folders in the current directory", + Long: `Delete session folders left behind by stopped or disconnected sessions. + +Only folders recorded in a sandbox_.json session log are considered; any +other directory is left alone. With no argument, every session log in the +current directory is swept. + +Usage example: + retask sandbox cleanup + retask sandbox cleanup --older-than 7d + retask sandbox cleanup --older-than 7d + retask sandbox cleanup --older-than 0 --yes + retask sandbox cleanup --dry-run + +Flags: + --older-than string Delete folders older than this. Values: 30d, 12h, 0 (0 = everything) (default: 30d) + --dry-run Print what would be deleted and exit + --yes Skip the confirmation prompt for --older-than 0`, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) (err error) { + window, err := parseDuration(olderThan) + if err != nil { + return err + } + baseDir, err := os.Getwd() + if err != nil { + return err + } + + var logs []*sessionLog + if len(args) == 1 { + logs = []*sessionLog{newSessionLog(baseDir, args[0])} + } else if logs, err = discoverSessionLogs(baseDir); err != nil { + return err + } + + out := cmd.OutOrStdout() + + // Dry-run first, so both --dry-run and the prompt report real counts. + planned := map[*sessionLog][]string{} + total := 0 + for _, l := range logs { + ids, sweepErr := l.sweep(baseDir, time.Now(), window, nil, true) + if sweepErr != nil { + fmt.Fprintf(out, "skipping %s: %v\n", filepath.Base(l.path), sweepErr) + continue + } + if len(ids) > 0 { + planned[l] = ids + total += len(ids) + } + } + + if total == 0 { + fmt.Fprintln(out, "Nothing to clean up.") + return nil + } + + for _, l := range logs { + for _, id := range planned[l] { + fmt.Fprintf(out, "%s %s\n", l.sandboxID, id) + } + } + + if dryRun { + fmt.Fprintf(out, "\n%d session folder(s) would be deleted (--dry-run).\n", total) + return nil + } + + // A separate process cannot know which sessions are live elsewhere, + // so wiping everything asks first. + if window == 0 && !yes { + prompt := fmt.Sprintf("\nThis will delete %d session folder(s) across %d sandbox(es). Continue? [y/N]: ", total, len(planned)) + if !confirm(cmd.InOrStdin(), out, prompt) { + fmt.Fprintln(out, "Aborted.") + return nil + } + } + + deletedTotal := 0 + for _, l := range logs { + if len(planned[l]) == 0 { + continue + } + deleted, sweepErr := l.sweep(baseDir, time.Now(), window, nil, false) + deletedTotal += len(deleted) + if sweepErr != nil { + err = errors.Join(err, sweepErr) + } + } + fmt.Fprintf(out, "\nDeleted %d session folder(s).\n", deletedTotal) + return err + }, + } + + cmd.Flags().StringVar(&olderThan, "older-than", "30d", "Delete folders older than this (e.g. 30d, 12h); 0 deletes everything") + cmd.Flags().BoolVar(&dryRun, "dry-run", false, "Print what would be deleted and exit") + cmd.Flags().BoolVar(&yes, "yes", false, "Skip the confirmation prompt for --older-than 0") + return cmd +} + +// discoverSessionLogs returns every valid session log in baseDir. A working +// directory holds ordinary JSON (package.json, tsconfig.json); anything failing +// the schema check is skipped, so cleanup can never act on it. +func discoverSessionLogs(baseDir string) (logs []*sessionLog, err error) { + matches, err := filepath.Glob(filepath.Join(baseDir, "*.json")) + if err != nil { + return nil, err + } + sort.Strings(matches) + for _, p := range matches { + d, loadErr := loadSessionLogFile(p) + if loadErr != nil { + if errors.Is(loadErr, errNewerLog) { + continue // written by a newer CLI — not ours to rewrite + } + return nil, loadErr + } + if d == nil { + continue // not a session log + } + logs = append(logs, newSessionLog(baseDir, d.SandboxID)) + } + return logs, nil +} + +// confirm reads a y/N answer. Anything other than y/yes is a no. +func confirm(in io.Reader, out io.Writer, prompt string) bool { + fmt.Fprint(out, prompt) + line, err := bufio.NewReader(in).ReadString('\n') + if err != nil && line == "" { + return false + } + answer := strings.ToLower(strings.TrimSpace(line)) + return answer == "y" || answer == "yes" +} diff --git a/internal/cmd/sandbox/cleanup_test.go b/internal/cmd/sandbox/cleanup_test.go new file mode 100644 index 0000000..8431e6a --- /dev/null +++ b/internal/cmd/sandbox/cleanup_test.go @@ -0,0 +1,199 @@ +package sandbox + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDiscoverSessionLogsSkipsForeignJSON(t *testing.T) { + dir := t.TempDir() + + // Two real logs... + a := newSessionLog(dir, "sb-a") + require.NoError(t, a.record("s1", "s1", "session-s1", time.Now().UTC())) + b := newSessionLog(dir, "sb-b") + require.NoError(t, b.record("s2", "s2", "session-s2", time.Now().UTC())) + + // ...and ordinary files that must be ignored. + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"name":"app"}`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "tsconfig.json"), []byte(`{"compilerOptions":{}}`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "broken.json"), []byte(`not json`), 0o644)) + + logs, err := discoverSessionLogs(dir) + require.NoError(t, err) + + var ids []string + for _, l := range logs { + ids = append(ids, l.sandboxID) + } + assert.ElementsMatch(t, []string{"sb-a", "sb-b"}, ids, "only real session logs are discovered") +} + +func TestDiscoverSessionLogsEmptyDir(t *testing.T) { + logs, err := discoverSessionLogs(t.TempDir()) + require.NoError(t, err) + assert.Empty(t, logs) +} + +func TestConfirmAcceptsYes(t *testing.T) { + for _, in := range []string{"y\n", "Y\n", "yes\n", "YES\n"} { + var out bytes.Buffer + assert.True(t, confirm(strings.NewReader(in), &out, "delete? "), "in=%q", in) + } +} + +func TestConfirmRejectsAnythingElse(t *testing.T) { + for _, in := range []string{"n\n", "\n", "no\n", "maybe\n", ""} { + var out bytes.Buffer + assert.False(t, confirm(strings.NewReader(in), &out, "delete? "), "in=%q", in) + } +} + +func TestCleanupDryRunDeletesNothing(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + sess := filepath.Join(dir, "session-old") + require.NoError(t, os.MkdirAll(sess, 0o755)) + require.NoError(t, l.record("old", "old", "session-old", time.Now().Add(-40*24*time.Hour))) + + out := runCleanup(t, dir, []string{"--dry-run"}) + + _, err := os.Stat(sess) + assert.NoError(t, err, "--dry-run must not delete") + assert.Contains(t, out, "old", "dry run reports what it would delete") +} + +func TestCleanupDeletesAged(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + old := filepath.Join(dir, "session-old") + fresh := filepath.Join(dir, "session-fresh") + require.NoError(t, os.MkdirAll(old, 0o755)) + require.NoError(t, os.MkdirAll(fresh, 0o755)) + require.NoError(t, l.record("old", "old", "session-old", time.Now().Add(-40*24*time.Hour))) + require.NoError(t, l.record("fresh", "fresh", "session-fresh", time.Now())) + + runCleanup(t, dir, nil) + + _, err := os.Stat(old) + assert.True(t, os.IsNotExist(err), "default 30d window reaps a 40-day-old folder") + _, err = os.Stat(fresh) + assert.NoError(t, err, "recent folder survives") +} + +func TestCleanupNothingToDo(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + require.NoError(t, os.MkdirAll(filepath.Join(dir, "session-fresh"), 0o755)) + require.NoError(t, l.record("fresh", "fresh", "session-fresh", time.Now())) + + out := runCleanup(t, dir, nil) + assert.Contains(t, out, "Nothing to clean up.") +} + +func TestCleanupOlderThanZeroPromptsAndAborts(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + sess := filepath.Join(dir, "session-a") + require.NoError(t, os.MkdirAll(sess, 0o755)) + require.NoError(t, l.record("a", "a", "session-a", time.Now())) + + cmd := newCleanupCommand(nil) + cmd.SetArgs([]string{"--older-than", "0"}) + cmd.SetIn(strings.NewReader("n\n")) + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&out) + withWd(t, dir, func() { require.NoError(t, cmd.Execute()) }) + + _, err := os.Stat(sess) + assert.NoError(t, err, "answering n must abort") + assert.Contains(t, out.String(), "Aborted") +} + +func TestCleanupOlderThanZeroWithYesTakesEverything(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + sess := filepath.Join(dir, "session-a") + require.NoError(t, os.MkdirAll(sess, 0o755)) + require.NoError(t, l.record("a", "a", "session-a", time.Now())) + + runCleanup(t, dir, []string{"--older-than", "0", "--yes"}) + + _, err := os.Stat(sess) + assert.True(t, os.IsNotExist(err), "--older-than 0 --yes deletes everything") +} + +func TestCleanupSandboxArgNarrowsScope(t *testing.T) { + dir := t.TempDir() + a := newSessionLog(dir, "sb-a") + b := newSessionLog(dir, "sb-b") + aDir := filepath.Join(dir, "session-a") + bDir := filepath.Join(dir, "session-b") + require.NoError(t, os.MkdirAll(aDir, 0o755)) + require.NoError(t, os.MkdirAll(bDir, 0o755)) + require.NoError(t, a.record("a", "a", "session-a", time.Now().Add(-40*24*time.Hour))) + require.NoError(t, b.record("b", "b", "session-b", time.Now().Add(-40*24*time.Hour))) + + runCleanup(t, dir, []string{"sb-a"}) + + _, err := os.Stat(aDir) + assert.True(t, os.IsNotExist(err), "named sandbox is swept") + _, err = os.Stat(bDir) + assert.NoError(t, err, "other sandboxes are untouched when an id is given") +} + +func TestCleanupIgnoresUnloggedFolders(t *testing.T) { + dir := t.TempDir() + orphan := filepath.Join(dir, "session-orphan") + require.NoError(t, os.MkdirAll(orphan, 0o755)) + + runCleanup(t, dir, []string{"--older-than", "0", "--yes"}) + + _, err := os.Stat(orphan) + assert.NoError(t, err, "log-only: a folder with no entry is never deleted") +} + +func TestCleanupRejectsBadOlderThan(t *testing.T) { + dir := t.TempDir() + cmd := newCleanupCommand(nil) + cmd.SetArgs([]string{"--older-than", "off"}) + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&out) + withWd(t, dir, func() { + assert.Error(t, cmd.Execute(), `"off" is retention-only, not valid for --older-than`) + }) +} + +// --- helpers --- + +// withWd runs fn with the process working directory set to dir. +func withWd(t *testing.T, dir string, fn func()) { + t.Helper() + orig, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(dir)) + defer func() { require.NoError(t, os.Chdir(orig)) }() + fn() +} + +// runCleanup executes the cleanup command in dir and returns its output. +func runCleanup(t *testing.T, dir string, args []string) string { + t.Helper() + cmd := newCleanupCommand(nil) + cmd.SetArgs(args) + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&out) + cmd.SetIn(strings.NewReader("")) + withWd(t, dir, func() { require.NoError(t, cmd.Execute()) }) + return out.String() +} diff --git a/internal/cmd/sandbox/command.go b/internal/cmd/sandbox/command.go index 15f1a73..08b1b33 100644 --- a/internal/cmd/sandbox/command.go +++ b/internal/cmd/sandbox/command.go @@ -35,6 +35,7 @@ func NewCommand(gf *flags.Global) *cobra.Command { newSessionCommand(gf), newConnectCommand(gf), newAttachCommand(gf), + newCleanupCommand(gf), ) return cmd } diff --git a/internal/cmd/sandbox/connect.go b/internal/cmd/sandbox/connect.go index 0ddcb7d..ae6d380 100644 --- a/internal/cmd/sandbox/connect.go +++ b/internal/cmd/sandbox/connect.go @@ -50,6 +50,7 @@ func newConnectCommand(gf *flags.Global) *cobra.Command { var mode string var autoOpen bool var noAutoRespond bool + var retention string var sessionBuffer string var logFlags logFileFlags cmd := &cobra.Command{ @@ -60,10 +61,16 @@ func newConnectCommand(gf *flags.Global) *cobra.Command { This is a long-running command that maintains a persistent WebSocket connection to sandbox-proxy and manages sessions as local PTY processes. +Session folders are created in the current directory and recorded in +sandbox_.json. Stopping a session, the sandbox, or this command leaves them +on disk; --retention deletes the ones older than its window, checked hourly. + Usage example: retask sandbox connect sandbox_abc123 retask sandbox connect sandbox_abc123 --mode headless retask sandbox connect sandbox_abc123 --auto-open + retask sandbox connect sandbox_abc123 --retention 7d + retask sandbox connect sandbox_abc123 --retention off A dropped connection never tears down a running agent: the agent PTY is stopped only by an explicit stop/delete. Both lanes self-heal — the data lane and the @@ -80,6 +87,7 @@ Flags: --mode string Running mode: auto, tui, headless (default: auto) --auto-open Auto-open a terminal tab for each new session (default: false) --no-auto-respond Disable auto-accepting known agent startup prompts (default: false) + --retention string Delete session folders older than this, checked hourly. Values: 30d, 12h, off (default: 30d) --session-buffer Per-session output retained across a session-lane drop, flushed on reconnect (default: 10MB). 0 disables buffering. Accepts 512KB, 10MB, ... --log-file string Log file written alongside the TUI/stderr output, relative to the @@ -106,6 +114,10 @@ Environment: if mode != "auto" && mode != "tui" && mode != "headless" { return fmt.Errorf("invalid --mode %q: must be auto, tui, or headless", mode) } + retentionWindow, retentionOn, err := parseRetention(retention) + if err != nil { + return err + } sandboxID := args[0] // Resolve credentials. @@ -202,6 +214,7 @@ Environment: if err != nil { return err } + sessLog := newSessionLog(baseDir, sandboxID) autoRespond := !(noAutoRespond || os.Getenv("RETASK_SANDBOX_NO_AUTO_RESPOND") == "1") // Per-session output buffer: flag wins, else env, else default. @@ -221,12 +234,33 @@ Environment: sbResp.Msg.Name, baseDir, profile.Endpoint, + sessLog, autoRespond, sessionBufBytes, ) + + if retentionOn { + sweeper := &retentionSweeper{ + log: sessLog, + baseDir: baseDir, + window: retentionWindow, + interval: retentionSweepInterval, + isActive: sm.isActive, + logger: logger, + } + go sweeper.Run(ctx) + } + dl := newDataLane(sandboxID, wsBase, jwt, sm, &rawConnState, logger) - go dl.Run(ctx) + // A deleted sandbox ends the data lane for good; there is nothing + // left to attach to, so unwind the CLI down the same path a Ctrl-C + // takes. stop() is idempotent, so returning for any other reason + // (ctx already cancelled) is harmless. + go func() { + dl.Run(ctx) + stop() + }() if useTUI { execPath, _ := os.Executable() @@ -252,6 +286,7 @@ Environment: cmd.Flags().StringVar(&mode, "mode", "auto", "Running mode: auto, tui, headless") cmd.Flags().BoolVar(&autoOpen, "auto-open", false, "Auto-open a terminal tab for each new session") cmd.Flags().BoolVar(&noAutoRespond, "no-auto-respond", false, "Disable auto-accepting known agent startup prompts (e.g. folder-trust)") + cmd.Flags().StringVar(&retention, "retention", "30d", `Delete session folders older than this (e.g. 30d, 12h); "off" disables`) cmd.Flags().StringVar(&sessionBuffer, "session-buffer", "10MB", "Per-session output buffered across a session-lane drop (drop-oldest), flushed on reconnect. 0 disables. Accepts 512KB, 10MB, ...") logFlags.register(cmd) return cmd diff --git a/internal/cmd/sandbox/datalane.go b/internal/cmd/sandbox/datalane.go index 69d114a..04b61b1 100644 --- a/internal/cmd/sandbox/datalane.go +++ b/internal/cmd/sandbox/datalane.go @@ -225,7 +225,7 @@ func (dl *DataLane) connectOnce(ctx context.Context) (established bool, err erro case "delete_sandbox": dl.logInfo("delete_sandbox", "sandbox_id", msg.SandboxID) - dl.sessions.StopAll() + dl.sessions.RemoveAll() conn.Close(websocket.StatusNormalClosure, "deleted") //nolint:errcheck return established, errSandboxDeleted } diff --git a/internal/cmd/sandbox/retention.go b/internal/cmd/sandbox/retention.go new file mode 100644 index 0000000..522e9c2 --- /dev/null +++ b/internal/cmd/sandbox/retention.go @@ -0,0 +1,101 @@ +// internal/cmd/sandbox/retention.go +package sandbox + +import ( + "context" + "fmt" + "log/slog" + "strconv" + "strings" + "time" +) + +// retentionSweepInterval is how often connect re-checks for aged-out folders. +const retentionSweepInterval = time.Hour + +// retentionSweeper deletes logged session folders older than window. It is the +// backstop for folders left behind by stop and disconnect — the paths that +// deliberately do not delete. Explicit deletes reclaim their own disk. +type retentionSweeper struct { + log *sessionLog + baseDir string + window time.Duration + interval time.Duration + isActive func(string) bool // live sessions are never reaped; may be nil + logger *slog.Logger // may be nil +} + +// Run sweeps once immediately, then every interval until ctx is cancelled. The +// startup sweep means a machine reconnecting after a long gap cleans up +// straight away rather than waiting a full interval. +func (s *retentionSweeper) Run(ctx context.Context) { + s.once() + t := time.NewTicker(s.interval) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + s.once() + } + } +} + +func (s *retentionSweeper) once() { + deleted, err := s.log.sweep(s.baseDir, time.Now(), s.window, s.isActive, false) + if err != nil && s.logger != nil { + s.logger.Error("retention_sweep_error", "error", err) + } + if len(deleted) > 0 && s.logger != nil { + s.logger.Info("retention_sweep", + "deleted", len(deleted), + "session_ids", deleted, + "older_than", s.window.String(), + ) + } +} + +// parseDuration parses a retention window. It accepts Go duration syntax +// ("12h", "90m", "0") plus a "d" day suffix, which time.ParseDuration rejects. +func parseDuration(s string) (d time.Duration, err error) { + s = strings.TrimSpace(s) + if s == "" { + return 0, fmt.Errorf("empty duration (want e.g. 30d, 12h, 0)") + } + if days, ok := strings.CutSuffix(s, "d"); ok { + n, convErr := strconv.ParseFloat(days, 64) + if convErr != nil { + return 0, fmt.Errorf("invalid duration %q (want e.g. 30d, 12h, 0)", s) + } + if n < 0 { + return 0, fmt.Errorf("duration %q must not be negative", s) + } + return time.Duration(n * 24 * float64(time.Hour)), nil + } + d, err = time.ParseDuration(s) + if err != nil { + return 0, fmt.Errorf("invalid duration %q (want e.g. 30d, 12h, 0)", s) + } + if d < 0 { + return 0, fmt.Errorf("duration %q must not be negative", s) + } + return d, nil +} + +// parseRetention parses the --retention flag, which additionally accepts "off". +// Zero is rejected: it means "delete everything" for --older-than, so accepting +// it here would turn an hourly sweep into an hourly wipe. Disabling is "off". +func parseRetention(s string) (d time.Duration, enabled bool, err error) { + if strings.EqualFold(strings.TrimSpace(s), "off") { + return 0, false, nil + } + d, err = parseDuration(s) + if err != nil { + return 0, false, err + } + if d == 0 { + return 0, false, fmt.Errorf(`invalid --retention %q: use "off" to disable retention`, s) + } + return d, true, nil +} diff --git a/internal/cmd/sandbox/retention_test.go b/internal/cmd/sandbox/retention_test.go new file mode 100644 index 0000000..2a4925b --- /dev/null +++ b/internal/cmd/sandbox/retention_test.go @@ -0,0 +1,110 @@ +package sandbox + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/nwebxyz/retask-cli/internal/flags" +) + +func TestParseDuration(t *testing.T) { + tests := []struct { + in string + want time.Duration + }{ + {"30d", 30 * 24 * time.Hour}, + {"1d", 24 * time.Hour}, + {"0d", 0}, + {"12h", 12 * time.Hour}, + {"90m", 90 * time.Minute}, + {"0", 0}, + {" 7d ", 7 * 24 * time.Hour}, + } + for _, tc := range tests { + got, err := parseDuration(tc.in) + require.NoError(t, err, "in=%q", tc.in) + assert.Equal(t, tc.want, got, "in=%q", tc.in) + } +} + +func TestParseDurationRejects(t *testing.T) { + for _, in := range []string{"", "off", "30days", "-1d", "-5h", "abc", "d"} { + _, err := parseDuration(in) + assert.Error(t, err, "in=%q should be rejected", in) + } +} + +func TestParseRetention(t *testing.T) { + d, enabled, err := parseRetention("30d") + require.NoError(t, err) + assert.True(t, enabled) + assert.Equal(t, 30*24*time.Hour, d) + + for _, in := range []string{"off", "OFF", " off "} { + _, enabled, err := parseRetention(in) + require.NoError(t, err, "in=%q", in) + assert.False(t, enabled, "in=%q should disable retention", in) + } +} + +func TestParseRetentionRejectsZero(t *testing.T) { + // 0 means "delete everything" for --older-than; allowing it here would turn + // an hourly sweep into an hourly wipe. Disabling is spelled "off". + _, _, err := parseRetention("0") + assert.Error(t, err) + _, _, err = parseRetention("0d") + assert.Error(t, err) +} + +func TestSweeperOnceDeletesAgedFolders(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + old := "session-old" + require.NoError(t, os.MkdirAll(filepath.Join(dir, old), 0o755)) + require.NoError(t, l.record("old", "old", old, time.Now().Add(-40*24*time.Hour))) + + s := &retentionSweeper{log: l, baseDir: dir, window: 30 * 24 * time.Hour} + s.once() + + _, err := os.Stat(filepath.Join(dir, old)) + assert.True(t, os.IsNotExist(err)) +} + +func TestSweeperRunSweepsAtStartupThenStops(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + old := "session-old" + require.NoError(t, os.MkdirAll(filepath.Join(dir, old), 0o755)) + require.NoError(t, l.record("old", "old", old, time.Now().Add(-40*24*time.Hour))) + + // A long interval proves the startup sweep happened, not a tick. + s := &retentionSweeper{log: l, baseDir: dir, window: 30 * 24 * time.Hour, interval: time.Hour} + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { s.Run(ctx); close(done) }() + + require.Eventually(t, func() bool { + _, err := os.Stat(filepath.Join(dir, old)) + return os.IsNotExist(err) + }, 2*time.Second, 10*time.Millisecond, "sweeper must sweep once at startup") + + cancel() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("sweeper must stop when ctx is cancelled") + } +} + +func TestConnectRetentionFlagDefault(t *testing.T) { + cmd := newConnectCommand(&flags.Global{}) + f := cmd.Flags().Lookup("retention") + require.NotNil(t, f, "--retention must be registered") + assert.Equal(t, "30d", f.DefValue, "retention defaults to 30 days") +} diff --git a/internal/cmd/sandbox/sessionlane.go b/internal/cmd/sandbox/sessionlane.go index 766bfe6..4e3f6bc 100644 --- a/internal/cmd/sandbox/sessionlane.go +++ b/internal/cmd/sandbox/sessionlane.go @@ -18,6 +18,36 @@ import ( sandboxv1 "github.com/nwebxyz/retask-cli/proto-gen/retask/sandbox/v1" ) +// sessionDrainTimeout bounds how long teardown waits for a session's PTY to +// exit after SIGTERM before deleting its folder anyway. +const sessionDrainTimeout = 5 * time.Second + +// stoppableRunner is the slice of *agentfleet.Runner that teardown needs, so +// drain can be tested without a real PTY. +type stoppableRunner interface { + Stop() error + Done() <-chan struct{} +} + +// drain sends SIGTERM and waits for the process to actually exit, up to +// timeout. agentfleet's Stop returns as soon as the signal is delivered, not +// when the process has exited — so deleting a session folder straight after it +// races the agent's own shutdown, destroying the working directory while the +// agent is still flushing into it. SIGTERM exists to grant that grace period. +// +// A process that ignores SIGTERM is never escalated to SIGKILL by agentfleet, +// so the timeout is the only backstop against a hung session blocking teardown. +func drain(r stoppableRunner, timeout time.Duration) { + if r == nil { + return + } + r.Stop() //nolint:errcheck + select { + case <-r.Done(): + case <-time.After(timeout): + } +} + // wsDialer dials a WebSocket URL. It is a field on SessionManager so tests can // inject a fake in place of the real network dial. type wsDialer func(ctx context.Context, url string) (*websocket.Conn, error) @@ -41,7 +71,8 @@ type SessionManager struct { sandboxName string baseDir string endpoint string - autoRespond bool // auto-accept known agent prompts (e.g. folder-trust) + sessionLog *sessionLog // records session start times for retention + autoRespond bool // auto-accept known agent prompts (e.g. folder-trust) // sessionBufBytes is the per-session outbound buffer retained across a // session-lane drop (drop-oldest). 0 disables buffering. @@ -64,20 +95,22 @@ func newSessionManager( agentCfg agentfleet.AgentConfig, log *slog.Logger, workspaceID, sandboxName, baseDir, endpoint string, + sessLog *sessionLog, autoRespond bool, sessionBufBytes int, ) *SessionManager { return &SessionManager{ - sandboxID: sandboxID, - wsBase: wsBase, - fleet: fleet, - fleetCfg: fleetCfg, - agentCfg: agentCfg, - log: log, - workspaceID: workspaceID, - sandboxName: sandboxName, - baseDir: baseDir, - endpoint: endpoint, + sandboxID: sandboxID, + wsBase: wsBase, + fleet: fleet, + fleetCfg: fleetCfg, + agentCfg: agentCfg, + log: log, + workspaceID: workspaceID, + sandboxName: sandboxName, + baseDir: baseDir, + endpoint: endpoint, + sessionLog: sessLog, autoRespond: autoRespond, sessionBufBytes: sessionBufBytes, dial: defaultWSDial, @@ -94,6 +127,28 @@ func (sm *SessionManager) sessionLaneURL(sessionID, token string) string { sm.wsBase, sm.sandboxID, sessionID, token) } +// recordSessionStart logs when a session started, before bootstrap runs. +// Bootstrap creates the folder early but can fail afterwards; under the +// log-only retention policy a folder with no entry could never be reaped, so +// the entry must exist as soon as the folder can. +func (sm *SessionManager) recordSessionStart(sessionID, name string, at time.Time) { + if sm.sessionLog == nil { + return + } + if err := sm.sessionLog.record(sessionID, name, "session-"+sessionID, at); err != nil { + sm.logError("session_log_record_failed", "session_id", sessionID, "error", err) + } +} + +// isActive reports whether a session currently has a live runner. The retention +// sweeper uses it so a long-running session is never reaped. +func (sm *SessionManager) isActive(sessionID string) bool { + sm.mu.Lock() + defer sm.mu.Unlock() + _, ok := sm.sessions[sessionID] + return ok +} + // Start handles a new_session event. It is IDEMPOTENT: if session_id already // has a live runner (the relay lost its state, or the VM data-lane reconnected // and the relay re-sent new_session), re-bind a fresh session-lane to the @@ -225,6 +280,10 @@ func (sm *SessionManager) create(ctx context.Context, sessionID, token, name str } sm.logInfo("session_lane_connected", "sandbox_id", sm.sandboxID, "session_id", sessionID) + // Record before bootstrap: setupFolder creates the folder early but Run can + // fail later, and an unlogged folder can never be reaped. + sm.recordSessionStart(sessionID, name, time.Now()) + // pending latches window sizes that arrive before the PTY exists: the // initial geometry reported in the new_session frame, or a resize that // races bootstrap. It is flushed once the PTY is up. @@ -466,19 +525,78 @@ func (sm *SessionManager) Stop(sessionID string) { } } -// Remove stops the session's PTY, drops it from the fleet, and deletes its -// folder. Used for delete_session. +// Remove tears down one session for delete_session: stop the PTY, wait for it +// to exit, then delete its working folder and log entry. An explicit delete +// reclaims disk immediately rather than waiting for the retention window. func (sm *SessionManager) Remove(sessionID string) { sm.logInfo("session_removing", "session_id", sessionID) sm.mu.Lock() entry := sm.sessions[sessionID] delete(sm.sessions, sessionID) sm.mu.Unlock() + if entry != nil { - entry.runner.Stop() //nolint:errcheck + drain(entry.runner, sessionDrainTimeout) sm.fleet.Remove(sessionID) } - os.RemoveAll(filepath.Join(sm.baseDir, "session-"+sessionID)) //nolint:errcheck + if err := os.RemoveAll(filepath.Join(sm.baseDir, "session-"+sessionID)); err != nil { + sm.logError("session_dir_remove_failed", "session_id", sessionID, "error", err) + } + if sm.sessionLog != nil { + if err := sm.sessionLog.remove(sessionID); err != nil { + sm.logError("session_log_remove_failed", "session_id", sessionID, "error", err) + } + } +} + +// RemoveAll tears everything down for a deleted sandbox: stop and drain every +// live session, delete every folder the log knows about, then delete the log +// file itself. Used for delete_sandbox, after which the CLI exits. +// +// Sessions drain concurrently, so teardown costs one drain timeout rather than +// one per session. +func (sm *SessionManager) RemoveAll() { + sm.logInfo("sandbox_removing", "sandbox_id", sm.sandboxID) + + sm.mu.Lock() + entries := make(map[string]*sessionEntry, len(sm.sessions)) + for id, entry := range sm.sessions { + entries[id] = entry + } + sm.sessions = make(map[string]*sessionEntry) + sm.mu.Unlock() + + var wg sync.WaitGroup + for id, entry := range entries { + wg.Add(1) + go func() { + defer wg.Done() + drain(entry.runner, sessionDrainTimeout) + if sm.fleet != nil { + sm.fleet.Remove(id) + } + }() + } + wg.Wait() + + if sm.sessionLog == nil { + return + } + // Delete every folder the log knows about — including sessions from earlier + // runs of this sandbox that are no longer live. Folders with no entry are + // not ours to touch. + logEntries, err := sm.sessionLog.entries() + if err != nil { + sm.logError("session_log_read_failed", "sandbox_id", sm.sandboxID, "error", err) + } + for id, e := range logEntries { + if rmErr := os.RemoveAll(filepath.Join(sm.baseDir, e.Dir)); rmErr != nil { + sm.logError("session_dir_remove_failed", "session_id", id, "error", rmErr) + } + } + if err := sm.sessionLog.destroy(); err != nil { + sm.logError("session_log_destroy_failed", "sandbox_id", sm.sandboxID, "error", err) + } } // StopAll stops every active session. diff --git a/internal/cmd/sandbox/sessionlane_test.go b/internal/cmd/sandbox/sessionlane_test.go new file mode 100644 index 0000000..22cc8b8 --- /dev/null +++ b/internal/cmd/sandbox/sessionlane_test.go @@ -0,0 +1,202 @@ +package sandbox + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// newTestSessionManager builds a SessionManager with only the fields the log +// and teardown tests need — no fleet, no websockets, no PTYs. sessions is left +// empty: these tests cover the "no live entry" teardown path (a restart lost +// it, or nothing was ever attached), matching how Remove/RemoveAll behave when +// sm.sessions has no entry for the id — folder and log cleanup still run. +// drain() itself is exercised directly below via fakeRunner, decoupled from +// SessionManager/sessionEntry construction. +func newTestSessionManager(t *testing.T, baseDir string) *SessionManager { + t.Helper() + return &SessionManager{ + sandboxID: "sb-1", + baseDir: baseDir, + sessionLog: newSessionLog(baseDir, "sb-1"), + sessions: map[string]*sessionEntry{}, + } +} + +// --- record on start --- + +func TestRecordSessionStartWritesLogBeforeBootstrap(t *testing.T) { + dir := t.TempDir() + sm := newTestSessionManager(t, dir) + + start := time.Date(2026, 7, 17, 9, 0, 0, 0, time.UTC) + sm.recordSessionStart("sess-a", "My Session", start) + + entries, err := sm.sessionLog.entries() + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Equal(t, "My Session", entries["sess-a"].Name) + assert.Equal(t, "session-sess-a", entries["sess-a"].Dir) + assert.True(t, start.Equal(entries["sess-a"].CreatedAt)) +} + +func TestRecordSessionStartWithNoLogIsSafe(t *testing.T) { + sm := &SessionManager{baseDir: t.TempDir()} + assert.NotPanics(t, func() { sm.recordSessionStart("sess-a", "n", time.Now()) }) +} + +func TestIsActiveTracksLiveSessions(t *testing.T) { + sm := newTestSessionManager(t, t.TempDir()) + assert.False(t, sm.isActive("sess-a"), "unknown session is not active") +} + +// --- drain --- + +// fakeRunner implements stoppableRunner with a controllable exit, so drain +// tests are deterministic instead of timing-dependent. +type fakeRunner struct { + done chan struct{} + stopped chan struct{} + exitOnStop bool +} + +func newFakeRunner(exitOnStop bool) *fakeRunner { + return &fakeRunner{ + done: make(chan struct{}), + stopped: make(chan struct{}, 1), + exitOnStop: exitOnStop, + } +} + +func (f *fakeRunner) Stop() error { + select { + case f.stopped <- struct{}{}: + default: + } + if f.exitOnStop { + close(f.done) // a well-behaved agent exits on SIGTERM + } + return nil +} + +func (f *fakeRunner) Done() <-chan struct{} { return f.done } + +func TestDrainWaitsForExit(t *testing.T) { + f := newFakeRunner(true) + + start := time.Now() + drain(f, 5*time.Second) + + assert.Less(t, time.Since(start), time.Second, "drain must return as soon as the process exits") + select { + case <-f.stopped: + default: + t.Fatal("drain must send SIGTERM via Stop") + } +} + +func TestDrainGivesUpAfterTimeout(t *testing.T) { + // agentfleet never escalates to SIGKILL, so a process that ignores SIGTERM + // must not block teardown forever. + f := newFakeRunner(false) + + start := time.Now() + drain(f, 50*time.Millisecond) + elapsed := time.Since(start) + + assert.GreaterOrEqual(t, elapsed, 50*time.Millisecond) + assert.Less(t, elapsed, time.Second, "drain must give up at the timeout") +} + +func TestDrainNilRunnerIsSafe(t *testing.T) { + assert.NotPanics(t, func() { drain(nil, time.Second) }) +} + +// --- delete_session --- + +func TestRemoveDeletesFolderAndLogEntry(t *testing.T) { + dir := t.TempDir() + sm := newTestSessionManager(t, dir) + sessDir := mkSession(t, sm.sessionLog, dir, "sess-a", time.Now().UTC()) + + sm.Remove("sess-a") + + _, err := os.Stat(sessDir) + assert.True(t, os.IsNotExist(err), "delete_session must delete the folder") + + entries, err := sm.sessionLog.entries() + require.NoError(t, err) + assert.Empty(t, entries, "delete_session must drop the log entry") +} + +// --- the stop/delete distinction --- + +func TestStopDoesNotDelete(t *testing.T) { + // Regression guard: stopping is not deleting. If deletion ever leaks into + // a stop path, this fails. + dir := t.TempDir() + sm := newTestSessionManager(t, dir) + sessDir := mkSession(t, sm.sessionLog, dir, "sess-a", time.Now().UTC()) + + sm.Stop("sess-a") + sm.StopAll() + + _, err := os.Stat(sessDir) + assert.NoError(t, err, "Stop/StopAll must never delete a session folder") + + entries, err := sm.sessionLog.entries() + require.NoError(t, err) + assert.Len(t, entries, 1, "Stop/StopAll must never touch the log") +} + +// --- delete_sandbox --- + +func TestRemoveAllDeletesEveryFolderAndTheLogFile(t *testing.T) { + dir := t.TempDir() + sm := newTestSessionManager(t, dir) + now := time.Now().UTC() + + a := mkSession(t, sm.sessionLog, dir, "sess-a", now) + b := mkSession(t, sm.sessionLog, dir, "sess-b", now.Add(-40*24*time.Hour)) + + sm.RemoveAll() + + for _, d := range []string{a, b} { + _, err := os.Stat(d) + assert.True(t, os.IsNotExist(err), "delete_sandbox must delete every logged folder: %s", d) + } + _, err := os.Stat(sessionLogPath(dir, "sb-1")) + assert.True(t, os.IsNotExist(err), "delete_sandbox must delete the log file") +} + +func TestRemoveAllLeavesUnloggedFoldersAlone(t *testing.T) { + dir := t.TempDir() + sm := newTestSessionManager(t, dir) + mkSession(t, sm.sessionLog, dir, "sess-a", time.Now().UTC()) + + orphan := filepath.Join(dir, "session-orphan") + require.NoError(t, os.MkdirAll(orphan, 0o755)) + + sm.RemoveAll() + + _, err := os.Stat(orphan) + assert.NoError(t, err, "log-only policy holds even on sandbox delete") +} + +func TestRemoveAllClosesLogAgainstSweeperRace(t *testing.T) { + dir := t.TempDir() + sm := newTestSessionManager(t, dir) + mkSession(t, sm.sessionLog, dir, "sess-a", time.Now().UTC()) + + sm.RemoveAll() + + // A sweep tick arriving after teardown must not resurrect the log file. + _, err := sm.sessionLog.sweep(dir, time.Now(), time.Hour, nil, false) + require.NoError(t, err) + _, err = os.Stat(sessionLogPath(dir, "sb-1")) + assert.True(t, os.IsNotExist(err), "a sweep after teardown must not recreate the log") +} diff --git a/internal/cmd/sandbox/sessionlog.go b/internal/cmd/sandbox/sessionlog.go new file mode 100644 index 0000000..818f2ec --- /dev/null +++ b/internal/cmd/sandbox/sessionlog.go @@ -0,0 +1,241 @@ +// internal/cmd/sandbox/sessionlog.go +package sandbox + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "sort" + "sync" + "time" +) + +// sessionLogVersion is the schema version written to sandbox_.json. +const sessionLogVersion = 1 + +// errNewerLog reports a log written by a newer CLI. Such files are skipped +// rather than rewritten, so an older binary cannot truncate fields it lost. +var errNewerLog = errors.New("session log written by a newer CLI version") + +// sessionLogEntry is one recorded session. +type sessionLogEntry struct { + Name string `json:"name"` + Dir string `json:"dir"` + CreatedAt time.Time `json:"created_at"` +} + +// sessionLogData is the on-disk shape of sandbox_.json. +type sessionLogData struct { + Version int `json:"version"` + SandboxID string `json:"sandbox_id"` + Sessions map[string]sessionLogEntry `json:"sessions"` +} + +// sessionLog owns /sandbox_.json. It records when each session +// started so folders can be reaped by age. It is the only source of truth for +// what may be deleted: a session-* folder with no entry is never touched. +// +// Every mutation is a read-modify-write under the mutex followed by an atomic +// rewrite, because concurrent new_session events and the retention sweep both +// mutate the file. +type sessionLog struct { + path string + sandboxID string + + mu sync.Mutex + closed bool +} + +// sessionLogPath returns the log path for a sandbox in baseDir. +func sessionLogPath(baseDir, sandboxID string) string { + return filepath.Join(baseDir, "sandbox_"+sandboxID+".json") +} + +func newSessionLog(baseDir, sandboxID string) *sessionLog { + return &sessionLog{path: sessionLogPath(baseDir, sandboxID), sandboxID: sandboxID} +} + +// loadSessionLogFile reads a log file. It returns (nil, nil) when the file is +// absent or is not one of ours — a working directory holds ordinary JSON +// (package.json, tsconfig.json) that must never be mistaken for a log. It +// returns errNewerLog for a log written by a newer CLI. +func loadSessionLogFile(path string) (data *sessionLogData, err error) { + raw, err := os.ReadFile(path) + if os.IsNotExist(err) { + return nil, nil + } + if err != nil { + return nil, err + } + var d sessionLogData + if json.Unmarshal(raw, &d) != nil { + return nil, nil // not JSON we understand — leave it alone + } + if d.Version == 0 || d.SandboxID == "" || d.Sessions == nil { + return nil, nil // valid JSON, but not a session log + } + if d.Version > sessionLogVersion { + return nil, fmt.Errorf("%s: %w (version %d)", path, errNewerLog, d.Version) + } + return &d, nil +} + +// load returns the current log contents, or a fresh empty one. +// Caller must hold l.mu. +func (l *sessionLog) load() (data *sessionLogData, err error) { + d, err := loadSessionLogFile(l.path) + if err != nil { + return nil, err + } + if d == nil { + d = &sessionLogData{ + Version: sessionLogVersion, + SandboxID: l.sandboxID, + Sessions: map[string]sessionLogEntry{}, + } + } + return d, nil +} + +// save atomically replaces the log file. Caller must hold l.mu. +func (l *sessionLog) save(d *sessionLogData) (err error) { + raw, err := json.MarshalIndent(d, "", " ") + if err != nil { + return err + } + raw = append(raw, '\n') + + // Temp file in the same directory keeps the rename on one filesystem. + tmp, err := os.CreateTemp(filepath.Dir(l.path), ".sessionlog-*.tmp") + if err != nil { + return err + } + tmpName := tmp.Name() + defer os.Remove(tmpName) //nolint:errcheck // no-op once renamed + + if _, err = tmp.Write(raw); err != nil { + tmp.Close() //nolint:errcheck + return err + } + if err = tmp.Close(); err != nil { + return err + } + return os.Rename(tmpName, l.path) +} + +// record adds or updates a session entry. It is called before bootstrap runs, +// so every folder we create has an entry and stays reapable even if bootstrap +// fails partway. +func (l *sessionLog) record(sessionID, name, dir string, createdAt time.Time) (err error) { + l.mu.Lock() + defer l.mu.Unlock() + if l.closed { + return nil + } + d, err := l.load() + if err != nil { + return err + } + d.Sessions[sessionID] = sessionLogEntry{Name: name, Dir: dir, CreatedAt: createdAt.UTC()} + return l.save(d) +} + +// remove drops a single session entry. +func (l *sessionLog) remove(sessionID string) (err error) { + l.mu.Lock() + defer l.mu.Unlock() + if l.closed { + return nil + } + d, err := l.load() + if err != nil { + return err + } + if _, ok := d.Sessions[sessionID]; !ok { + return nil + } + delete(d.Sessions, sessionID) + return l.save(d) +} + +// entries returns a copy of the recorded sessions. +func (l *sessionLog) entries() (out map[string]sessionLogEntry, err error) { + l.mu.Lock() + defer l.mu.Unlock() + d, err := l.load() + if err != nil { + return nil, err + } + out = make(map[string]sessionLogEntry, len(d.Sessions)) + for k, v := range d.Sessions { + out[k] = v + } + return out, nil +} + +// sweep deletes every logged session at least olderThan old and drops its +// entry, returning the session ids it took. olderThan == 0 matches everything. +// +// skip reports sessions that must not be touched — the retention sweeper passes +// live sessions, so a session running longer than the window never has its own +// working directory deleted underneath it. It may be nil. +// +// Only folders listed in the log are considered: a session-* folder with no +// entry is not ours to delete. +// +// With dryRun, nothing is deleted but the same ids are reported. +func (l *sessionLog) sweep(baseDir string, now time.Time, olderThan time.Duration, skip func(string) bool, dryRun bool) (deleted []string, err error) { + l.mu.Lock() + defer l.mu.Unlock() + if l.closed { + return nil, nil + } + d, err := loadSessionLogFile(l.path) + if err != nil || d == nil { + return nil, err + } + + for id, e := range d.Sessions { + if skip != nil && skip(id) { + continue + } + if now.Sub(e.CreatedAt) < olderThan { + continue + } + if dryRun { + deleted = append(deleted, id) + continue + } + if rmErr := os.RemoveAll(filepath.Join(baseDir, e.Dir)); rmErr != nil { + // Keep the entry so a later sweep retries this folder. + err = errors.Join(err, rmErr) + continue + } + delete(d.Sessions, id) + deleted = append(deleted, id) + } + sort.Strings(deleted) + + if !dryRun && len(deleted) > 0 { + if saveErr := l.save(d); saveErr != nil { + return deleted, errors.Join(err, saveErr) + } + } + return deleted, err +} + +// destroy deletes the log file and closes the store. Closing matters: on +// delete_sandbox the retention sweeper may still be alive, and a sweep tick +// after the file is gone would otherwise recreate a log for a sandbox that no +// longer exists. Once closed, every mutation is a no-op. +func (l *sessionLog) destroy() (err error) { + l.mu.Lock() + defer l.mu.Unlock() + l.closed = true + if err = os.Remove(l.path); err != nil && !os.IsNotExist(err) { + return err + } + return nil +} diff --git a/internal/cmd/sandbox/sessionlog_test.go b/internal/cmd/sandbox/sessionlog_test.go new file mode 100644 index 0000000..63da503 --- /dev/null +++ b/internal/cmd/sandbox/sessionlog_test.go @@ -0,0 +1,245 @@ +package sandbox + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSessionLogRecordAndLoad(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + now := time.Date(2026, 7, 16, 21, 17, 3, 0, time.UTC) + + require.NoError(t, l.record("sess-a", "My Session", "session-sess-a", now)) + + entries, err := l.entries() + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Equal(t, "My Session", entries["sess-a"].Name) + assert.Equal(t, "session-sess-a", entries["sess-a"].Dir) + assert.True(t, now.Equal(entries["sess-a"].CreatedAt)) + + // The file is named after the sandbox, next to the session folders. + _, err = os.Stat(filepath.Join(dir, "sandbox_sb-1.json")) + assert.NoError(t, err) +} + +func TestSessionLogRecordIsIdempotent(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + now := time.Now().UTC() + + require.NoError(t, l.record("sess-a", "First", "session-sess-a", now)) + require.NoError(t, l.record("sess-a", "Second", "session-sess-a", now)) + + entries, err := l.entries() + require.NoError(t, err) + assert.Len(t, entries, 1, "re-recording a session must not duplicate it") + assert.Equal(t, "Second", entries["sess-a"].Name) +} + +func TestSessionLogRemove(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + now := time.Now().UTC() + require.NoError(t, l.record("sess-a", "A", "session-sess-a", now)) + require.NoError(t, l.record("sess-b", "B", "session-sess-b", now)) + + require.NoError(t, l.remove("sess-a")) + + entries, err := l.entries() + require.NoError(t, err) + require.Len(t, entries, 1) + _, ok := entries["sess-b"] + assert.True(t, ok) +} + +func TestSessionLogMissingFileIsEmpty(t *testing.T) { + l := newSessionLog(t.TempDir(), "sb-nope") + entries, err := l.entries() + require.NoError(t, err, "a missing log is the normal first-run case") + assert.Empty(t, entries) +} + +func TestSessionLogIgnoresForeignJSON(t *testing.T) { + dir := t.TempDir() + // A working directory holds ordinary JSON. It must never read as a log. + pkg := filepath.Join(dir, "package.json") + require.NoError(t, os.WriteFile(pkg, []byte(`{"name":"app","version":"1.0.0"}`), 0o644)) + + data, err := loadSessionLogFile(pkg) + require.NoError(t, err) + assert.Nil(t, data, "package.json must not parse as a session log") +} + +func TestSessionLogIgnoresGarbage(t *testing.T) { + dir := t.TempDir() + bad := filepath.Join(dir, "notjson.json") + require.NoError(t, os.WriteFile(bad, []byte("this is not json"), 0o644)) + + data, err := loadSessionLogFile(bad) + require.NoError(t, err) + assert.Nil(t, data) +} + +func TestSessionLogRejectsNewerVersion(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "sb-future.json") + require.NoError(t, os.WriteFile(p, []byte(`{"version":99,"sandbox_id":"sb-future","sessions":{}}`), 0o644)) + + _, err := loadSessionLogFile(p) + assert.ErrorIs(t, err, errNewerLog, "an older CLI must not truncate a newer log") +} + +func TestSessionLogAtomicWriteLeavesNoTemp(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + require.NoError(t, l.record("sess-a", "A", "session-sess-a", time.Now().UTC())) + + names, err := filepath.Glob(filepath.Join(dir, "*")) + require.NoError(t, err) + require.Len(t, names, 1) + assert.Equal(t, "sandbox_sb-1.json", filepath.Base(names[0])) +} + +func TestSessionLogDestroyDeletesFileAndLatches(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + require.NoError(t, l.record("sess-a", "A", "session-sess-a", time.Now().UTC())) + + require.NoError(t, l.destroy()) + _, err := os.Stat(filepath.Join(dir, "sandbox_sb-1.json")) + assert.True(t, os.IsNotExist(err), "destroy must delete the log file") + + // A sweep or a late session start must not resurrect the file. + require.NoError(t, l.record("sess-b", "B", "session-sess-b", time.Now().UTC())) + _, err = os.Stat(filepath.Join(dir, "sandbox_sb-1.json")) + assert.True(t, os.IsNotExist(err), "a closed log must not be recreated") +} + +func TestSessionLogDestroyOnMissingFileIsNoError(t *testing.T) { + l := newSessionLog(t.TempDir(), "sb-1") + assert.NoError(t, l.destroy()) +} + +// mkSession creates a session folder and records it as started at createdAt. +func mkSession(t *testing.T, l *sessionLog, baseDir, id string, createdAt time.Time) string { + t.Helper() + dir := "session-" + id + require.NoError(t, os.MkdirAll(filepath.Join(baseDir, dir), 0o755)) + require.NoError(t, l.record(id, id, dir, createdAt)) + return filepath.Join(baseDir, dir) +} + +func TestSweepDeletesOldKeepsNew(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + now := time.Date(2026, 7, 17, 12, 0, 0, 0, time.UTC) + + oldDir := mkSession(t, l, dir, "old", now.Add(-40*24*time.Hour)) + newDir := mkSession(t, l, dir, "fresh", now.Add(-2*24*time.Hour)) + + deleted, err := l.sweep(dir, now, 30*24*time.Hour, nil, false) + require.NoError(t, err) + assert.Equal(t, []string{"old"}, deleted) + + _, err = os.Stat(oldDir) + assert.True(t, os.IsNotExist(err), "aged-out folder should be gone") + _, err = os.Stat(newDir) + assert.NoError(t, err, "recent folder must survive") + + entries, err := l.entries() + require.NoError(t, err) + require.Len(t, entries, 1) + _, ok := entries["fresh"] + assert.True(t, ok, "sweep must drop the entry with the folder") +} + +func TestSweepSkipsLiveSessions(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + now := time.Date(2026, 7, 17, 12, 0, 0, 0, time.UTC) + + // A session running longer than the window must not have its own working + // directory deleted out from under it. + liveDir := mkSession(t, l, dir, "live", now.Add(-40*24*time.Hour)) + + deleted, err := l.sweep(dir, now, 30*24*time.Hour, func(id string) bool { return id == "live" }, false) + require.NoError(t, err) + assert.Empty(t, deleted) + + _, err = os.Stat(liveDir) + assert.NoError(t, err, "a live session's folder must survive its own age") +} + +func TestSweepZeroTakesEverything(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + now := time.Date(2026, 7, 17, 12, 0, 0, 0, time.UTC) + + mkSession(t, l, dir, "a", now.Add(-40*24*time.Hour)) + mkSession(t, l, dir, "b", now) // created this instant + + deleted, err := l.sweep(dir, now, 0, nil, false) + require.NoError(t, err) + assert.Equal(t, []string{"a", "b"}, deleted, "olderThan 0 means everything") + + entries, err := l.entries() + require.NoError(t, err) + assert.Empty(t, entries) +} + +func TestSweepDryRunDeletesNothing(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + now := time.Date(2026, 7, 17, 12, 0, 0, 0, time.UTC) + oldDir := mkSession(t, l, dir, "old", now.Add(-40*24*time.Hour)) + + deleted, err := l.sweep(dir, now, 30*24*time.Hour, nil, true) + require.NoError(t, err) + assert.Equal(t, []string{"old"}, deleted, "dry run still reports what it would delete") + + _, err = os.Stat(oldDir) + assert.NoError(t, err, "dry run must not delete") + + entries, err := l.entries() + require.NoError(t, err) + assert.Len(t, entries, 1, "dry run must not touch the log") +} + +func TestSweepIgnoresUnloggedFolders(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + now := time.Date(2026, 7, 17, 12, 0, 0, 0, time.UTC) + + // Log-only policy: a folder with no entry is invisible to cleanup. + orphan := filepath.Join(dir, "session-orphan") + require.NoError(t, os.MkdirAll(orphan, 0o755)) + mkSession(t, l, dir, "old", now.Add(-40*24*time.Hour)) + + deleted, err := l.sweep(dir, now, 30*24*time.Hour, nil, false) + require.NoError(t, err) + assert.Equal(t, []string{"old"}, deleted) + + _, err = os.Stat(orphan) + assert.NoError(t, err, "an unlogged folder must never be touched") +} + +func TestSweepOnClosedLogIsNoop(t *testing.T) { + dir := t.TempDir() + l := newSessionLog(dir, "sb-1") + now := time.Now().UTC() + mkSession(t, l, dir, "old", now.Add(-40*24*time.Hour)) + require.NoError(t, l.destroy()) + + deleted, err := l.sweep(dir, now, 30*24*time.Hour, nil, false) + require.NoError(t, err) + assert.Empty(t, deleted, "a closed log must not be swept or recreated") + _, err = os.Stat(filepath.Join(dir, "sandbox_sb-1.json")) + assert.True(t, os.IsNotExist(err)) +}