From 16450efbda223975458be075f1723a902c5aadf4 Mon Sep 17 00:00:00 2001 From: nwebbot Date: Fri, 17 Jul 2026 11:40:26 +1000 Subject: [PATCH 01/10] docs: design for sandbox session retention & cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records session start times to .json, makes age the single folder-deletion policy, and adds an hourly sweeper plus a `sandbox cleanup` command. Two findings reshaped the original request: - The elapsed-time panel is rendered by agentfleet, not retask-cli, and TUIConfig exposes no formatting hook — that item must land upstream. - Stop and disconnect already never deleted session folders; only delete_session did. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...-07-17-sandbox-session-retention-design.md | 270 ++++++++++++++++++ 1 file changed, 270 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md diff --git a/docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md b/docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md new file mode 100644 index 0000000..06b1560 --- /dev/null +++ b/docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md @@ -0,0 +1,270 @@ +# Sandbox session retention & cleanup + +Date: 2026-07-17 +Status: Approved + +## Summary + +Session working folders currently live and die with the session record. This +change makes them durable artifacts governed by an age-based retention policy: +`retask sandbox connect` records every session's start time to a per-sandbox log +file, stops deleting folders on delete, and sweeps folders older than a +configurable window. A new `retask sandbox cleanup` command exposes the same +sweep for manual use. + +Separately, the session panel's elapsed timer gains an hours component. + +## Scope + +Three deliverables: + +1. Elapsed time renders `h:mm:ss` past one hour — **in `agentfleet`, not this repo**. +2. Session folder retention — session log, no delete-on-remove, hourly sweeper, + `sandbox cleanup` command. +3. `help-llm` manifest updated for the new command and flags. + +## 1. Elapsed time (agentfleet) + +The panel is rendered by `github.com/hoaitan/agentfleet`, not `retask-cli`. +`tui/tui.go:renderCard` formats elapsed time as minutes:seconds, so a 90-minute +session renders `90:30`: + +```go +d := time.Since(r.StartedAt()).Round(time.Second) +elapsed = fmt.Sprintf("%02d:%02d", int(d.Minutes()), int(d.Seconds())%60) +... +elapsedStr := styleMeta.Width(5).Render(elapsed) +``` + +`agentfleet.TUIConfig` exposes `Title`, `TitleRight`, `AutoOpen`, `Log`, +`OnClose`, and `FilterLines` — no hook for elapsed formatting — and `go.mod` +carries no `replace` directive. The format cannot be overridden from +`retask-cli`. The change must land in agentfleet. + +**Decision:** hardcode the format in agentfleet rather than add a config hook. + +```go +d := time.Since(r.StartedAt()).Round(time.Second) +if h := int(d.Hours()); h > 0 { + elapsed = fmt.Sprintf("%d:%02d:%02d", h, int(d.Minutes())%60, int(d.Seconds())%60) +} else { + elapsed = fmt.Sprintf("%02d:%02d", int(d.Minutes()), int(d.Seconds())%60) +} +``` + +`styleMeta.Width(5)` widens to `Width(8)` — `1:15:30` is 7 characters and a +3-digit hour count needs 8. `renderCard` derives `nameMaxW` from +`lipgloss.Width(rightStr)`, so the name column reflows with no further change. + +Sub-hour rendering is unchanged (`05:30` stays `05:30`). + +**Sequencing:** patch agentfleet → tag a release → bump `go.mod` in `retask-cli`. +Tagging requires credentials with write access to `hoaitan/agentfleet`; the +sandbox's `gh` token has READ only. This item is independently landable and does +not block section 2 or 3. + +## 2. Session folder retention + +### 2.1 Current behaviour + +Contrary to the original premise, stop and disconnect do **not** delete session +folders today. The repo contains exactly one `os.RemoveAll` +(`internal/cmd/sandbox/sessionlane.go:213`), inside `SessionManager.Remove`, +reached only from the `delete_session` data-lane message +(`internal/cmd/sandbox/datalane.go:156`). + +| Trigger | Path | Deletes folder? | +|---|---|---| +| `stop_session` | `Stop` → PTY SIGTERM | No | +| `stop_sandbox` | `StopAll` | No | +| Disconnect / TUI exit | `StopAll` | No | +| TUI `x` on a task | `OnClose` → `Stop` + `terminate_session` | No | +| `delete_session` | `Remove` → `os.RemoveAll` | Yes | + +So "stop/disconnect must not delete folders" requires no code change. The only +change needed is to `delete_session`. + +### 2.2 Session log + +New file: `internal/cmd/sandbox/sessionlog.go`. + +Location: `/.json`, where `baseDir` is `os.Getwd()` from +`connect.go` — the same folder that holds the `session-/` directories it +tracks. Naming by sandbox id means several connected sandboxes can share one +working folder without collision. + +```json +{ + "version": 1, + "sandbox_id": "abf05a5d-5df3-45c2-9944-9dc55e4f8c1f", + "sessions": { + "5a868a9c-1320-4146-aa8b-28dae66e33ba": { + "name": "Tan's MacMini — 2026-07-16 21:17", + "dir": "session-5a868a9c-1320-4146-aa8b-28dae66e33ba", + "created_at": "2026-07-16T21:17:03Z" + } + } +} +``` + +```go +type sessionLog struct { + Version int `json:"version"` + SandboxID string `json:"sandbox_id"` + Sessions map[string]sessionEntry `json:"sessions"` +} + +type sessionEntry struct { + Name string `json:"name"` + Dir string `json:"dir"` + CreatedAt time.Time `json:"created_at"` +} +``` + +Keyed by session id: upsert is idempotent (a reconnect cannot duplicate a row) +and cleanup removal is a map delete. + +**Store semantics.** A mutex-guarded store owns the file. Concurrent +`new_session` events and the hourly sweep both mutate it, so every mutation takes +the lock, then rewrites the whole file atomically (write temp in the same +directory, `os.Rename` over the target). At tens of sessions, full rewrite is +cheaper than the complexity of incremental updates. + +**Load semantics.** A missing file yields an empty map — not an error; first run +is the common case. A file that parses but lacks `version` / `sandbox_id` / +`sessions` is treated as "not ours" and left untouched. This is what stops a +`cleanup` sweep from touching `package.json` or `tsconfig.json` in a working +folder. A log whose `version` is greater than the version this binary +understands is skipped with a warning rather than rewritten, so an older CLI +cannot silently truncate a newer log's fields. + +**Cross-process caveat:** two `connect` processes for the *same* sandbox in the +*same* cwd would race on one log file (last writer wins). Out of scope — that +configuration is already broken for other reasons (both would drive +`session-/` for the same ids). Not defended against. + +### 2.3 Write placement + +The log entry is recorded in `SessionManager.Start` (`sessionlane.go`) +**immediately before** `sb.Run(ctx, wsConn)` — not after. + +`SessionBootstrap.setupFolder` creates the folder early in `Run`, but `Run` can +fail afterward (git clone, agent config write). Under the log-only policy +(§2.4), a folder created without a log entry is invisible to cleanup forever. +Recording before bootstrap guarantees every folder we create is reapable. The +directory path is deterministic (`session-`), so nothing is lost by +recording early. + +### 2.4 Orphan folders: out of scope, by decision + +`session-*` folders with no log entry are ignored entirely. Cleanup only ever +deletes what the log lists. + +**Accepted consequence:** folders on disk before this feature ships, and any +folder created while the log was missing or deleted, are never auto-reaped and +must be removed by hand. An adoption scan (recording unclaimed folders using +filesystem mtime) was considered and explicitly rejected. + +### 2.5 Deletion policy + +`SessionManager.Remove` drops its `os.RemoveAll` call. It continues to stop the +PTY and drop the fleet card, so `delete_session` still makes the session +disappear from the UI immediately. The folder survives until it ages out or +`sandbox cleanup` reaps it. + +Age becomes the single deletion policy across every path. + +### 2.6 Retention sweeper + +`retask sandbox connect` gains `--retention` (default `30d`; `off` disables). +When enabled, a goroutine sweeps once at startup and then hourly until the +command's context is cancelled. + +Each sweep, for every entry older than the window: delete the folder, drop the +entry, rewrite the log. + +**Live-session guard.** The sweeper skips any session id currently in +`SessionManager.sessions`. Without it, a session running longer than the +retention window would have its own working directory deleted out from under its +PTY. + +`--retention 30d` is not valid `time.ParseDuration` input (no `d` unit), so a +`parseDuration` helper handles the `d` suffix on top of the standard units. + +```go +// parseDuration accepts "30d", "12h", "0". Shared by both flags. +func parseDuration(s string) (d time.Duration, err error) + +// parseRetention wraps it, additionally accepting "off". +func parseRetention(s string) (d time.Duration, enabled bool, err error) +``` + +The two flags share the duration grammar but not their keywords: `off` is valid +only for `--retention` (it is an error for `--older-than`), and `--retention 0` +is an error rather than a silent "reap everything hourly" — disabling is spelled +`off` and only `off`. + +## 3. `retask sandbox cleanup` + +New file: `internal/cmd/sandbox/cleanup.go`, wired into the `sandbox` command's +`AddCommand` block. + +``` +retask sandbox cleanup # every log in cwd, default 30d +retask sandbox cleanup # one sandbox +retask sandbox cleanup --older-than 7d +retask sandbox cleanup --older-than 0 # everything (prompts) +retask sandbox cleanup --older-than 0 --yes # everything, no prompt +retask sandbox cleanup --dry-run # report only +``` + +**Flags:** `--older-than` (default `30d`), `--dry-run`, `--yes`. + +**Vocabulary.** `--retention 30d` and `--older-than 30d` share one duration +grammar. Retention disables via `off` rather than `0`, which frees `0` to +unambiguously mean "delete everything" — the two meanings cannot collide. + +**Scope.** With no argument, every valid log in cwd (files failing the §2.2 +schema check are skipped). With an argument, only `.json`. + +**Confirmation.** `--older-than 0` prompts before deleting, because a separate +process cannot know which sessions are live. `--yes` bypasses for scripts. +Non-zero windows do not prompt. + +**Shared implementation.** The command and the sweeper call one sweep function. +The sweeper passes its live-session set; the command passes an empty set. + +## 4. `help-llm` + +`cmd/retask/main_test.go:83` asserts the hand-maintained manifest matches the +command tree — an undocumented flag fails the build. Update +`internal/cmd/helpcmd/command.go`: + +- Add `retask sandbox cleanup` with `--older-than`, `--dry-run`, `--yes`. +- Add `--retention` to the existing `retask sandbox connect` entry (line 163). + +Also update the `Long` help on `connect` (which documents its flags inline) per +the repo's help-text template. + +## 5. Testing + +| File | Cases | +|---|---| +| `sessionlog_test.go` | round-trip save/load; upsert idempotency; atomic write leaves no temp file; missing file → empty map; foreign JSON (`package.json`) rejected | +| `sessionlog_test.go` | `parseRetention`: `30d`, `12h`, `off`, invalid input | +| `sessionlog_test.go` | sweep: reaps older-than-window, keeps newer, removes entry + folder together, skips live sessions | +| `cleanup_test.go` | multi-log cwd; foreign JSON skipped; `--dry-run` deletes nothing; `--older-than 0` takes all; single-sandbox arg narrows scope | +| `main_test.go` | existing manifest sync test covers the new command and flags | + +Sweep tests inject a clock and base directory rather than sleeping. + +## Decisions rejected + +- **`FormatElapsed` hook in agentfleet's `TUIConfig`** — more flexible, but a + larger API change than the format warrants. +- **Adopting orphan folders via mtime** — would have reaped the pre-existing + backlog; rejected in favour of a strict log-only policy. +- **Keeping `os.RemoveAll` on `delete_session`** — would leave "delete" meaning + two different things depending on the path. +- **`--after-days 30` (numeric)** — closer to the original phrasing, but splits + the duration vocabulary and cannot express sub-day windows. From 7cb50a9a96180d74c283667c3950210dd86ad8c3 Mon Sep 17 00:00:00 2001 From: nwebbot Date: Fri, 17 Jul 2026 11:51:10 +1000 Subject: [PATCH 02/10] docs: explicit-delete teardown for delete_session/delete_sandbox MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverses the earlier "defer deletion to the sweeper" call: explicit deletes now reclaim disk immediately, and retention covers only folders left behind by stop/disconnect. delete_sandbox becomes a full teardown — stop, drain, delete folders, delete the log file, exit the CLI. Two hazards this design has to handle: - PtyAgent.Stop returns on SIGTERM delivery, not process exit, so deleting straight after it races the agent's own cleanup. Every delete path now drains Runner.Done() with a timeout first. - Deleting the log file while the sweeper is alive lets a later tick recreate it, so the store latches closed on teardown. Stop, StopAll and CLI-stop remain deletion-free, with regression tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...-07-17-sandbox-session-retention-design.md | 150 ++++++++++++++++-- 1 file changed, 134 insertions(+), 16 deletions(-) diff --git a/docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md b/docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md index 06b1560..4e23b2a 100644 --- a/docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md +++ b/docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md @@ -5,12 +5,20 @@ Status: Approved ## Summary -Session working folders currently live and die with the session record. This -change makes them durable artifacts governed by an age-based retention policy: -`retask sandbox connect` records every session's start time to a per-sandbox log -file, stops deleting folders on delete, and sweeps folders older than a -configurable window. A new `retask sandbox cleanup` command exposes the same -sweep for manual use. +Session working folders survive being stopped. `retask sandbox connect` records +every session's start time to a per-sandbox log file and sweeps folders older +than a configurable window; a new `retask sandbox cleanup` command exposes the +same sweep for manual use. + +Deletion has two triggers, and they are deliberately distinct from stopping: + +- **Explicit delete** (`delete_session`, `delete_sandbox`) reclaims disk + immediately. `delete_sandbox` is a full teardown: it stops sessions, deletes + their folders and the log file, then exits the CLI. +- **Age** reclaims folders left behind by stop and disconnect. + +Stopping a session, stopping a sandbox, and stopping the CLI itself never delete +anything. Separately, the session panel's elapsed timer gains an hours component. @@ -19,7 +27,8 @@ Separately, the session panel's elapsed timer gains an hours component. Three deliverables: 1. Elapsed time renders `h:mm:ss` past one hour — **in `agentfleet`, not this repo**. -2. Session folder retention — session log, no delete-on-remove, hourly sweeper, +2. Session folder lifecycle — session log, explicit-delete teardown for + `delete_session` / `delete_sandbox`, hourly retention sweeper, and a `sandbox cleanup` command. 3. `help-llm` manifest updated for the new command and flags. @@ -167,14 +176,112 @@ filesystem mtime) was considered and explicitly rejected. ### 2.5 Deletion policy -`SessionManager.Remove` drops its `os.RemoveAll` call. It continues to stop the -PTY and drop the fleet card, so `delete_session` still makes the session -disappear from the UI immediately. The folder survives until it ages out or -`sandbox cleanup` reaps it. +Deleting is explicit and immediate; stopping never deletes. The two must not be +conflated — the whole point of the split is that a user can stop work and come +back to their files. + +| Trigger | Session PTYs | Folders | `.json` | CLI | +|---|---|---|---|---| +| User stops CLI (Ctrl-C / `kill`) | SIGTERM | keep | keep | exits | +| `stop_session` | SIGTERM | keep | keep | runs on | +| `stop_sandbox` | SIGTERM (all) | keep | keep | runs on | +| Retention sweep / `cleanup` | untouched (live skipped) | delete aged | drop entries | runs on | +| `delete_session` | SIGTERM | delete one | drop that entry | runs on | +| `delete_sandbox` | SIGTERM (all) | delete all | delete file | **exits** | + +This is enforced structurally: `Stop` and `StopAll` stay pure "signal the +process" operations containing no disk access, and deletion lives only in the +`delete_session` / `delete_sandbox` branches of the data lane. The CLI-stop path +(`connect.go:193`) calls `StopAll` and returns, so it cannot acquire deletion +behaviour by accident. + +**The drain wait.** `PtyAgent.Stop` sends SIGTERM and returns immediately — +it does not wait for the process to exit: + +```go +if err := a.cmd.Process.Signal(syscall.SIGTERM); err != nil { + return a.cmd.Process.Kill() // only if delivery failed, i.e. already gone +} +return nil +``` + +So today's `Remove` already races: it deletes the session folder while the agent +is still handling SIGTERM and may be flushing files into it. SIGTERM's entire +purpose is to grant that grace period, so deleting the directory mid-cleanup +defeats it. Every delete path therefore stops the PTY, waits on `Runner.Done()` +(closed at `runner.go:159` on process exit) up to a bounded timeout, and only +then deletes: + +```go +r.Stop() // SIGTERM +select { +case <-r.Done(): // clean exit +case <-time.After(sessionDrainTimeout): // hung; reap anyway +} +os.RemoveAll(dir) +``` + +A hung process still gets its folder reclaimed after the timeout, so a stuck +agent cannot block teardown indefinitely. Note that agentfleet never escalates +to `SIGKILL` for a process that *ignores* SIGTERM (the `Kill()` above fires only +when delivery fails), so the timeout is the only backstop. `delete_sandbox` +drains sessions concurrently, bounding total teardown at one timeout rather than +one per session. + +`sessionDrainTimeout` is 5 seconds. + +### 2.6 `delete_sandbox` teardown + +`delete_sandbox` tears the whole thing down and exits the CLI — leaving a TUI +attached to a sandbox that no longer exists is not a useful state. -Age becomes the single deletion policy across every path. +Sequence, in `SessionManager`: -### 2.6 Retention sweeper +1. `StopAll` — SIGTERM every live session. +2. Drain — wait on each `Runner.Done()` concurrently, bounded by + `sessionDrainTimeout`. +3. Delete every session folder listed in the log. +4. Delete `.json`. +5. Close the log store (§2.6.1). +6. Return `errSandboxDeleted`, which unwinds `DataLane.Run`. + +Then the CLI exits. `dl.Run(ctx)` is launched as a goroutine +(`connect.go:174`), so its return currently signals nothing — `ctx` is never +cancelled and the TUI keeps running. The fix is to cancel on return: + +```go +go func() { + dl.Run(ctx) + stop() // cancel ctx: unblocks tui.Run / <-ctx.Done(), then StopAll +}() +``` + +`stop()` is the existing `signal.NotifyContext` cancel from `connect.go:94`, so +this reuses the exact path a Ctrl-C already takes — `tui.Run` returns, the +deferred `stop()` is a no-op (cancel is idempotent), and `sm.StopAll()` at +`connect.go:193` runs against an already-empty session map. Calling `stop()` +when `Run` returns for any other reason (ctx already cancelled) is equally +harmless. + +#### 2.6.1 Closing the log store + +Deleting `.json` while the retention sweeper is still alive is a +resurrection hazard: a sweep tick between step 4 and the TUI actually exiting +would rewrite the file we just deleted, leaving a log for a sandbox that no +longer exists. + +Cancelling `ctx` first would stop the sweeper but kill the TUI before teardown +finishes, so ordering alone cannot fix it. Instead the store gets an explicit +`Close()` that latches a `closed` flag under the same mutex that guards writes; +every subsequent mutation becomes a no-op. Teardown is then correct regardless +of how the sweeper and the exit path interleave. + +### 2.7 Retention sweeper + +Retention is the backstop for folders left behind by stop and disconnect — +the paths that deliberately do not delete. Explicit deletes reclaim their own +disk immediately (§2.5), so the sweeper exists for the folders nobody ever +explicitly deleted. `retask sandbox connect` gains `--retention` (default `30d`; `off` disables). When enabled, a goroutine sweeps once at startup and then hourly until the @@ -254,9 +361,17 @@ the repo's help-text template. | `sessionlog_test.go` | `parseRetention`: `30d`, `12h`, `off`, invalid input | | `sessionlog_test.go` | sweep: reaps older-than-window, keeps newer, removes entry + folder together, skips live sessions | | `cleanup_test.go` | multi-log cwd; foreign JSON skipped; `--dry-run` deletes nothing; `--older-than 0` takes all; single-sandbox arg narrows scope | +| `sessionlane_test.go` | `delete_session` deletes folder + drops entry; `delete_sandbox` deletes all folders + the log file; drain waits for `Done()` before deleting; drain gives up after the timeout on a process that ignores SIGTERM | +| `sessionlane_test.go` | **stop does not delete**: `Stop`, `StopAll`, and the CLI-stop path leave folders and log intact | +| `sessionlog_test.go` | `Close()` latches: a sweep after teardown cannot recreate the deleted log file | | `main_test.go` | existing manifest sync test covers the new command and flags | -Sweep tests inject a clock and base directory rather than sleeping. +Sweep tests inject a clock and base directory rather than sleeping. Drain tests +use a fake runner exposing a controllable `Done()` channel, so the SIGTERM-race +and timeout cases are deterministic rather than timing-dependent. + +The "stop does not delete" cases are the regression guard for the distinction in +§2.5 — they fail loudly if deletion ever leaks into a stop path. ## Decisions rejected @@ -264,7 +379,10 @@ Sweep tests inject a clock and base directory rather than sleeping. larger API change than the format warrants. - **Adopting orphan folders via mtime** — would have reaped the pre-existing backlog; rejected in favour of a strict log-only policy. -- **Keeping `os.RemoveAll` on `delete_session`** — would leave "delete" meaning - two different things depending on the path. +- **Deferring `delete_session` folder removal to the sweeper** — briefly adopted, + then reversed: an explicit delete should reclaim its disk immediately rather + than leave the folder sitting for up to the retention window. +- **Deleting folders without draining the PTY** — matches today's behaviour, but + destroys the working directory while the agent is still handling SIGTERM. - **`--after-days 30` (numeric)** — closer to the original phrasing, but splits the duration vocabulary and cannot express sub-day windows. From e0679e7be2d7653c7abe3dfd25a1cb8df352c277 Mon Sep 17 00:00:00 2001 From: nwebbot Date: Fri, 17 Jul 2026 12:04:58 +1000 Subject: [PATCH 03/10] docs: implementation plan for session retention & cleanup 11 TDD tasks across two repos. Task 1 (agentfleet elapsed format) lands upstream first and needs a manual release; Task 11 bumps the dep and is blocked on it. Tasks 2-10 are independent of that. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-07-17-sandbox-session-retention.md | 2357 +++++++++++++++++ 1 file changed, 2357 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-17-sandbox-session-retention.md diff --git a/docs/superpowers/plans/2026-07-17-sandbox-session-retention.md b/docs/superpowers/plans/2026-07-17-sandbox-session-retention.md new file mode 100644 index 0000000..38fc472 --- /dev/null +++ b/docs/superpowers/plans/2026-07-17-sandbox-session-retention.md @@ -0,0 +1,2357 @@ +# Sandbox Session Retention & Cleanup Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Session working folders survive being stopped, are deleted immediately on explicit delete, and are reaped by age otherwise — plus the TUI shows `h:mm:ss` for sessions past an hour. + +**Architecture:** `retask sandbox connect` records each session's start time to `.json` in its working directory. That log is the only source of truth for what may be deleted. Deletion has two triggers: explicit (`delete_session`, `delete_sandbox`) reclaims disk immediately after draining the PTY, and age (an hourly sweeper, or `retask sandbox cleanup`) reaps folders left behind by stop/disconnect. Stopping — a session, a sandbox, or the CLI itself — never deletes anything. + +**Tech Stack:** Go 1.26.4, cobra, testify (`assert`/`require`), `github.com/hoaitan/agentfleet` (TUI + PTY runners), lipgloss. + +**Spec:** `docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md` + +## Global Constraints + +- Two repos, strictly sequenced: **agentfleet first** (Task 1), then retask-cli (Tasks 2–11). Task 11 bumps the dep and requires a released agentfleet tag, which the user cuts manually. +- `gh` is authenticated as `nwebbot` with **READ only** on `hoaitan/agentfleet` — the agentfleet PR must come from a fork. +- Retention default is `30d`; disabled with `off` only. `--retention 0` is an error. +- `--older-than 0` means "everything". `off` is not valid for `--older-than`. +- Sweep interval is exactly `1 * time.Hour`. Drain timeout is exactly `5 * time.Second`. +- Log file lives at `/.json` where `baseDir` is `os.Getwd()` from `connect.go`. Schema version is `1`. +- **Log-only policy:** never delete a `session-*` folder that has no log entry. No mtime fallback, no adoption scan. +- **Stop never deletes.** `Stop`, `StopAll`, and the CLI-stop path must contain no disk access. Deletion lives only in the `delete_session` / `delete_sandbox` branches. +- Named return parameters are required for multi-value returns (repo convention, `CLAUDE.md`). +- Every command's `Long` follows the repo help template: one-line summary, `Usage example:`, `Flags:`. +- Never edit `proto-gen/` by hand. + +--- + +## File Structure + +**agentfleet (fork):** +- Modify: `tui/tui.go` — extract `formatElapsed`, widen the elapsed column. +- Create: `tui/tui_test.go` — table test for `formatElapsed`. + +**retask-cli:** +- Create: `internal/cmd/sandbox/retention.go` — duration parsing (`parseDuration`, `parseRetention`) and the `retentionSweeper`. +- Create: `internal/cmd/sandbox/retention_test.go` +- Create: `internal/cmd/sandbox/sessionlog.go` — the `sessionLog` store: load/record/remove/sweep/destroy. +- Create: `internal/cmd/sandbox/sessionlog_test.go` +- Create: `internal/cmd/sandbox/cleanup.go` — the `sandbox cleanup` command + log discovery. +- Create: `internal/cmd/sandbox/cleanup_test.go` +- Modify: `internal/cmd/sandbox/sessionlane.go` — `drain`, record-on-start, `Remove` teardown, `RemoveAll`. +- Create: `internal/cmd/sandbox/sessionlane_test.go` +- Modify: `internal/cmd/sandbox/datalane.go:162` — `delete_sandbox` calls `RemoveAll`. +- Modify: `internal/cmd/sandbox/connect.go` — `--retention` flag, log store, sweeper, CLI exit on lane return. +- Modify: `internal/cmd/sandbox/command.go:26` — register `newCleanupCommand`. +- Modify: `internal/cmd/helpcmd/command.go:163` — manifest entries. + +Splitting `sessionlog.go` (persistence) from `retention.go` (policy + scheduling) from `cleanup.go` (CLI surface) keeps each file single-purpose; `sessionlane.go` is already 256 lines and only gains teardown logic. + +--- + +## Task 1: agentfleet — `h:mm:ss` elapsed time + +**Repo:** `hoaitan/agentfleet` (fork required — `nwebbot` has READ only) + +**Files:** +- Modify: `tui/tui.go:550-566` (inside `renderCard`) +- Test: `tui/tui_test.go` (create) + +**Interfaces:** +- Consumes: nothing. +- Produces: `formatElapsed(d time.Duration) string` (package-private to `tui`). Released as a new agentfleet tag consumed by Task 11. + +**Context:** `renderCard` currently renders minutes:seconds, so a 90-minute session reads `90:30`. `styleMeta.Width(5)` sizes the column; `1:15:30` needs 7. `renderCard` derives `nameMaxW` from `lipgloss.Width(rightStr)`, so the name column reflows on its own. + +- [ ] **Step 1: Fork and clone** + +```bash +cd /tmp +gh repo fork hoaitan/agentfleet --clone --remote-name upstream --fork-name agentfleet +cd agentfleet +git checkout -b feat/elapsed-hours +``` + +Expected: a fork under `nwebbot/agentfleet`, cloned, with `upstream` pointing at `hoaitan/agentfleet`. + +- [ ] **Step 2: Write the failing test** + +Create `tui/tui_test.go`: + +```go +package tui + +import ( + "testing" + "time" +) + +func TestFormatElapsed(t *testing.T) { + tests := []struct { + name string + in time.Duration + want string + }{ + {"zero", 0, "00:00"}, + {"seconds", 5 * time.Second, "00:05"}, + {"sub-minute rounds down", 5*time.Second + 400*time.Millisecond, "00:05"}, + {"minutes", 90 * time.Second, "01:30"}, + {"just under an hour", 59*time.Minute + 59*time.Second, "59:59"}, + {"exactly one hour", time.Hour, "1:00:00"}, + {"hours minutes seconds", time.Hour + 15*time.Minute + 30*time.Second, "1:15:30"}, + {"multi-day", 25*time.Hour + time.Minute + 2*time.Second, "25:01:02"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := formatElapsed(tc.in); got != tc.want { + t.Errorf("formatElapsed(%v) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} +``` + +Note: agentfleet's existing tests (`tui/chrome_test.go`) use the stdlib `testing` package, not testify. Match that. + +- [ ] **Step 3: Run test to verify it fails** + +Run: `go test ./tui/ -run TestFormatElapsed -v` +Expected: FAIL — `undefined: formatElapsed` + +- [ ] **Step 4: Add `formatElapsed`** + +In `tui/tui.go`, add above `renderCard`: + +```go +// formatElapsed renders a running task's elapsed time. Past one hour it gains +// an hours component, so a long session reads 1:30:00 rather than 90:00. +func formatElapsed(d time.Duration) string { + d = d.Round(time.Second) + if h := int(d.Hours()); h > 0 { + return fmt.Sprintf("%d:%02d:%02d", h, int(d.Minutes())%60, int(d.Seconds())%60) + } + return fmt.Sprintf("%02d:%02d", int(d.Minutes()), int(d.Seconds())%60) +} +``` + +- [ ] **Step 5: Run test to verify it passes** + +Run: `go test ./tui/ -run TestFormatElapsed -v` +Expected: PASS (all 8 subtests) + +- [ ] **Step 6: Call it from `renderCard` and widen the column** + +In `tui/tui.go:550-566`, replace: + +```go + elapsed := "" + if r.Status() == agentfleet.StatusRunning { + d := time.Since(r.StartedAt()).Round(time.Second) + elapsed = fmt.Sprintf("%02d:%02d", int(d.Minutes()), int(d.Seconds())%60) + } +``` + +with: + +```go + elapsed := "" + if r.Status() == agentfleet.StatusRunning { + elapsed = formatElapsed(time.Since(r.StartedAt())) + } +``` + +and replace: + +```go + elapsedStr := styleMeta.Width(5).Render(elapsed) +``` + +with: + +```go + // Width fits "25:01:02"; sub-hour values stay right-sized by padding. + elapsedStr := styleMeta.Width(8).Render(elapsed) +``` + +- [ ] **Step 7: Verify the whole package still builds and passes** + +Run: `go build ./... && go test ./...` +Expected: PASS, no build errors. + +- [ ] **Step 8: Commit and open the PR** + +```bash +git add tui/tui.go tui/tui_test.go +git commit -m "feat(tui): show hours in elapsed time past one hour + +A 90-minute session rendered as 90:30, which reads as a minute count +rather than an hour and a half. Past 1h the timer now renders h:mm:ss. + +The elapsed column widens from 5 to 8 to fit 25:01:02; sub-hour +rendering is unchanged. renderCard derives nameMaxW from the rendered +width, so the name column reflows automatically." +git push -u origin feat/elapsed-hours +gh pr create --repo hoaitan/agentfleet \ + --title "feat(tui): show hours in elapsed time past one hour" \ + --body "Past one hour the task card timer renders \`h:mm:ss\` (\`1:15:30\`) instead of a raw minute count (\`75:30\`). + +- Extracts \`formatElapsed\` as a pure function with a table test. +- Widens the elapsed column 5 → 8 to fit \`25:01:02\`. \`renderCard\` derives \`nameMaxW\` from \`lipgloss.Width(rightStr)\`, so the name column reflows automatically. +- Sub-hour rendering is unchanged. + +Needed by retask-cli, which shows long-lived sandbox sessions and cannot override the format (\`TUIConfig\` exposes no formatting hook)." +``` + +**Known limitation (do not fix):** a session running 100+ hours renders 9 characters into a width-8 column, which lipgloss wraps. That requires a continuously-running 4+ day session; accepted rather than complicating the layout. + +- [ ] **Step 9: Hand off for release** + +Report the PR URL. **STOP** — the user merges and tags the release manually. Tasks 2–10 do not depend on it; only Task 11 does. + +--- + +## Task 2: Duration parsing + +**Files:** +- Create: `internal/cmd/sandbox/retention.go` +- Test: `internal/cmd/sandbox/retention_test.go` + +**Interfaces:** +- Consumes: nothing. +- Produces: + - `parseDuration(s string) (d time.Duration, err error)` — accepts Go duration syntax plus a `d` (days) suffix. Used by `--older-than`. + - `parseRetention(s string) (d time.Duration, enabled bool, err error)` — as above, plus `off`. Used by `--retention`. + +**Context:** `time.ParseDuration` has no day unit, so `30d` fails. Both flags share the grammar but not the keywords: `off` is retention-only, and `0` is meaningful only for `--older-than`. + +- [ ] **Step 1: Write the failing test** + +Create `internal/cmd/sandbox/retention_test.go`: + +```go +package sandbox + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +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 + // make an hourly sweep wipe every folder. Disabling is spelled "off". + _, _, err := parseRetention("0") + assert.Error(t, err) + _, _, err = parseRetention("0d") + assert.Error(t, err) +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestParse' -v` +Expected: FAIL — `undefined: parseDuration` + +- [ ] **Step 3: Implement** + +Create `internal/cmd/sandbox/retention.go`: + +```go +package sandbox + +import ( + "fmt" + "strconv" + "strings" + "time" +) + +// 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 +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestParse' -v` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add internal/cmd/sandbox/retention.go internal/cmd/sandbox/retention_test.go +git commit -m "feat(sandbox): parse retention durations with a day suffix + +time.ParseDuration has no day unit, so 30d needs handling. --retention +additionally accepts off; 0 is rejected there because it means delete +everything for --older-than." +``` + +--- + +## Task 3: Session log store + +**Files:** +- Create: `internal/cmd/sandbox/sessionlog.go` +- Test: `internal/cmd/sandbox/sessionlog_test.go` + +**Interfaces:** +- Consumes: nothing. +- Produces: + - `type sessionEntry struct { Name, Dir string; CreatedAt time.Time }` + - `newSessionLog(baseDir, sandboxID string) *sessionLog` + - `sessionLogPath(baseDir, sandboxID string) string` + - `(*sessionLog) record(sessionID, name, dir string, createdAt time.Time) error` + - `(*sessionLog) remove(sessionID string) error` + - `(*sessionLog) entries() (map[string]sessionEntry, error)` + - `(*sessionLog) destroy() error` + - `loadSessionLogFile(path string) (*sessionLogData, error)` + - `errNewerLog` sentinel + +**Context:** Concurrent `new_session` events and the hourly sweep both mutate the file, so every mutation takes a mutex and rewrites atomically (temp + rename). Read-modify-write per mutation avoids holding stale in-memory state. `destroy()` latches `closed` so a sweep after `delete_sandbox` cannot recreate the file. + +- [ ] **Step 1: Write the failing test** + +Create `internal/cmd/sandbox/sessionlog_test.go`: + +```go +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, "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 cwd can hold ordinary JSON. It must never be read as a session 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, "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, "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, "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()) +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestSessionLog' -v` +Expected: FAIL — `undefined: newSessionLog` + +- [ ] **Step 3: Implement** + +Create `internal/cmd/sandbox/sessionlog.go`: + +```go +package sandbox + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "sync" + "time" +) + +// sessionLogVersion is the schema version written to .json. +const sessionLogVersion = 1 + +// errNewerLog reports a log written by a newer CLI. We skip such files rather +// than rewriting them, so an older binary cannot truncate fields it lost. +var errNewerLog = errors.New("session log written by a newer CLI version") + +// sessionEntry is one recorded session. +type sessionEntry struct { + Name string `json:"name"` + Dir string `json:"dir"` + CreatedAt time.Time `json:"created_at"` +} + +// sessionLogData is the on-disk shape of .json. +type sessionLogData struct { + Version int `json:"version"` + SandboxID string `json:"sandbox_id"` + Sessions map[string]sessionEntry `json:"sessions"` +} + +// sessionLog owns /.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, then 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, 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 from 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]sessionEntry{}, + } + } + 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 so the rename stays 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] = sessionEntry{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]sessionEntry, err error) { + l.mu.Lock() + defer l.mu.Unlock() + d, err := l.load() + if err != nil { + return nil, err + } + out = make(map[string]sessionEntry, len(d.Sessions)) + for k, v := range d.Sessions { + out[k] = v + } + return out, nil +} + +// 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 +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestSessionLog' -v` +Expected: PASS (10 tests) + +- [ ] **Step 5: Commit** + +```bash +git add internal/cmd/sandbox/sessionlog.go internal/cmd/sandbox/sessionlog_test.go +git commit -m "feat(sandbox): add per-sandbox session start-time log + +Records session start times to .json next to the session +folders. The log is the only source of truth for what may be deleted. + +Mutations are read-modify-write under a mutex plus an atomic rename, +since session starts and the retention sweep both write it. Files that +aren't ours (package.json) and logs from newer CLIs are left alone." +``` + +--- + +## Task 4: Sweep by age + +**Files:** +- Modify: `internal/cmd/sandbox/sessionlog.go` (add `sweep`) +- Test: `internal/cmd/sandbox/sessionlog_test.go` (append) + +**Interfaces:** +- Consumes: `sessionLog`, `sessionEntry` (Task 3). +- Produces: `(*sessionLog) sweep(baseDir string, now time.Time, olderThan time.Duration, skip func(string) bool, dryRun bool) (deleted []string, err error)` — deletes folders whose entry is at least `olderThan` old, returns sorted session ids. `olderThan == 0` matches everything. `skip` may be nil. + +**Context:** One sweep function serves both the hourly goroutine (which passes `sm.isActive` so a long-running session never has its cwd deleted) and the `cleanup` command (which passes nil). `now` is injected so tests never sleep. + +- [ ] **Step 1: Write the failing test** + +Append to `internal/cmd/sandbox/sessionlog_test.go`: + +```go +// 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, "sb-1.json")) + assert.True(t, os.IsNotExist(err)) +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestSweep' -v` +Expected: FAIL — `l.sweep undefined` + +- [ ] **Step 3: Implement** + +Add to `internal/cmd/sandbox/sessionlog.go` (and add `"sort"` to the imports): + +```go +// 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 +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestSweep' -v` +Expected: PASS (6 tests) + +- [ ] **Step 5: Commit** + +```bash +git add internal/cmd/sandbox/sessionlog.go internal/cmd/sandbox/sessionlog_test.go +git commit -m "feat(sandbox): sweep logged session folders by age + +One sweep serves both the hourly goroutine and the cleanup command. The +skip predicate keeps the sweeper from deleting a live session's own +working directory when it outlives the retention window. Folders with no +log entry are never touched." +``` + +--- + +## Task 5: Record sessions on start + +**Files:** +- Modify: `internal/cmd/sandbox/sessionlane.go:20-61` (SessionManager fields + constructor), `:65-108` (Start) +- Modify: `internal/cmd/sandbox/connect.go:162-171` (construct the log, pass it in) +- Test: `internal/cmd/sandbox/sessionlane_test.go` (create) + +**Interfaces:** +- Consumes: `newSessionLog`, `(*sessionLog) record` (Task 3). +- Produces: + - `SessionManager.log *sessionLog` field. + - `newSessionManager(..., log *sessionLog, autoRespond bool) *SessionManager` — `log` is added as the **second-to-last** parameter, immediately before `autoRespond`. + - `(*SessionManager) isActive(sessionID string) bool` — used by the sweeper in Task 8. + +**Context:** The entry is recorded **before** `sb.Run` because `SessionBootstrap.setupFolder` creates the folder early (`sessionBootstrap.go:238`) but `Run` can fail later at git clone or config write. Under the log-only policy a folder with no entry can never be cleaned, so recording after success would leak every failed bootstrap permanently. + +**Naming note:** `SessionManager` already has a field named `log` (the +`*slog.Logger`). The new field is therefore named `sessionLog`, and every task +below uses `sm.sessionLog`. + +- [ ] **Step 1: Write the failing test** + +Create `internal/cmd/sandbox/sessionlane_test.go`: + +```go +package sandbox + +import ( + "testing" + "time" + + agentfleet "github.com/hoaitan/agentfleet" + "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. +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]*agentfleet.Runner{}, + } +} + +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") +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestRecordSessionStart|TestIsActive' -v` +Expected: FAIL — `sm.recordSessionStart undefined`, `unknown field sessionLog` + +- [ ] **Step 3: Add the field, constructor param, and helpers** + +In `internal/cmd/sandbox/sessionlane.go`, add to the `SessionManager` struct immediately after `endpoint string`: + +```go + sessionLog *sessionLog // records session start times for retention +``` + +Update the constructor signature (add `sessionLog` immediately before `autoRespond`): + +```go +func newSessionManager( + sandboxID, wsBase string, + fleet *agentfleet.Fleet, + fleetCfg agentfleet.FleetConfig, + agentCfg agentfleet.AgentConfig, + log *slog.Logger, + workspaceID, sandboxName, baseDir, endpoint string, + sessionLog *sessionLog, + autoRespond bool, +) *SessionManager { +``` + +and inside the returned struct literal add: + +```go + sessionLog: sessionLog, +``` + +Add these methods: + +```go +// 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 +} +``` + +Add `"time"` to the imports. + +- [ ] **Step 4: Call it from Start, before bootstrap** + +In `sessionlane.go`, in `Start`, immediately before the `sb := &SessionBootstrap{...}` literal (currently line 90), insert: + +```go + // 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()) +``` + +- [ ] **Step 5: Update the caller in connect.go** + +In `internal/cmd/sandbox/connect.go`, after `baseDir, err := os.Getwd()` (line 157-160) add: + +```go + sessLog := newSessionLog(baseDir, sandboxID) +``` + +and pass it in the `newSessionManager` call, before `autoRespond`: + +```go + sm := newSessionManager( + sandboxID, wsBase, + fleet, fleetCfg.Fleet, fleetCfg.Agent, + logger, + sbResp.Msg.WorkspaceId, + sbResp.Msg.Name, + baseDir, + profile.Endpoint, + sessLog, + autoRespond, + ) +``` + +- [ ] **Step 6: Run tests and build** + +Run: `go build ./... && go test ./internal/cmd/sandbox/ -v` +Expected: PASS + +- [ ] **Step 7: Commit** + +```bash +git add internal/cmd/sandbox/sessionlane.go internal/cmd/sandbox/sessionlane_test.go internal/cmd/sandbox/connect.go +git commit -m "feat(sandbox): record session start times on session start + +The entry is written before bootstrap, not after: setupFolder creates +the folder early but bootstrap can fail later, and under the log-only +retention policy an unlogged folder can never be reaped." +``` + +--- + +## Task 6: Drain the PTY before deleting, and delete on `delete_session` + +**Files:** +- Modify: `internal/cmd/sandbox/sessionlane.go:203-214` (`Remove`) +- Test: `internal/cmd/sandbox/sessionlane_test.go` (append) + +**Interfaces:** +- Consumes: `(*sessionLog) remove` (Task 3), `isActive` (Task 5). +- Produces: + - `const sessionDrainTimeout = 5 * time.Second` + - `type stoppableRunner interface { Stop() error; Done() <-chan struct{} }` + - `drain(r stoppableRunner, timeout time.Duration)` + +**Context:** `PtyAgent.Stop` sends SIGTERM and returns immediately — it does not wait for the process. Today's `Remove` deletes the folder right after, racing the agent's own cleanup. SIGTERM exists precisely to grant that grace period. agentfleet never escalates to SIGKILL for a process that *ignores* SIGTERM (its `Kill()` fires only when signal *delivery* fails), so the timeout is the only backstop. + +- [ ] **Step 1: Write the failing test** + +Append to `internal/cmd/sandbox/sessionlane_test.go`: + +```go +// 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) // 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) }) +} + +func TestRemoveDeletesFolderAndLogEntry(t *testing.T) { + dir := t.TempDir() + sm := newTestSessionManager(t, dir) + now := time.Now().UTC() + sessDir := mkSession(t, sm.sessionLog, dir, "sess-a", now) + + 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") +} + +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") +} +``` + +Add `"os"` to the test imports. + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestDrain|TestRemove|TestStopDoesNot' -v` +Expected: FAIL — `undefined: drain` + +- [ ] **Step 3: Implement drain and rewrite Remove** + +In `internal/cmd/sandbox/sessionlane.go`, add near the top (after the imports): + +```go +// 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): + } +} +``` + +Replace `Remove` (currently lines 203-214) with: + +```go +// 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() + r := sm.sessions[sessionID] + delete(sm.sessions, sessionID) + sm.mu.Unlock() + + if r != nil { + drain(r, sessionDrainTimeout) + sm.fleet.Remove(sessionID) + } + 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) + } + } +} +``` + +Note `sm.fleet` is nil in tests, so `Remove` must only touch it when `r != nil` — which the test relies on (it registers no runner). + +- [ ] **Step 4: Run test to verify it passes** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestDrain|TestRemove|TestStopDoesNot' -v` +Expected: PASS (5 tests) + +- [ ] **Step 5: Full package test + build** + +Run: `go build ./... && go test ./internal/cmd/sandbox/` +Expected: PASS + +- [ ] **Step 6: Commit** + +```bash +git add internal/cmd/sandbox/sessionlane.go internal/cmd/sandbox/sessionlane_test.go +git commit -m "fix(sandbox): drain the PTY before deleting a session folder + +PtyAgent.Stop returns when SIGTERM is delivered, not when the process +exits, so deleting straight after it destroyed the working directory +while the agent was still flushing into it. delete_session now waits for +Runner.Done() (5s cap) before deleting, and drops the log entry too. + +agentfleet never escalates to SIGKILL for a process that ignores +SIGTERM, so the timeout is the only backstop. + +Adds a regression test that Stop/StopAll never delete." +``` + +--- + +## Task 7: `delete_sandbox` full teardown + +**Files:** +- Modify: `internal/cmd/sandbox/sessionlane.go` (add `RemoveAll`) +- Modify: `internal/cmd/sandbox/datalane.go:162-167` +- Modify: `internal/cmd/sandbox/connect.go:174` +- Test: `internal/cmd/sandbox/sessionlane_test.go` (append) + +**Interfaces:** +- Consumes: `drain`, `sessionDrainTimeout` (Task 6), `(*sessionLog) entries`/`destroy` (Task 3). +- Produces: `(*SessionManager) RemoveAll()` — stop, drain concurrently, delete every logged folder, delete the log file, latch the store closed. + +**Context:** `dl.Run(ctx)` is a goroutine (`connect.go:174`), so returning `errSandboxDeleted` currently signals nothing — `ctx` is never cancelled and the TUI keeps running against a sandbox that no longer exists. Cancelling on return reuses the exact path Ctrl-C takes. Deleting the log file while the sweeper is alive would let a later tick recreate it, which `destroy()`'s latch prevents. + +- [ ] **Step 1: Write the failing test** + +Append to `internal/cmd/sandbox/sessionlane_test.go`: + +```go +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") +} +``` + +Add `"path/filepath"` to the test imports. + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestRemoveAll' -v` +Expected: FAIL — `sm.RemoveAll undefined` + +- [ ] **Step 3: Implement RemoveAll** + +Add to `internal/cmd/sandbox/sessionlane.go` after `Remove`: + +```go +// 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() + runners := make(map[string]*agentfleet.Runner, len(sm.sessions)) + for id, r := range sm.sessions { + runners[id] = r + } + sm.sessions = make(map[string]*agentfleet.Runner) + sm.mu.Unlock() + + var wg sync.WaitGroup + for id, r := range runners { + wg.Add(1) + go func() { + defer wg.Done() + drain(r, 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. + entries, err := sm.sessionLog.entries() + if err != nil { + sm.logError("session_log_read_failed", "sandbox_id", sm.sandboxID, "error", err) + } + for id, e := range entries { + 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) + } +} +``` + +`sync` is already imported by this file. + +- [ ] **Step 4: Run test to verify it passes** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestRemoveAll' -v` +Expected: PASS (3 tests) + +- [ ] **Step 5: Wire delete_sandbox to RemoveAll** + +In `internal/cmd/sandbox/datalane.go`, replace the `delete_sandbox` case (lines 162-167): + +```go + case "delete_sandbox": + dl.logInfo("delete_sandbox", "sandbox_id", msg.SandboxID) + dl.sessions.RemoveAll() + conn.Close(websocket.StatusNormalClosure, "deleted") //nolint:errcheck + return errSandboxDeleted +``` + +- [ ] **Step 6: Exit the CLI when the data lane ends** + +In `internal/cmd/sandbox/connect.go`, replace line 174: + +```go + go dl.Run(ctx) +``` + +with: + +```go + // 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() + }() +``` + +- [ ] **Step 7: Build and run the full suite** + +Run: `go build ./... && go test ./...` +Expected: PASS + +- [ ] **Step 8: Commit** + +```bash +git add internal/cmd/sandbox/sessionlane.go internal/cmd/sandbox/sessionlane_test.go internal/cmd/sandbox/datalane.go internal/cmd/sandbox/connect.go +git commit -m "feat(sandbox): tear down fully on delete_sandbox + +delete_sandbox now stops and drains every session, deletes every logged +folder and the log file, then exits the CLI — previously the data lane +goroutine just returned, leaving a TUI attached to a sandbox that no +longer existed. + +Closing the log store matters: the retention sweeper can outlive the +file deletion and would otherwise recreate a log for a dead sandbox." +``` + +--- + +## Task 8: Retention sweeper and `--retention` + +**Files:** +- Modify: `internal/cmd/sandbox/retention.go` (add `retentionSweeper`) +- Modify: `internal/cmd/sandbox/connect.go` (flag + goroutine + help text) +- Test: `internal/cmd/sandbox/retention_test.go` (append) + +**Interfaces:** +- Consumes: `(*sessionLog) sweep` (Task 4), `(*SessionManager) isActive` (Task 5), `parseRetention` (Task 2). +- Produces: + - `const retentionSweepInterval = time.Hour` + - `type retentionSweeper struct { log *sessionLog; baseDir string; window, interval time.Duration; isActive func(string) bool; logger *slog.Logger }` + - `(*retentionSweeper) Run(ctx context.Context)` — sweeps once immediately, then every `interval` until ctx is cancelled. + - `(*retentionSweeper) once()` + +**Context:** Retention is the backstop for folders left by stop/disconnect — explicit deletes reclaim their own disk. Sweeping once at startup means a machine reconnecting after a month cleans up without waiting an hour. + +- [ ] **Step 1: Write the failing test** + +Append to `internal/cmd/sandbox/retention_test.go`: + +```go +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") + } +} +``` + +Add `"context"`, `"os"`, `"path/filepath"` to the test imports. + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestSweeper' -v` +Expected: FAIL — `undefined: retentionSweeper` + +- [ ] **Step 3: Implement the sweeper** + +Append to `internal/cmd/sandbox/retention.go` (adding `"context"` and `"log/slog"` to the imports): + +```go +// 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()) + } +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestSweeper' -v` +Expected: PASS + +- [ ] **Step 5: Add the flag and start the sweeper** + +In `internal/cmd/sandbox/connect.go`, add to the var block at the top of `newConnectCommand`: + +```go + var retention string +``` + +Register the flag next to the others (after the `--no-auto-respond` registration): + +```go + cmd.Flags().StringVar(&retention, "retention", "30d", `Delete session folders older than this (e.g. 30d, 12h); "off" disables`) +``` + +In `RunE`, validate early — next to the existing `--mode` validation (line 77-79): + +```go + retentionWindow, retentionOn, err := parseRetention(retention) + if err != nil { + return err + } +``` + +Note: `err` is already declared later in `RunE` via `cfg, err := config.Load(path)`; because this new statement comes first and uses `:=` with two new variables, it compiles. Verify with the build in Step 7. + +After the `sm := newSessionManager(...)` block and before `dl := newDataLane(...)`, add: + +```go + if retentionOn { + sweeper := &retentionSweeper{ + log: sessLog, + baseDir: baseDir, + window: retentionWindow, + interval: retentionSweepInterval, + isActive: sm.isActive, + logger: logger, + } + go sweeper.Run(ctx) + } +``` + +Update the command's `Long` to document the flag (repo help template). Replace the `Flags:` block: + +``` +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) +``` + +and add to the usage examples: + +``` + retask sandbox connect sandbox_abc123 --retention 7d + retask sandbox connect sandbox_abc123 --retention off +``` + +- [ ] **Step 6: Verify the flag rejects bad input** + +Append to `internal/cmd/sandbox/retention_test.go`: + +```go +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") +} +``` + +Add `"github.com/nwebxyz/retask-cli/internal/flags"` to the test imports. + +- [ ] **Step 7: Build and test** + +Run: `go build ./... && go test ./internal/cmd/sandbox/` +Expected: PASS + +- [ ] **Step 8: Commit** + +```bash +git add internal/cmd/sandbox/retention.go internal/cmd/sandbox/retention_test.go internal/cmd/sandbox/connect.go +git commit -m "feat(sandbox): sweep aged session folders hourly on connect + +--retention 30d (default) deletes logged session folders older than the +window, checked at startup and then hourly; --retention off disables it. + +Live sessions are skipped, so a session outliving the window never has +its own working directory deleted underneath it." +``` + +--- + +## Task 9: `retask sandbox cleanup` + +**Files:** +- Create: `internal/cmd/sandbox/cleanup.go` +- Test: `internal/cmd/sandbox/cleanup_test.go` +- Modify: `internal/cmd/sandbox/command.go:26-35` (register the command) + +**Interfaces:** +- Consumes: `parseDuration` (Task 2), `newSessionLog`, `loadSessionLogFile`, `errNewerLog` (Task 3), `(*sessionLog) sweep` (Task 4). +- Produces: + - `newCleanupCommand(gf *flags.Global) *cobra.Command` + - `discoverSessionLogs(baseDir string) (logs []*sessionLog, err error)` + - `confirm(in io.Reader, out io.Writer, prompt string) bool` + +**Context:** Bare `cleanup` sweeps every valid log in cwd; an id narrows it. Files failing the schema check are skipped, which is what protects `package.json`. `--older-than 0` prompts, because a separate process cannot know which sessions another process has live. + +- [ ] **Step 1: Write the failing test** + +Create `internal/cmd/sandbox/cleanup_test.go`: + +```go +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 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") +} + +// --- 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() +} +``` + +Note: these tests `os.Chdir`, so they must not run in parallel — do not add `t.Parallel()`. + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestCleanup|TestDiscover|TestConfirm' -v` +Expected: FAIL — `undefined: newCleanupCommand` + +- [ ] **Step 3: Implement** + +Create `internal/cmd/sandbox/cleanup.go`: + +```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 .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 that +// fails 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" +} +``` + +- [ ] **Step 4: Register the command** + +In `internal/cmd/sandbox/command.go`, add to the `AddCommand` block (after `newAttachCommand(gf)`): + +```go + newCleanupCommand(gf), +``` + +- [ ] **Step 5: Run test to verify it passes** + +Run: `go test ./internal/cmd/sandbox/ -run 'TestCleanup|TestDiscover|TestConfirm' -v` +Expected: PASS (10 tests) + +- [ ] **Step 6: Try it by hand** + +```bash +go build -o /tmp/retask ./cmd/retask/ +mkdir -p /tmp/cleanup-demo && cd /tmp/cleanup-demo +mkdir -p session-demo +printf '{\n "version": 1,\n "sandbox_id": "sb-demo",\n "sessions": {\n "demo": {"name":"demo","dir":"session-demo","created_at":"2020-01-01T00:00:00Z"}\n }\n}\n' > sb-demo.json +printf '{"name":"app"}' > package.json +/tmp/retask sandbox cleanup --dry-run +``` + +Expected: reports `sb-demo demo` and `1 session folder(s) would be deleted (--dry-run).`; `session-demo` and `package.json` both still present. + +```bash +/tmp/retask sandbox cleanup +ls +``` + +Expected: `Deleted 1 session folder(s).`; `session-demo` gone, `package.json` untouched, `sb-demo.json` now has an empty `sessions` map. + +- [ ] **Step 7: Commit** + +```bash +git add internal/cmd/sandbox/cleanup.go internal/cmd/sandbox/cleanup_test.go internal/cmd/sandbox/command.go +git commit -m "feat(sandbox): add sandbox cleanup command + +Sweeps every session log in the working directory (or one named +sandbox). --older-than 0 deletes everything and prompts first, since a +separate process cannot know which sessions are live; --yes skips the +prompt and --dry-run reports without deleting. + +Files that fail the log schema check are skipped, so package.json and +friends are never touched." +``` + +--- + +## Task 10: `help-llm` manifest + +**Files:** +- Modify: `internal/cmd/helpcmd/command.go:163` (connect entry) and add a cleanup entry +- Test: `cmd/retask/main_test.go` (existing sync test — no new test needed) + +**Interfaces:** +- Consumes: the command tree from Tasks 8 and 9. +- Produces: nothing consumed by later tasks. + +**Context:** `cmd/retask/main_test.go:83` asserts the hand-maintained manifest matches the real command tree in both directions — an undocumented flag or an undocumented command fails the suite. This task exists to satisfy it. + +- [ ] **Step 1: Run the sync test to see it fail** + +Run: `go test ./cmd/retask/ -run TestHelpLLM -v` +Expected: FAIL — `retask sandbox cleanup` is not documented, and `retask sandbox connect` flags drift (missing `--retention`). + +If the test name differs, find it with: `grep -n "func Test" cmd/retask/main_test.go` + +- [ ] **Step 2: Update the connect entry** + +In `internal/cmd/helpcmd/command.go`, replace line 163: + +```go + {Command: "retask sandbox connect", Description: "Connect this machine as a Private VM sandbox (long-running)", Flags: []string{"--mode", "--auto-open", "--no-auto-respond"}, Example: "retask sandbox connect "}, +``` + +with: + +```go + {Command: "retask sandbox connect", Description: "Connect this machine as a Private VM sandbox (long-running). Session folders are created in the current directory and recorded in .json. --retention deletes folders older than the window (checked hourly); \"off\" disables it. Live sessions are never deleted", Flags: []string{"--mode", "--auto-open", "--no-auto-respond", "--retention"}, Example: "retask sandbox connect --retention 30d"}, +``` + +- [ ] **Step 3: Add the cleanup entry** + +Immediately after the `retask sandbox attach` entry (line 164), add: + +```go + {Command: "retask sandbox cleanup", Description: "Delete session folders left by stopped sessions, in the current directory. Only folders recorded in a .json session log are considered. With no argument every log in the directory is swept; pass a sandbox id to narrow it. --older-than 0 deletes everything and prompts unless --yes", Flags: []string{"--older-than", "--dry-run", "--yes"}, Example: "retask sandbox cleanup --older-than 7d"}, +``` + +- [ ] **Step 4: Run the sync test to verify it passes** + +Run: `go test ./cmd/retask/ -v` +Expected: PASS + +- [ ] **Step 5: Eyeball the manifest** + +```bash +go build -o /tmp/retask ./cmd/retask/ +/tmp/retask help-llm | jq '.commands[] | select(.command | contains("sandbox cleanup") or contains("sandbox connect"))' +``` + +Expected: both entries present, `--retention` on connect, three flags on cleanup. + +- [ ] **Step 6: Run everything** + +Run: `go build ./... && go test ./...` +Expected: PASS + +- [ ] **Step 7: Commit** + +```bash +git add internal/cmd/helpcmd/command.go +git commit -m "docs(help-llm): document sandbox cleanup and --retention" +``` + +--- + +## Task 11: Bump agentfleet and open the retask-cli PR + +**Files:** +- Modify: `go.mod`, `go.sum` + +**Interfaces:** +- Consumes: the agentfleet release from Task 1. +- Produces: the final PR. + +**BLOCKED** until the user merges Task 1's PR and pushes a tag. Confirm the released version before starting. + +- [ ] **Step 1: Bump the dependency** + +```bash +go get github.com/hoaitan/agentfleet@ +go mod tidy +``` + +Replace `` with the tag the user cut (e.g. `v0.6.28`). + +- [ ] **Step 2: Verify the build and suite** + +Run: `go build ./... && go test ./...` +Expected: PASS + +- [ ] **Step 3: Verify the elapsed format is actually live** + +```bash +grep -n "Width(8)" $(go env GOMODCACHE)/github.com/hoaitan/agentfleet@/tui/tui.go +``` + +Expected: the widened column is present — confirming the tag contains Task 1's change rather than an older commit. + +- [ ] **Step 4: Commit** + +```bash +git add go.mod go.sum +git commit -m "chore(deps): bump agentfleet for h:mm:ss elapsed time + +Picks up the TUI change that renders hours in the session panel's +elapsed timer past one hour." +``` + +- [ ] **Step 5: Push and open the PR** + +```bash +git push -u origin feat/sandbox-session-retention +gh pr create --title "feat(sandbox): session folder retention and cleanup" --body "$(cat <<'BODY' +Session working folders now survive being stopped, are deleted immediately on an explicit delete, and are reaped by age otherwise. + +## What changed + +- **Session log.** `sandbox connect` records each session's start time to `.json` beside the session folders. It is the only source of truth for what may be deleted — a `session-*` folder with no entry is never touched. The entry is written *before* bootstrap, since bootstrap creates the folder early but can fail later. +- **Explicit delete reclaims disk now.** `delete_session` deletes the folder and its entry. `delete_sandbox` is a full teardown: stop, drain, delete every logged folder, delete the log file, exit the CLI — previously it left a TUI attached to a sandbox that no longer existed. +- **Drain before delete.** `PtyAgent.Stop` returns when SIGTERM is *delivered*, not when the process exits, so deleting straight after it destroyed the working directory while the agent was still flushing into it. Every delete path now waits on `Runner.Done()` (5s cap) first. +- **Retention.** `--retention 30d` (default, `off` disables) sweeps aged folders at startup and hourly. Live sessions are skipped, so a session outliving the window keeps its own cwd. +- **`retask sandbox cleanup`.** Manual sweep of every log in the working directory, or one named sandbox. `--older-than 0` takes everything (prompts unless `--yes`), `--dry-run` reports only. +- **agentfleet bump** for `h:mm:ss` elapsed time past one hour. + +## Notes for reviewers + +- **Stop still never deletes** — `Stop`, `StopAll`, and the CLI-stop path contain no disk access, with regression tests pinning that. +- **Log-only, by design.** Folders already on disk before this ships have no log entry and are never auto-reaped; they need a manual `rm`. + +Design: `docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md` +Plan: `docs/superpowers/plans/2026-07-17-sandbox-session-retention.md` + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +BODY +)" +``` + +--- + +## Verification checklist + +Run before calling the work done: + +- [ ] `go build ./... && go test ./...` passes in retask-cli +- [ ] `go test ./...` passes in the agentfleet fork +- [ ] `retask sandbox cleanup --dry-run` in a folder containing `package.json` leaves it untouched +- [ ] `retask help-llm | jq '.commands[] | select(.command | contains("cleanup"))'` returns the entry +- [ ] `retask sandbox connect --retention off` starts with no sweeper +- [ ] `retask sandbox connect --retention 0` errors, pointing at `off` From 8bd153dc9c51a51ec91fcd8358572cee43dc015c Mon Sep 17 00:00:00 2001 From: nwebbot Date: Fri, 17 Jul 2026 12:08:06 +1000 Subject: [PATCH 04/10] feat(sandbox): parse retention durations with a day suffix time.ParseDuration has no day unit, so 30d needs handling. --retention additionally accepts off; 0 is rejected there because it means delete everything for --older-than. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cmd/sandbox/retention.go | 53 +++++++++++++++++++++++ internal/cmd/sandbox/retention_test.go | 58 ++++++++++++++++++++++++++ 2 files changed, 111 insertions(+) create mode 100644 internal/cmd/sandbox/retention.go create mode 100644 internal/cmd/sandbox/retention_test.go diff --git a/internal/cmd/sandbox/retention.go b/internal/cmd/sandbox/retention.go new file mode 100644 index 0000000..ce10b1f --- /dev/null +++ b/internal/cmd/sandbox/retention.go @@ -0,0 +1,53 @@ +// internal/cmd/sandbox/retention.go +package sandbox + +import ( + "fmt" + "strconv" + "strings" + "time" +) + +// 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..54eaff6 --- /dev/null +++ b/internal/cmd/sandbox/retention_test.go @@ -0,0 +1,58 @@ +package sandbox + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +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) +} From f74775a27d0896115d7015d8d098fe6dc323480a Mon Sep 17 00:00:00 2001 From: nwebbot Date: Fri, 17 Jul 2026 12:09:24 +1000 Subject: [PATCH 05/10] feat(sandbox): add per-sandbox session log with age-based sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records session start times to .json next to the session folders. The log is the only source of truth for what may be deleted — a session-* folder with no entry is never touched. Mutations are read-modify-write under a mutex plus an atomic rename, since session starts and the retention sweep both write it. Files that aren't ours (package.json) and logs from newer CLIs are left alone. One sweep serves both the hourly goroutine and the cleanup command. Its skip predicate keeps a live session's own working directory from being deleted when it outlives the retention window. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cmd/sandbox/sessionlog.go | 241 +++++++++++++++++++++++ internal/cmd/sandbox/sessionlog_test.go | 245 ++++++++++++++++++++++++ 2 files changed, 486 insertions(+) create mode 100644 internal/cmd/sandbox/sessionlog.go create mode 100644 internal/cmd/sandbox/sessionlog_test.go diff --git a/internal/cmd/sandbox/sessionlog.go b/internal/cmd/sandbox/sessionlog.go new file mode 100644 index 0000000..f4ac268 --- /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 .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") + +// sessionEntry is one recorded session. +type sessionEntry struct { + Name string `json:"name"` + Dir string `json:"dir"` + CreatedAt time.Time `json:"created_at"` +} + +// sessionLogData is the on-disk shape of .json. +type sessionLogData struct { + Version int `json:"version"` + SandboxID string `json:"sandbox_id"` + Sessions map[string]sessionEntry `json:"sessions"` +} + +// sessionLog owns /.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, 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]sessionEntry{}, + } + } + 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] = sessionEntry{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]sessionEntry, err error) { + l.mu.Lock() + defer l.mu.Unlock() + d, err := l.load() + if err != nil { + return nil, err + } + out = make(map[string]sessionEntry, 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..346aa2e --- /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, "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, "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, "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, "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, "sb-1.json")) + assert.True(t, os.IsNotExist(err)) +} From 0a2ac9ad42f00b1deec48ffd4b12629a85882421 Mon Sep 17 00:00:00 2001 From: nwebbot Date: Fri, 17 Jul 2026 12:11:54 +1000 Subject: [PATCH 06/10] feat(sandbox): explicit-delete teardown and hourly retention sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sessions are recorded before bootstrap, since setupFolder creates the folder early but bootstrap can fail later — an unlogged folder could never be reaped. Every delete path now drains the PTY first. PtyAgent.Stop returns when SIGTERM is delivered, not when the process exits, so deleting straight after it destroyed the working directory while the agent was still flushing into it. agentfleet never escalates to SIGKILL for a process that ignores SIGTERM, so the 5s timeout is the only backstop. delete_session deletes the folder and entry. delete_sandbox stops and drains every session, deletes every logged folder and the log file, then exits the CLI — previously the data lane goroutine just returned, leaving a TUI attached to a sandbox that no longer existed. Closing the log store stops a late sweep tick from recreating the deleted file. --retention 30d (default; off disables) sweeps at startup and hourly, skipping live sessions. Stop, StopAll and the CLI-stop path stay deletion-free, with a regression test pinning that. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cmd/sandbox/connect.go | 37 ++++- internal/cmd/sandbox/datalane.go | 2 +- internal/cmd/sandbox/retention.go | 48 ++++++ internal/cmd/sandbox/retention_test.go | 52 ++++++ internal/cmd/sandbox/sessionlane.go | 129 ++++++++++++++- internal/cmd/sandbox/sessionlane_test.go | 198 +++++++++++++++++++++++ 6 files changed, 459 insertions(+), 7 deletions(-) create mode 100644 internal/cmd/sandbox/sessionlane_test.go diff --git a/internal/cmd/sandbox/connect.go b/internal/cmd/sandbox/connect.go index b470eb0..066bd6a 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 cmd := &cobra.Command{ Use: "connect ", Short: "Connect this machine as a Private VM sandbox", @@ -58,15 +59,22 @@ 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 +.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 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) Environment: SANDBOX_PROXY_ENDPOINT Proxy base URL (default: https://sandbox-proxy.prd.nweb.app/) @@ -77,6 +85,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. @@ -158,6 +170,7 @@ Environment: if err != nil { return err } + sessLog := newSessionLog(baseDir, sandboxID) autoRespond := !(noAutoRespond || os.Getenv("RETASK_SANDBOX_NO_AUTO_RESPOND") == "1") sm := newSessionManager( sandboxID, wsBase, @@ -167,11 +180,32 @@ Environment: sbResp.Msg.Name, baseDir, profile.Endpoint, + sessLog, autoRespond, ) + + 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() @@ -197,6 +231,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`) return cmd } diff --git a/internal/cmd/sandbox/datalane.go b/internal/cmd/sandbox/datalane.go index b06ca0f..203a592 100644 --- a/internal/cmd/sandbox/datalane.go +++ b/internal/cmd/sandbox/datalane.go @@ -161,7 +161,7 @@ func (dl *DataLane) connectOnce(ctx context.Context) error { 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 errSandboxDeleted } diff --git a/internal/cmd/sandbox/retention.go b/internal/cmd/sandbox/retention.go index ce10b1f..522e9c2 100644 --- a/internal/cmd/sandbox/retention.go +++ b/internal/cmd/sandbox/retention.go @@ -2,12 +2,60 @@ 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) { diff --git a/internal/cmd/sandbox/retention_test.go b/internal/cmd/sandbox/retention_test.go index 54eaff6..2a4925b 100644 --- a/internal/cmd/sandbox/retention_test.go +++ b/internal/cmd/sandbox/retention_test.go @@ -1,11 +1,16 @@ 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) { @@ -56,3 +61,50 @@ func TestParseRetentionRejectsZero(t *testing.T) { _, _, 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 ac65406..99cd999 100644 --- a/internal/cmd/sandbox/sessionlane.go +++ b/internal/cmd/sandbox/sessionlane.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "sync" + "time" "github.com/coder/websocket" agentfleet "github.com/hoaitan/agentfleet" @@ -17,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): + } +} + // SessionManager creates and tracks one agentfleet Runner per active sandbox session. type SessionManager struct { sandboxID string @@ -29,7 +60,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) mu sync.Mutex sessions map[string]*agentfleet.Runner // keyed by session_id @@ -42,6 +74,7 @@ func newSessionManager( agentCfg agentfleet.AgentConfig, log *slog.Logger, workspaceID, sandboxName, baseDir, endpoint string, + sessLog *sessionLog, autoRespond bool, ) *SessionManager { return &SessionManager{ @@ -55,11 +88,34 @@ func newSessionManager( sandboxName: sandboxName, baseDir: baseDir, endpoint: endpoint, + sessionLog: sessLog, autoRespond: autoRespond, sessions: make(map[string]*agentfleet.Runner), } } +// 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: connects the session lane, runs bootstrap, // then launches the PTY and bridges it to the session lane. func (sm *SessionManager) Start(ctx context.Context, sessionID, token, name string, configJSON json.RawMessage, systemPrompt, seedPrompt string) { @@ -86,6 +142,10 @@ func (sm *SessionManager) Start(ctx context.Context, sessionID, token, name stri } 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()) + // Run bootstrap — writes files, clones repos, builds env. sb := &SessionBootstrap{ SessionID: sessionID, @@ -198,19 +258,78 @@ func (sm *SessionManager) Stop(sessionID string) { } } -// Remove stops the session's PTY and removes it from the fleet so it -// disappears from the TUI immediately. Used for delete_session messages. +// 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() r := sm.sessions[sessionID] delete(sm.sessions, sessionID) sm.mu.Unlock() + if r != nil { - r.Stop() //nolint:errcheck + drain(r, 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() + runners := make(map[string]*agentfleet.Runner, len(sm.sessions)) + for id, r := range sm.sessions { + runners[id] = r + } + sm.sessions = make(map[string]*agentfleet.Runner) + sm.mu.Unlock() + + var wg sync.WaitGroup + for id, r := range runners { + wg.Add(1) + go func() { + defer wg.Done() + drain(r, 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. + entries, err := sm.sessionLog.entries() + if err != nil { + sm.logError("session_log_read_failed", "sandbox_id", sm.sandboxID, "error", err) + } + for id, e := range entries { + 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..4b9ac62 --- /dev/null +++ b/internal/cmd/sandbox/sessionlane_test.go @@ -0,0 +1,198 @@ +package sandbox + +import ( + "os" + "path/filepath" + "testing" + "time" + + agentfleet "github.com/hoaitan/agentfleet" + "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. +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]*agentfleet.Runner{}, + } +} + +// --- 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") +} From e91b792d69900a65e11bdc25d4764c848b06a821 Mon Sep 17 00:00:00 2001 From: nwebbot Date: Fri, 17 Jul 2026 12:13:46 +1000 Subject: [PATCH 07/10] feat(sandbox): add sandbox cleanup command and document it Sweeps every session log in the working directory, or one named sandbox. --older-than 0 deletes everything and prompts first, since a separate process cannot know which sessions another process has live; --yes skips the prompt and --dry-run reports without deleting. Files failing the log schema check are skipped, so package.json and friends are never touched. help-llm gains the cleanup entry and --retention on connect; the manifest sync test in cmd/retask enforces both. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cmd/helpcmd/command.go | 3 +- internal/cmd/sandbox/cleanup.go | 162 ++++++++++++++++++++++ internal/cmd/sandbox/cleanup_test.go | 199 +++++++++++++++++++++++++++ internal/cmd/sandbox/command.go | 1 + 4 files changed, 364 insertions(+), 1 deletion(-) create mode 100644 internal/cmd/sandbox/cleanup.go create mode 100644 internal/cmd/sandbox/cleanup_test.go diff --git a/internal/cmd/helpcmd/command.go b/internal/cmd/helpcmd/command.go index 21cf996..edb5430 100644 --- a/internal/cmd/helpcmd/command.go +++ b/internal/cmd/helpcmd/command.go @@ -160,8 +160,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)", Flags: []string{"--mode", "--auto-open", "--no-auto-respond"}, Example: "retask sandbox connect "}, + {Command: "retask sandbox connect", Description: "Connect this machine as a Private VM sandbox (long-running). Session folders are created in the current directory and recorded in .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"}, 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 .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..f9035eb --- /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 .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 0be975b..9ab49c1 100644 --- a/internal/cmd/sandbox/command.go +++ b/internal/cmd/sandbox/command.go @@ -33,6 +33,7 @@ func NewCommand(gf *flags.Global) *cobra.Command { newSessionCommand(gf), newConnectCommand(gf), newAttachCommand(gf), + newCleanupCommand(gf), ) return cmd } From ae7bb3cbc0f7de319c7da1a6c2ca27512afaa842 Mon Sep 17 00:00:00 2001 From: nwebbot Date: Fri, 17 Jul 2026 12:37:00 +1000 Subject: [PATCH 08/10] chore(deps): bump agentfleet to v0.7.0 for scaled elapsed time Picks up the session panel timer that scales precision to duration: mm:ss under an hour, h:mm:ss under a day, and 2d3h beyond that. Co-Authored-By: Claude Opus 4.8 (1M context) --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 1e2c3e1..d3895e5 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( connectrpc.com/connect v1.20.0 github.com/charmbracelet/lipgloss v1.1.0 github.com/coder/websocket v1.8.14 - github.com/hoaitan/agentfleet v0.6.27 + github.com/hoaitan/agentfleet v0.7.0 github.com/spf13/cobra v1.10.2 github.com/stretchr/testify v1.11.1 golang.org/x/term v0.43.0 diff --git a/go.sum b/go.sum index d686cc2..7d3bd1f 100644 --- a/go.sum +++ b/go.sum @@ -28,8 +28,8 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/hinshun/vt10x v0.0.0-20220301184237-5011da428d02 h1:AgcIVYPa6XJnU3phs104wLj8l5GEththEw6+F79YsIY= github.com/hinshun/vt10x v0.0.0-20220301184237-5011da428d02/go.mod h1:Q48J4R4DvxnHolD5P8pOtXigYlRuPLGl6moFx3ulM68= -github.com/hoaitan/agentfleet v0.6.27 h1:mAsBWhaZnwC6eUhxtZCgV4GXfcGgASUStHqRgBU3Gmk= -github.com/hoaitan/agentfleet v0.6.27/go.mod h1:nYrGTu9Xzn4fwzlYA9Na8g2PuBKBgYov5uy0mj1J1uM= +github.com/hoaitan/agentfleet v0.7.0 h1:0Tn+koLPxV5Q+0npUfpgbCJn1uvAYhFf86DQXvPyrIs= +github.com/hoaitan/agentfleet v0.7.0/go.mod h1:nYrGTu9Xzn4fwzlYA9Na8g2PuBKBgYov5uy0mj1J1uM= github.com/inconshreveable/go-update v0.0.0-20160112193335-8152e7eb6ccf h1:WfD7VjIE6z8dIvMsI4/s+1qr5EL+zoIGev1BQj1eoJ8= github.com/inconshreveable/go-update v0.0.0-20160112193335-8152e7eb6ccf/go.mod h1:hyb9oH7vZsitZCiBt0ZvifOrB+qc8PS5IiilCIb87rg= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= From 3073f6591d8525c17b413860deb4733cb6b1e0d6 Mon Sep 17 00:00:00 2001 From: nwebbot Date: Sat, 18 Jul 2026 12:08:38 +1000 Subject: [PATCH 09/10] docs: drop superpowers spec and plan from PR Remove the design spec and implementation plan under docs/superpowers/; they were working artifacts and don't belong in the repo. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-07-17-sandbox-session-retention.md | 2357 ----------------- ...-07-17-sandbox-session-retention-design.md | 388 --- 2 files changed, 2745 deletions(-) delete mode 100644 docs/superpowers/plans/2026-07-17-sandbox-session-retention.md delete mode 100644 docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md diff --git a/docs/superpowers/plans/2026-07-17-sandbox-session-retention.md b/docs/superpowers/plans/2026-07-17-sandbox-session-retention.md deleted file mode 100644 index 38fc472..0000000 --- a/docs/superpowers/plans/2026-07-17-sandbox-session-retention.md +++ /dev/null @@ -1,2357 +0,0 @@ -# Sandbox Session Retention & Cleanup Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Session working folders survive being stopped, are deleted immediately on explicit delete, and are reaped by age otherwise — plus the TUI shows `h:mm:ss` for sessions past an hour. - -**Architecture:** `retask sandbox connect` records each session's start time to `.json` in its working directory. That log is the only source of truth for what may be deleted. Deletion has two triggers: explicit (`delete_session`, `delete_sandbox`) reclaims disk immediately after draining the PTY, and age (an hourly sweeper, or `retask sandbox cleanup`) reaps folders left behind by stop/disconnect. Stopping — a session, a sandbox, or the CLI itself — never deletes anything. - -**Tech Stack:** Go 1.26.4, cobra, testify (`assert`/`require`), `github.com/hoaitan/agentfleet` (TUI + PTY runners), lipgloss. - -**Spec:** `docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md` - -## Global Constraints - -- Two repos, strictly sequenced: **agentfleet first** (Task 1), then retask-cli (Tasks 2–11). Task 11 bumps the dep and requires a released agentfleet tag, which the user cuts manually. -- `gh` is authenticated as `nwebbot` with **READ only** on `hoaitan/agentfleet` — the agentfleet PR must come from a fork. -- Retention default is `30d`; disabled with `off` only. `--retention 0` is an error. -- `--older-than 0` means "everything". `off` is not valid for `--older-than`. -- Sweep interval is exactly `1 * time.Hour`. Drain timeout is exactly `5 * time.Second`. -- Log file lives at `/.json` where `baseDir` is `os.Getwd()` from `connect.go`. Schema version is `1`. -- **Log-only policy:** never delete a `session-*` folder that has no log entry. No mtime fallback, no adoption scan. -- **Stop never deletes.** `Stop`, `StopAll`, and the CLI-stop path must contain no disk access. Deletion lives only in the `delete_session` / `delete_sandbox` branches. -- Named return parameters are required for multi-value returns (repo convention, `CLAUDE.md`). -- Every command's `Long` follows the repo help template: one-line summary, `Usage example:`, `Flags:`. -- Never edit `proto-gen/` by hand. - ---- - -## File Structure - -**agentfleet (fork):** -- Modify: `tui/tui.go` — extract `formatElapsed`, widen the elapsed column. -- Create: `tui/tui_test.go` — table test for `formatElapsed`. - -**retask-cli:** -- Create: `internal/cmd/sandbox/retention.go` — duration parsing (`parseDuration`, `parseRetention`) and the `retentionSweeper`. -- Create: `internal/cmd/sandbox/retention_test.go` -- Create: `internal/cmd/sandbox/sessionlog.go` — the `sessionLog` store: load/record/remove/sweep/destroy. -- Create: `internal/cmd/sandbox/sessionlog_test.go` -- Create: `internal/cmd/sandbox/cleanup.go` — the `sandbox cleanup` command + log discovery. -- Create: `internal/cmd/sandbox/cleanup_test.go` -- Modify: `internal/cmd/sandbox/sessionlane.go` — `drain`, record-on-start, `Remove` teardown, `RemoveAll`. -- Create: `internal/cmd/sandbox/sessionlane_test.go` -- Modify: `internal/cmd/sandbox/datalane.go:162` — `delete_sandbox` calls `RemoveAll`. -- Modify: `internal/cmd/sandbox/connect.go` — `--retention` flag, log store, sweeper, CLI exit on lane return. -- Modify: `internal/cmd/sandbox/command.go:26` — register `newCleanupCommand`. -- Modify: `internal/cmd/helpcmd/command.go:163` — manifest entries. - -Splitting `sessionlog.go` (persistence) from `retention.go` (policy + scheduling) from `cleanup.go` (CLI surface) keeps each file single-purpose; `sessionlane.go` is already 256 lines and only gains teardown logic. - ---- - -## Task 1: agentfleet — `h:mm:ss` elapsed time - -**Repo:** `hoaitan/agentfleet` (fork required — `nwebbot` has READ only) - -**Files:** -- Modify: `tui/tui.go:550-566` (inside `renderCard`) -- Test: `tui/tui_test.go` (create) - -**Interfaces:** -- Consumes: nothing. -- Produces: `formatElapsed(d time.Duration) string` (package-private to `tui`). Released as a new agentfleet tag consumed by Task 11. - -**Context:** `renderCard` currently renders minutes:seconds, so a 90-minute session reads `90:30`. `styleMeta.Width(5)` sizes the column; `1:15:30` needs 7. `renderCard` derives `nameMaxW` from `lipgloss.Width(rightStr)`, so the name column reflows on its own. - -- [ ] **Step 1: Fork and clone** - -```bash -cd /tmp -gh repo fork hoaitan/agentfleet --clone --remote-name upstream --fork-name agentfleet -cd agentfleet -git checkout -b feat/elapsed-hours -``` - -Expected: a fork under `nwebbot/agentfleet`, cloned, with `upstream` pointing at `hoaitan/agentfleet`. - -- [ ] **Step 2: Write the failing test** - -Create `tui/tui_test.go`: - -```go -package tui - -import ( - "testing" - "time" -) - -func TestFormatElapsed(t *testing.T) { - tests := []struct { - name string - in time.Duration - want string - }{ - {"zero", 0, "00:00"}, - {"seconds", 5 * time.Second, "00:05"}, - {"sub-minute rounds down", 5*time.Second + 400*time.Millisecond, "00:05"}, - {"minutes", 90 * time.Second, "01:30"}, - {"just under an hour", 59*time.Minute + 59*time.Second, "59:59"}, - {"exactly one hour", time.Hour, "1:00:00"}, - {"hours minutes seconds", time.Hour + 15*time.Minute + 30*time.Second, "1:15:30"}, - {"multi-day", 25*time.Hour + time.Minute + 2*time.Second, "25:01:02"}, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - if got := formatElapsed(tc.in); got != tc.want { - t.Errorf("formatElapsed(%v) = %q, want %q", tc.in, got, tc.want) - } - }) - } -} -``` - -Note: agentfleet's existing tests (`tui/chrome_test.go`) use the stdlib `testing` package, not testify. Match that. - -- [ ] **Step 3: Run test to verify it fails** - -Run: `go test ./tui/ -run TestFormatElapsed -v` -Expected: FAIL — `undefined: formatElapsed` - -- [ ] **Step 4: Add `formatElapsed`** - -In `tui/tui.go`, add above `renderCard`: - -```go -// formatElapsed renders a running task's elapsed time. Past one hour it gains -// an hours component, so a long session reads 1:30:00 rather than 90:00. -func formatElapsed(d time.Duration) string { - d = d.Round(time.Second) - if h := int(d.Hours()); h > 0 { - return fmt.Sprintf("%d:%02d:%02d", h, int(d.Minutes())%60, int(d.Seconds())%60) - } - return fmt.Sprintf("%02d:%02d", int(d.Minutes()), int(d.Seconds())%60) -} -``` - -- [ ] **Step 5: Run test to verify it passes** - -Run: `go test ./tui/ -run TestFormatElapsed -v` -Expected: PASS (all 8 subtests) - -- [ ] **Step 6: Call it from `renderCard` and widen the column** - -In `tui/tui.go:550-566`, replace: - -```go - elapsed := "" - if r.Status() == agentfleet.StatusRunning { - d := time.Since(r.StartedAt()).Round(time.Second) - elapsed = fmt.Sprintf("%02d:%02d", int(d.Minutes()), int(d.Seconds())%60) - } -``` - -with: - -```go - elapsed := "" - if r.Status() == agentfleet.StatusRunning { - elapsed = formatElapsed(time.Since(r.StartedAt())) - } -``` - -and replace: - -```go - elapsedStr := styleMeta.Width(5).Render(elapsed) -``` - -with: - -```go - // Width fits "25:01:02"; sub-hour values stay right-sized by padding. - elapsedStr := styleMeta.Width(8).Render(elapsed) -``` - -- [ ] **Step 7: Verify the whole package still builds and passes** - -Run: `go build ./... && go test ./...` -Expected: PASS, no build errors. - -- [ ] **Step 8: Commit and open the PR** - -```bash -git add tui/tui.go tui/tui_test.go -git commit -m "feat(tui): show hours in elapsed time past one hour - -A 90-minute session rendered as 90:30, which reads as a minute count -rather than an hour and a half. Past 1h the timer now renders h:mm:ss. - -The elapsed column widens from 5 to 8 to fit 25:01:02; sub-hour -rendering is unchanged. renderCard derives nameMaxW from the rendered -width, so the name column reflows automatically." -git push -u origin feat/elapsed-hours -gh pr create --repo hoaitan/agentfleet \ - --title "feat(tui): show hours in elapsed time past one hour" \ - --body "Past one hour the task card timer renders \`h:mm:ss\` (\`1:15:30\`) instead of a raw minute count (\`75:30\`). - -- Extracts \`formatElapsed\` as a pure function with a table test. -- Widens the elapsed column 5 → 8 to fit \`25:01:02\`. \`renderCard\` derives \`nameMaxW\` from \`lipgloss.Width(rightStr)\`, so the name column reflows automatically. -- Sub-hour rendering is unchanged. - -Needed by retask-cli, which shows long-lived sandbox sessions and cannot override the format (\`TUIConfig\` exposes no formatting hook)." -``` - -**Known limitation (do not fix):** a session running 100+ hours renders 9 characters into a width-8 column, which lipgloss wraps. That requires a continuously-running 4+ day session; accepted rather than complicating the layout. - -- [ ] **Step 9: Hand off for release** - -Report the PR URL. **STOP** — the user merges and tags the release manually. Tasks 2–10 do not depend on it; only Task 11 does. - ---- - -## Task 2: Duration parsing - -**Files:** -- Create: `internal/cmd/sandbox/retention.go` -- Test: `internal/cmd/sandbox/retention_test.go` - -**Interfaces:** -- Consumes: nothing. -- Produces: - - `parseDuration(s string) (d time.Duration, err error)` — accepts Go duration syntax plus a `d` (days) suffix. Used by `--older-than`. - - `parseRetention(s string) (d time.Duration, enabled bool, err error)` — as above, plus `off`. Used by `--retention`. - -**Context:** `time.ParseDuration` has no day unit, so `30d` fails. Both flags share the grammar but not the keywords: `off` is retention-only, and `0` is meaningful only for `--older-than`. - -- [ ] **Step 1: Write the failing test** - -Create `internal/cmd/sandbox/retention_test.go`: - -```go -package sandbox - -import ( - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -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 - // make an hourly sweep wipe every folder. Disabling is spelled "off". - _, _, err := parseRetention("0") - assert.Error(t, err) - _, _, err = parseRetention("0d") - assert.Error(t, err) -} -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestParse' -v` -Expected: FAIL — `undefined: parseDuration` - -- [ ] **Step 3: Implement** - -Create `internal/cmd/sandbox/retention.go`: - -```go -package sandbox - -import ( - "fmt" - "strconv" - "strings" - "time" -) - -// 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 -} -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestParse' -v` -Expected: PASS - -- [ ] **Step 5: Commit** - -```bash -git add internal/cmd/sandbox/retention.go internal/cmd/sandbox/retention_test.go -git commit -m "feat(sandbox): parse retention durations with a day suffix - -time.ParseDuration has no day unit, so 30d needs handling. --retention -additionally accepts off; 0 is rejected there because it means delete -everything for --older-than." -``` - ---- - -## Task 3: Session log store - -**Files:** -- Create: `internal/cmd/sandbox/sessionlog.go` -- Test: `internal/cmd/sandbox/sessionlog_test.go` - -**Interfaces:** -- Consumes: nothing. -- Produces: - - `type sessionEntry struct { Name, Dir string; CreatedAt time.Time }` - - `newSessionLog(baseDir, sandboxID string) *sessionLog` - - `sessionLogPath(baseDir, sandboxID string) string` - - `(*sessionLog) record(sessionID, name, dir string, createdAt time.Time) error` - - `(*sessionLog) remove(sessionID string) error` - - `(*sessionLog) entries() (map[string]sessionEntry, error)` - - `(*sessionLog) destroy() error` - - `loadSessionLogFile(path string) (*sessionLogData, error)` - - `errNewerLog` sentinel - -**Context:** Concurrent `new_session` events and the hourly sweep both mutate the file, so every mutation takes a mutex and rewrites atomically (temp + rename). Read-modify-write per mutation avoids holding stale in-memory state. `destroy()` latches `closed` so a sweep after `delete_sandbox` cannot recreate the file. - -- [ ] **Step 1: Write the failing test** - -Create `internal/cmd/sandbox/sessionlog_test.go`: - -```go -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, "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 cwd can hold ordinary JSON. It must never be read as a session 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, "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, "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, "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()) -} -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestSessionLog' -v` -Expected: FAIL — `undefined: newSessionLog` - -- [ ] **Step 3: Implement** - -Create `internal/cmd/sandbox/sessionlog.go`: - -```go -package sandbox - -import ( - "encoding/json" - "errors" - "fmt" - "os" - "path/filepath" - "sync" - "time" -) - -// sessionLogVersion is the schema version written to .json. -const sessionLogVersion = 1 - -// errNewerLog reports a log written by a newer CLI. We skip such files rather -// than rewriting them, so an older binary cannot truncate fields it lost. -var errNewerLog = errors.New("session log written by a newer CLI version") - -// sessionEntry is one recorded session. -type sessionEntry struct { - Name string `json:"name"` - Dir string `json:"dir"` - CreatedAt time.Time `json:"created_at"` -} - -// sessionLogData is the on-disk shape of .json. -type sessionLogData struct { - Version int `json:"version"` - SandboxID string `json:"sandbox_id"` - Sessions map[string]sessionEntry `json:"sessions"` -} - -// sessionLog owns /.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, then 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, 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 from 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]sessionEntry{}, - } - } - 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 so the rename stays 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] = sessionEntry{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]sessionEntry, err error) { - l.mu.Lock() - defer l.mu.Unlock() - d, err := l.load() - if err != nil { - return nil, err - } - out = make(map[string]sessionEntry, len(d.Sessions)) - for k, v := range d.Sessions { - out[k] = v - } - return out, nil -} - -// 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 -} -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestSessionLog' -v` -Expected: PASS (10 tests) - -- [ ] **Step 5: Commit** - -```bash -git add internal/cmd/sandbox/sessionlog.go internal/cmd/sandbox/sessionlog_test.go -git commit -m "feat(sandbox): add per-sandbox session start-time log - -Records session start times to .json next to the session -folders. The log is the only source of truth for what may be deleted. - -Mutations are read-modify-write under a mutex plus an atomic rename, -since session starts and the retention sweep both write it. Files that -aren't ours (package.json) and logs from newer CLIs are left alone." -``` - ---- - -## Task 4: Sweep by age - -**Files:** -- Modify: `internal/cmd/sandbox/sessionlog.go` (add `sweep`) -- Test: `internal/cmd/sandbox/sessionlog_test.go` (append) - -**Interfaces:** -- Consumes: `sessionLog`, `sessionEntry` (Task 3). -- Produces: `(*sessionLog) sweep(baseDir string, now time.Time, olderThan time.Duration, skip func(string) bool, dryRun bool) (deleted []string, err error)` — deletes folders whose entry is at least `olderThan` old, returns sorted session ids. `olderThan == 0` matches everything. `skip` may be nil. - -**Context:** One sweep function serves both the hourly goroutine (which passes `sm.isActive` so a long-running session never has its cwd deleted) and the `cleanup` command (which passes nil). `now` is injected so tests never sleep. - -- [ ] **Step 1: Write the failing test** - -Append to `internal/cmd/sandbox/sessionlog_test.go`: - -```go -// 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, "sb-1.json")) - assert.True(t, os.IsNotExist(err)) -} -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestSweep' -v` -Expected: FAIL — `l.sweep undefined` - -- [ ] **Step 3: Implement** - -Add to `internal/cmd/sandbox/sessionlog.go` (and add `"sort"` to the imports): - -```go -// 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 -} -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestSweep' -v` -Expected: PASS (6 tests) - -- [ ] **Step 5: Commit** - -```bash -git add internal/cmd/sandbox/sessionlog.go internal/cmd/sandbox/sessionlog_test.go -git commit -m "feat(sandbox): sweep logged session folders by age - -One sweep serves both the hourly goroutine and the cleanup command. The -skip predicate keeps the sweeper from deleting a live session's own -working directory when it outlives the retention window. Folders with no -log entry are never touched." -``` - ---- - -## Task 5: Record sessions on start - -**Files:** -- Modify: `internal/cmd/sandbox/sessionlane.go:20-61` (SessionManager fields + constructor), `:65-108` (Start) -- Modify: `internal/cmd/sandbox/connect.go:162-171` (construct the log, pass it in) -- Test: `internal/cmd/sandbox/sessionlane_test.go` (create) - -**Interfaces:** -- Consumes: `newSessionLog`, `(*sessionLog) record` (Task 3). -- Produces: - - `SessionManager.log *sessionLog` field. - - `newSessionManager(..., log *sessionLog, autoRespond bool) *SessionManager` — `log` is added as the **second-to-last** parameter, immediately before `autoRespond`. - - `(*SessionManager) isActive(sessionID string) bool` — used by the sweeper in Task 8. - -**Context:** The entry is recorded **before** `sb.Run` because `SessionBootstrap.setupFolder` creates the folder early (`sessionBootstrap.go:238`) but `Run` can fail later at git clone or config write. Under the log-only policy a folder with no entry can never be cleaned, so recording after success would leak every failed bootstrap permanently. - -**Naming note:** `SessionManager` already has a field named `log` (the -`*slog.Logger`). The new field is therefore named `sessionLog`, and every task -below uses `sm.sessionLog`. - -- [ ] **Step 1: Write the failing test** - -Create `internal/cmd/sandbox/sessionlane_test.go`: - -```go -package sandbox - -import ( - "testing" - "time" - - agentfleet "github.com/hoaitan/agentfleet" - "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. -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]*agentfleet.Runner{}, - } -} - -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") -} -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestRecordSessionStart|TestIsActive' -v` -Expected: FAIL — `sm.recordSessionStart undefined`, `unknown field sessionLog` - -- [ ] **Step 3: Add the field, constructor param, and helpers** - -In `internal/cmd/sandbox/sessionlane.go`, add to the `SessionManager` struct immediately after `endpoint string`: - -```go - sessionLog *sessionLog // records session start times for retention -``` - -Update the constructor signature (add `sessionLog` immediately before `autoRespond`): - -```go -func newSessionManager( - sandboxID, wsBase string, - fleet *agentfleet.Fleet, - fleetCfg agentfleet.FleetConfig, - agentCfg agentfleet.AgentConfig, - log *slog.Logger, - workspaceID, sandboxName, baseDir, endpoint string, - sessionLog *sessionLog, - autoRespond bool, -) *SessionManager { -``` - -and inside the returned struct literal add: - -```go - sessionLog: sessionLog, -``` - -Add these methods: - -```go -// 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 -} -``` - -Add `"time"` to the imports. - -- [ ] **Step 4: Call it from Start, before bootstrap** - -In `sessionlane.go`, in `Start`, immediately before the `sb := &SessionBootstrap{...}` literal (currently line 90), insert: - -```go - // 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()) -``` - -- [ ] **Step 5: Update the caller in connect.go** - -In `internal/cmd/sandbox/connect.go`, after `baseDir, err := os.Getwd()` (line 157-160) add: - -```go - sessLog := newSessionLog(baseDir, sandboxID) -``` - -and pass it in the `newSessionManager` call, before `autoRespond`: - -```go - sm := newSessionManager( - sandboxID, wsBase, - fleet, fleetCfg.Fleet, fleetCfg.Agent, - logger, - sbResp.Msg.WorkspaceId, - sbResp.Msg.Name, - baseDir, - profile.Endpoint, - sessLog, - autoRespond, - ) -``` - -- [ ] **Step 6: Run tests and build** - -Run: `go build ./... && go test ./internal/cmd/sandbox/ -v` -Expected: PASS - -- [ ] **Step 7: Commit** - -```bash -git add internal/cmd/sandbox/sessionlane.go internal/cmd/sandbox/sessionlane_test.go internal/cmd/sandbox/connect.go -git commit -m "feat(sandbox): record session start times on session start - -The entry is written before bootstrap, not after: setupFolder creates -the folder early but bootstrap can fail later, and under the log-only -retention policy an unlogged folder can never be reaped." -``` - ---- - -## Task 6: Drain the PTY before deleting, and delete on `delete_session` - -**Files:** -- Modify: `internal/cmd/sandbox/sessionlane.go:203-214` (`Remove`) -- Test: `internal/cmd/sandbox/sessionlane_test.go` (append) - -**Interfaces:** -- Consumes: `(*sessionLog) remove` (Task 3), `isActive` (Task 5). -- Produces: - - `const sessionDrainTimeout = 5 * time.Second` - - `type stoppableRunner interface { Stop() error; Done() <-chan struct{} }` - - `drain(r stoppableRunner, timeout time.Duration)` - -**Context:** `PtyAgent.Stop` sends SIGTERM and returns immediately — it does not wait for the process. Today's `Remove` deletes the folder right after, racing the agent's own cleanup. SIGTERM exists precisely to grant that grace period. agentfleet never escalates to SIGKILL for a process that *ignores* SIGTERM (its `Kill()` fires only when signal *delivery* fails), so the timeout is the only backstop. - -- [ ] **Step 1: Write the failing test** - -Append to `internal/cmd/sandbox/sessionlane_test.go`: - -```go -// 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) // 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) }) -} - -func TestRemoveDeletesFolderAndLogEntry(t *testing.T) { - dir := t.TempDir() - sm := newTestSessionManager(t, dir) - now := time.Now().UTC() - sessDir := mkSession(t, sm.sessionLog, dir, "sess-a", now) - - 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") -} - -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") -} -``` - -Add `"os"` to the test imports. - -- [ ] **Step 2: Run test to verify it fails** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestDrain|TestRemove|TestStopDoesNot' -v` -Expected: FAIL — `undefined: drain` - -- [ ] **Step 3: Implement drain and rewrite Remove** - -In `internal/cmd/sandbox/sessionlane.go`, add near the top (after the imports): - -```go -// 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): - } -} -``` - -Replace `Remove` (currently lines 203-214) with: - -```go -// 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() - r := sm.sessions[sessionID] - delete(sm.sessions, sessionID) - sm.mu.Unlock() - - if r != nil { - drain(r, sessionDrainTimeout) - sm.fleet.Remove(sessionID) - } - 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) - } - } -} -``` - -Note `sm.fleet` is nil in tests, so `Remove` must only touch it when `r != nil` — which the test relies on (it registers no runner). - -- [ ] **Step 4: Run test to verify it passes** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestDrain|TestRemove|TestStopDoesNot' -v` -Expected: PASS (5 tests) - -- [ ] **Step 5: Full package test + build** - -Run: `go build ./... && go test ./internal/cmd/sandbox/` -Expected: PASS - -- [ ] **Step 6: Commit** - -```bash -git add internal/cmd/sandbox/sessionlane.go internal/cmd/sandbox/sessionlane_test.go -git commit -m "fix(sandbox): drain the PTY before deleting a session folder - -PtyAgent.Stop returns when SIGTERM is delivered, not when the process -exits, so deleting straight after it destroyed the working directory -while the agent was still flushing into it. delete_session now waits for -Runner.Done() (5s cap) before deleting, and drops the log entry too. - -agentfleet never escalates to SIGKILL for a process that ignores -SIGTERM, so the timeout is the only backstop. - -Adds a regression test that Stop/StopAll never delete." -``` - ---- - -## Task 7: `delete_sandbox` full teardown - -**Files:** -- Modify: `internal/cmd/sandbox/sessionlane.go` (add `RemoveAll`) -- Modify: `internal/cmd/sandbox/datalane.go:162-167` -- Modify: `internal/cmd/sandbox/connect.go:174` -- Test: `internal/cmd/sandbox/sessionlane_test.go` (append) - -**Interfaces:** -- Consumes: `drain`, `sessionDrainTimeout` (Task 6), `(*sessionLog) entries`/`destroy` (Task 3). -- Produces: `(*SessionManager) RemoveAll()` — stop, drain concurrently, delete every logged folder, delete the log file, latch the store closed. - -**Context:** `dl.Run(ctx)` is a goroutine (`connect.go:174`), so returning `errSandboxDeleted` currently signals nothing — `ctx` is never cancelled and the TUI keeps running against a sandbox that no longer exists. Cancelling on return reuses the exact path Ctrl-C takes. Deleting the log file while the sweeper is alive would let a later tick recreate it, which `destroy()`'s latch prevents. - -- [ ] **Step 1: Write the failing test** - -Append to `internal/cmd/sandbox/sessionlane_test.go`: - -```go -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") -} -``` - -Add `"path/filepath"` to the test imports. - -- [ ] **Step 2: Run test to verify it fails** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestRemoveAll' -v` -Expected: FAIL — `sm.RemoveAll undefined` - -- [ ] **Step 3: Implement RemoveAll** - -Add to `internal/cmd/sandbox/sessionlane.go` after `Remove`: - -```go -// 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() - runners := make(map[string]*agentfleet.Runner, len(sm.sessions)) - for id, r := range sm.sessions { - runners[id] = r - } - sm.sessions = make(map[string]*agentfleet.Runner) - sm.mu.Unlock() - - var wg sync.WaitGroup - for id, r := range runners { - wg.Add(1) - go func() { - defer wg.Done() - drain(r, 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. - entries, err := sm.sessionLog.entries() - if err != nil { - sm.logError("session_log_read_failed", "sandbox_id", sm.sandboxID, "error", err) - } - for id, e := range entries { - 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) - } -} -``` - -`sync` is already imported by this file. - -- [ ] **Step 4: Run test to verify it passes** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestRemoveAll' -v` -Expected: PASS (3 tests) - -- [ ] **Step 5: Wire delete_sandbox to RemoveAll** - -In `internal/cmd/sandbox/datalane.go`, replace the `delete_sandbox` case (lines 162-167): - -```go - case "delete_sandbox": - dl.logInfo("delete_sandbox", "sandbox_id", msg.SandboxID) - dl.sessions.RemoveAll() - conn.Close(websocket.StatusNormalClosure, "deleted") //nolint:errcheck - return errSandboxDeleted -``` - -- [ ] **Step 6: Exit the CLI when the data lane ends** - -In `internal/cmd/sandbox/connect.go`, replace line 174: - -```go - go dl.Run(ctx) -``` - -with: - -```go - // 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() - }() -``` - -- [ ] **Step 7: Build and run the full suite** - -Run: `go build ./... && go test ./...` -Expected: PASS - -- [ ] **Step 8: Commit** - -```bash -git add internal/cmd/sandbox/sessionlane.go internal/cmd/sandbox/sessionlane_test.go internal/cmd/sandbox/datalane.go internal/cmd/sandbox/connect.go -git commit -m "feat(sandbox): tear down fully on delete_sandbox - -delete_sandbox now stops and drains every session, deletes every logged -folder and the log file, then exits the CLI — previously the data lane -goroutine just returned, leaving a TUI attached to a sandbox that no -longer existed. - -Closing the log store matters: the retention sweeper can outlive the -file deletion and would otherwise recreate a log for a dead sandbox." -``` - ---- - -## Task 8: Retention sweeper and `--retention` - -**Files:** -- Modify: `internal/cmd/sandbox/retention.go` (add `retentionSweeper`) -- Modify: `internal/cmd/sandbox/connect.go` (flag + goroutine + help text) -- Test: `internal/cmd/sandbox/retention_test.go` (append) - -**Interfaces:** -- Consumes: `(*sessionLog) sweep` (Task 4), `(*SessionManager) isActive` (Task 5), `parseRetention` (Task 2). -- Produces: - - `const retentionSweepInterval = time.Hour` - - `type retentionSweeper struct { log *sessionLog; baseDir string; window, interval time.Duration; isActive func(string) bool; logger *slog.Logger }` - - `(*retentionSweeper) Run(ctx context.Context)` — sweeps once immediately, then every `interval` until ctx is cancelled. - - `(*retentionSweeper) once()` - -**Context:** Retention is the backstop for folders left by stop/disconnect — explicit deletes reclaim their own disk. Sweeping once at startup means a machine reconnecting after a month cleans up without waiting an hour. - -- [ ] **Step 1: Write the failing test** - -Append to `internal/cmd/sandbox/retention_test.go`: - -```go -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") - } -} -``` - -Add `"context"`, `"os"`, `"path/filepath"` to the test imports. - -- [ ] **Step 2: Run test to verify it fails** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestSweeper' -v` -Expected: FAIL — `undefined: retentionSweeper` - -- [ ] **Step 3: Implement the sweeper** - -Append to `internal/cmd/sandbox/retention.go` (adding `"context"` and `"log/slog"` to the imports): - -```go -// 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()) - } -} -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestSweeper' -v` -Expected: PASS - -- [ ] **Step 5: Add the flag and start the sweeper** - -In `internal/cmd/sandbox/connect.go`, add to the var block at the top of `newConnectCommand`: - -```go - var retention string -``` - -Register the flag next to the others (after the `--no-auto-respond` registration): - -```go - cmd.Flags().StringVar(&retention, "retention", "30d", `Delete session folders older than this (e.g. 30d, 12h); "off" disables`) -``` - -In `RunE`, validate early — next to the existing `--mode` validation (line 77-79): - -```go - retentionWindow, retentionOn, err := parseRetention(retention) - if err != nil { - return err - } -``` - -Note: `err` is already declared later in `RunE` via `cfg, err := config.Load(path)`; because this new statement comes first and uses `:=` with two new variables, it compiles. Verify with the build in Step 7. - -After the `sm := newSessionManager(...)` block and before `dl := newDataLane(...)`, add: - -```go - if retentionOn { - sweeper := &retentionSweeper{ - log: sessLog, - baseDir: baseDir, - window: retentionWindow, - interval: retentionSweepInterval, - isActive: sm.isActive, - logger: logger, - } - go sweeper.Run(ctx) - } -``` - -Update the command's `Long` to document the flag (repo help template). Replace the `Flags:` block: - -``` -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) -``` - -and add to the usage examples: - -``` - retask sandbox connect sandbox_abc123 --retention 7d - retask sandbox connect sandbox_abc123 --retention off -``` - -- [ ] **Step 6: Verify the flag rejects bad input** - -Append to `internal/cmd/sandbox/retention_test.go`: - -```go -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") -} -``` - -Add `"github.com/nwebxyz/retask-cli/internal/flags"` to the test imports. - -- [ ] **Step 7: Build and test** - -Run: `go build ./... && go test ./internal/cmd/sandbox/` -Expected: PASS - -- [ ] **Step 8: Commit** - -```bash -git add internal/cmd/sandbox/retention.go internal/cmd/sandbox/retention_test.go internal/cmd/sandbox/connect.go -git commit -m "feat(sandbox): sweep aged session folders hourly on connect - ---retention 30d (default) deletes logged session folders older than the -window, checked at startup and then hourly; --retention off disables it. - -Live sessions are skipped, so a session outliving the window never has -its own working directory deleted underneath it." -``` - ---- - -## Task 9: `retask sandbox cleanup` - -**Files:** -- Create: `internal/cmd/sandbox/cleanup.go` -- Test: `internal/cmd/sandbox/cleanup_test.go` -- Modify: `internal/cmd/sandbox/command.go:26-35` (register the command) - -**Interfaces:** -- Consumes: `parseDuration` (Task 2), `newSessionLog`, `loadSessionLogFile`, `errNewerLog` (Task 3), `(*sessionLog) sweep` (Task 4). -- Produces: - - `newCleanupCommand(gf *flags.Global) *cobra.Command` - - `discoverSessionLogs(baseDir string) (logs []*sessionLog, err error)` - - `confirm(in io.Reader, out io.Writer, prompt string) bool` - -**Context:** Bare `cleanup` sweeps every valid log in cwd; an id narrows it. Files failing the schema check are skipped, which is what protects `package.json`. `--older-than 0` prompts, because a separate process cannot know which sessions another process has live. - -- [ ] **Step 1: Write the failing test** - -Create `internal/cmd/sandbox/cleanup_test.go`: - -```go -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 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") -} - -// --- 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() -} -``` - -Note: these tests `os.Chdir`, so they must not run in parallel — do not add `t.Parallel()`. - -- [ ] **Step 2: Run test to verify it fails** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestCleanup|TestDiscover|TestConfirm' -v` -Expected: FAIL — `undefined: newCleanupCommand` - -- [ ] **Step 3: Implement** - -Create `internal/cmd/sandbox/cleanup.go`: - -```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 .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 that -// fails 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" -} -``` - -- [ ] **Step 4: Register the command** - -In `internal/cmd/sandbox/command.go`, add to the `AddCommand` block (after `newAttachCommand(gf)`): - -```go - newCleanupCommand(gf), -``` - -- [ ] **Step 5: Run test to verify it passes** - -Run: `go test ./internal/cmd/sandbox/ -run 'TestCleanup|TestDiscover|TestConfirm' -v` -Expected: PASS (10 tests) - -- [ ] **Step 6: Try it by hand** - -```bash -go build -o /tmp/retask ./cmd/retask/ -mkdir -p /tmp/cleanup-demo && cd /tmp/cleanup-demo -mkdir -p session-demo -printf '{\n "version": 1,\n "sandbox_id": "sb-demo",\n "sessions": {\n "demo": {"name":"demo","dir":"session-demo","created_at":"2020-01-01T00:00:00Z"}\n }\n}\n' > sb-demo.json -printf '{"name":"app"}' > package.json -/tmp/retask sandbox cleanup --dry-run -``` - -Expected: reports `sb-demo demo` and `1 session folder(s) would be deleted (--dry-run).`; `session-demo` and `package.json` both still present. - -```bash -/tmp/retask sandbox cleanup -ls -``` - -Expected: `Deleted 1 session folder(s).`; `session-demo` gone, `package.json` untouched, `sb-demo.json` now has an empty `sessions` map. - -- [ ] **Step 7: Commit** - -```bash -git add internal/cmd/sandbox/cleanup.go internal/cmd/sandbox/cleanup_test.go internal/cmd/sandbox/command.go -git commit -m "feat(sandbox): add sandbox cleanup command - -Sweeps every session log in the working directory (or one named -sandbox). --older-than 0 deletes everything and prompts first, since a -separate process cannot know which sessions are live; --yes skips the -prompt and --dry-run reports without deleting. - -Files that fail the log schema check are skipped, so package.json and -friends are never touched." -``` - ---- - -## Task 10: `help-llm` manifest - -**Files:** -- Modify: `internal/cmd/helpcmd/command.go:163` (connect entry) and add a cleanup entry -- Test: `cmd/retask/main_test.go` (existing sync test — no new test needed) - -**Interfaces:** -- Consumes: the command tree from Tasks 8 and 9. -- Produces: nothing consumed by later tasks. - -**Context:** `cmd/retask/main_test.go:83` asserts the hand-maintained manifest matches the real command tree in both directions — an undocumented flag or an undocumented command fails the suite. This task exists to satisfy it. - -- [ ] **Step 1: Run the sync test to see it fail** - -Run: `go test ./cmd/retask/ -run TestHelpLLM -v` -Expected: FAIL — `retask sandbox cleanup` is not documented, and `retask sandbox connect` flags drift (missing `--retention`). - -If the test name differs, find it with: `grep -n "func Test" cmd/retask/main_test.go` - -- [ ] **Step 2: Update the connect entry** - -In `internal/cmd/helpcmd/command.go`, replace line 163: - -```go - {Command: "retask sandbox connect", Description: "Connect this machine as a Private VM sandbox (long-running)", Flags: []string{"--mode", "--auto-open", "--no-auto-respond"}, Example: "retask sandbox connect "}, -``` - -with: - -```go - {Command: "retask sandbox connect", Description: "Connect this machine as a Private VM sandbox (long-running). Session folders are created in the current directory and recorded in .json. --retention deletes folders older than the window (checked hourly); \"off\" disables it. Live sessions are never deleted", Flags: []string{"--mode", "--auto-open", "--no-auto-respond", "--retention"}, Example: "retask sandbox connect --retention 30d"}, -``` - -- [ ] **Step 3: Add the cleanup entry** - -Immediately after the `retask sandbox attach` entry (line 164), add: - -```go - {Command: "retask sandbox cleanup", Description: "Delete session folders left by stopped sessions, in the current directory. Only folders recorded in a .json session log are considered. With no argument every log in the directory is swept; pass a sandbox id to narrow it. --older-than 0 deletes everything and prompts unless --yes", Flags: []string{"--older-than", "--dry-run", "--yes"}, Example: "retask sandbox cleanup --older-than 7d"}, -``` - -- [ ] **Step 4: Run the sync test to verify it passes** - -Run: `go test ./cmd/retask/ -v` -Expected: PASS - -- [ ] **Step 5: Eyeball the manifest** - -```bash -go build -o /tmp/retask ./cmd/retask/ -/tmp/retask help-llm | jq '.commands[] | select(.command | contains("sandbox cleanup") or contains("sandbox connect"))' -``` - -Expected: both entries present, `--retention` on connect, three flags on cleanup. - -- [ ] **Step 6: Run everything** - -Run: `go build ./... && go test ./...` -Expected: PASS - -- [ ] **Step 7: Commit** - -```bash -git add internal/cmd/helpcmd/command.go -git commit -m "docs(help-llm): document sandbox cleanup and --retention" -``` - ---- - -## Task 11: Bump agentfleet and open the retask-cli PR - -**Files:** -- Modify: `go.mod`, `go.sum` - -**Interfaces:** -- Consumes: the agentfleet release from Task 1. -- Produces: the final PR. - -**BLOCKED** until the user merges Task 1's PR and pushes a tag. Confirm the released version before starting. - -- [ ] **Step 1: Bump the dependency** - -```bash -go get github.com/hoaitan/agentfleet@ -go mod tidy -``` - -Replace `` with the tag the user cut (e.g. `v0.6.28`). - -- [ ] **Step 2: Verify the build and suite** - -Run: `go build ./... && go test ./...` -Expected: PASS - -- [ ] **Step 3: Verify the elapsed format is actually live** - -```bash -grep -n "Width(8)" $(go env GOMODCACHE)/github.com/hoaitan/agentfleet@/tui/tui.go -``` - -Expected: the widened column is present — confirming the tag contains Task 1's change rather than an older commit. - -- [ ] **Step 4: Commit** - -```bash -git add go.mod go.sum -git commit -m "chore(deps): bump agentfleet for h:mm:ss elapsed time - -Picks up the TUI change that renders hours in the session panel's -elapsed timer past one hour." -``` - -- [ ] **Step 5: Push and open the PR** - -```bash -git push -u origin feat/sandbox-session-retention -gh pr create --title "feat(sandbox): session folder retention and cleanup" --body "$(cat <<'BODY' -Session working folders now survive being stopped, are deleted immediately on an explicit delete, and are reaped by age otherwise. - -## What changed - -- **Session log.** `sandbox connect` records each session's start time to `.json` beside the session folders. It is the only source of truth for what may be deleted — a `session-*` folder with no entry is never touched. The entry is written *before* bootstrap, since bootstrap creates the folder early but can fail later. -- **Explicit delete reclaims disk now.** `delete_session` deletes the folder and its entry. `delete_sandbox` is a full teardown: stop, drain, delete every logged folder, delete the log file, exit the CLI — previously it left a TUI attached to a sandbox that no longer existed. -- **Drain before delete.** `PtyAgent.Stop` returns when SIGTERM is *delivered*, not when the process exits, so deleting straight after it destroyed the working directory while the agent was still flushing into it. Every delete path now waits on `Runner.Done()` (5s cap) first. -- **Retention.** `--retention 30d` (default, `off` disables) sweeps aged folders at startup and hourly. Live sessions are skipped, so a session outliving the window keeps its own cwd. -- **`retask sandbox cleanup`.** Manual sweep of every log in the working directory, or one named sandbox. `--older-than 0` takes everything (prompts unless `--yes`), `--dry-run` reports only. -- **agentfleet bump** for `h:mm:ss` elapsed time past one hour. - -## Notes for reviewers - -- **Stop still never deletes** — `Stop`, `StopAll`, and the CLI-stop path contain no disk access, with regression tests pinning that. -- **Log-only, by design.** Folders already on disk before this ships have no log entry and are never auto-reaped; they need a manual `rm`. - -Design: `docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md` -Plan: `docs/superpowers/plans/2026-07-17-sandbox-session-retention.md` - -🤖 Generated with [Claude Code](https://claude.com/claude-code) -BODY -)" -``` - ---- - -## Verification checklist - -Run before calling the work done: - -- [ ] `go build ./... && go test ./...` passes in retask-cli -- [ ] `go test ./...` passes in the agentfleet fork -- [ ] `retask sandbox cleanup --dry-run` in a folder containing `package.json` leaves it untouched -- [ ] `retask help-llm | jq '.commands[] | select(.command | contains("cleanup"))'` returns the entry -- [ ] `retask sandbox connect --retention off` starts with no sweeper -- [ ] `retask sandbox connect --retention 0` errors, pointing at `off` diff --git a/docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md b/docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md deleted file mode 100644 index 4e23b2a..0000000 --- a/docs/superpowers/specs/2026-07-17-sandbox-session-retention-design.md +++ /dev/null @@ -1,388 +0,0 @@ -# Sandbox session retention & cleanup - -Date: 2026-07-17 -Status: Approved - -## Summary - -Session working folders survive being stopped. `retask sandbox connect` records -every session's start time to a per-sandbox log file and sweeps folders older -than a configurable window; a new `retask sandbox cleanup` command exposes the -same sweep for manual use. - -Deletion has two triggers, and they are deliberately distinct from stopping: - -- **Explicit delete** (`delete_session`, `delete_sandbox`) reclaims disk - immediately. `delete_sandbox` is a full teardown: it stops sessions, deletes - their folders and the log file, then exits the CLI. -- **Age** reclaims folders left behind by stop and disconnect. - -Stopping a session, stopping a sandbox, and stopping the CLI itself never delete -anything. - -Separately, the session panel's elapsed timer gains an hours component. - -## Scope - -Three deliverables: - -1. Elapsed time renders `h:mm:ss` past one hour — **in `agentfleet`, not this repo**. -2. Session folder lifecycle — session log, explicit-delete teardown for - `delete_session` / `delete_sandbox`, hourly retention sweeper, and a - `sandbox cleanup` command. -3. `help-llm` manifest updated for the new command and flags. - -## 1. Elapsed time (agentfleet) - -The panel is rendered by `github.com/hoaitan/agentfleet`, not `retask-cli`. -`tui/tui.go:renderCard` formats elapsed time as minutes:seconds, so a 90-minute -session renders `90:30`: - -```go -d := time.Since(r.StartedAt()).Round(time.Second) -elapsed = fmt.Sprintf("%02d:%02d", int(d.Minutes()), int(d.Seconds())%60) -... -elapsedStr := styleMeta.Width(5).Render(elapsed) -``` - -`agentfleet.TUIConfig` exposes `Title`, `TitleRight`, `AutoOpen`, `Log`, -`OnClose`, and `FilterLines` — no hook for elapsed formatting — and `go.mod` -carries no `replace` directive. The format cannot be overridden from -`retask-cli`. The change must land in agentfleet. - -**Decision:** hardcode the format in agentfleet rather than add a config hook. - -```go -d := time.Since(r.StartedAt()).Round(time.Second) -if h := int(d.Hours()); h > 0 { - elapsed = fmt.Sprintf("%d:%02d:%02d", h, int(d.Minutes())%60, int(d.Seconds())%60) -} else { - elapsed = fmt.Sprintf("%02d:%02d", int(d.Minutes()), int(d.Seconds())%60) -} -``` - -`styleMeta.Width(5)` widens to `Width(8)` — `1:15:30` is 7 characters and a -3-digit hour count needs 8. `renderCard` derives `nameMaxW` from -`lipgloss.Width(rightStr)`, so the name column reflows with no further change. - -Sub-hour rendering is unchanged (`05:30` stays `05:30`). - -**Sequencing:** patch agentfleet → tag a release → bump `go.mod` in `retask-cli`. -Tagging requires credentials with write access to `hoaitan/agentfleet`; the -sandbox's `gh` token has READ only. This item is independently landable and does -not block section 2 or 3. - -## 2. Session folder retention - -### 2.1 Current behaviour - -Contrary to the original premise, stop and disconnect do **not** delete session -folders today. The repo contains exactly one `os.RemoveAll` -(`internal/cmd/sandbox/sessionlane.go:213`), inside `SessionManager.Remove`, -reached only from the `delete_session` data-lane message -(`internal/cmd/sandbox/datalane.go:156`). - -| Trigger | Path | Deletes folder? | -|---|---|---| -| `stop_session` | `Stop` → PTY SIGTERM | No | -| `stop_sandbox` | `StopAll` | No | -| Disconnect / TUI exit | `StopAll` | No | -| TUI `x` on a task | `OnClose` → `Stop` + `terminate_session` | No | -| `delete_session` | `Remove` → `os.RemoveAll` | Yes | - -So "stop/disconnect must not delete folders" requires no code change. The only -change needed is to `delete_session`. - -### 2.2 Session log - -New file: `internal/cmd/sandbox/sessionlog.go`. - -Location: `/.json`, where `baseDir` is `os.Getwd()` from -`connect.go` — the same folder that holds the `session-/` directories it -tracks. Naming by sandbox id means several connected sandboxes can share one -working folder without collision. - -```json -{ - "version": 1, - "sandbox_id": "abf05a5d-5df3-45c2-9944-9dc55e4f8c1f", - "sessions": { - "5a868a9c-1320-4146-aa8b-28dae66e33ba": { - "name": "Tan's MacMini — 2026-07-16 21:17", - "dir": "session-5a868a9c-1320-4146-aa8b-28dae66e33ba", - "created_at": "2026-07-16T21:17:03Z" - } - } -} -``` - -```go -type sessionLog struct { - Version int `json:"version"` - SandboxID string `json:"sandbox_id"` - Sessions map[string]sessionEntry `json:"sessions"` -} - -type sessionEntry struct { - Name string `json:"name"` - Dir string `json:"dir"` - CreatedAt time.Time `json:"created_at"` -} -``` - -Keyed by session id: upsert is idempotent (a reconnect cannot duplicate a row) -and cleanup removal is a map delete. - -**Store semantics.** A mutex-guarded store owns the file. Concurrent -`new_session` events and the hourly sweep both mutate it, so every mutation takes -the lock, then rewrites the whole file atomically (write temp in the same -directory, `os.Rename` over the target). At tens of sessions, full rewrite is -cheaper than the complexity of incremental updates. - -**Load semantics.** A missing file yields an empty map — not an error; first run -is the common case. A file that parses but lacks `version` / `sandbox_id` / -`sessions` is treated as "not ours" and left untouched. This is what stops a -`cleanup` sweep from touching `package.json` or `tsconfig.json` in a working -folder. A log whose `version` is greater than the version this binary -understands is skipped with a warning rather than rewritten, so an older CLI -cannot silently truncate a newer log's fields. - -**Cross-process caveat:** two `connect` processes for the *same* sandbox in the -*same* cwd would race on one log file (last writer wins). Out of scope — that -configuration is already broken for other reasons (both would drive -`session-/` for the same ids). Not defended against. - -### 2.3 Write placement - -The log entry is recorded in `SessionManager.Start` (`sessionlane.go`) -**immediately before** `sb.Run(ctx, wsConn)` — not after. - -`SessionBootstrap.setupFolder` creates the folder early in `Run`, but `Run` can -fail afterward (git clone, agent config write). Under the log-only policy -(§2.4), a folder created without a log entry is invisible to cleanup forever. -Recording before bootstrap guarantees every folder we create is reapable. The -directory path is deterministic (`session-`), so nothing is lost by -recording early. - -### 2.4 Orphan folders: out of scope, by decision - -`session-*` folders with no log entry are ignored entirely. Cleanup only ever -deletes what the log lists. - -**Accepted consequence:** folders on disk before this feature ships, and any -folder created while the log was missing or deleted, are never auto-reaped and -must be removed by hand. An adoption scan (recording unclaimed folders using -filesystem mtime) was considered and explicitly rejected. - -### 2.5 Deletion policy - -Deleting is explicit and immediate; stopping never deletes. The two must not be -conflated — the whole point of the split is that a user can stop work and come -back to their files. - -| Trigger | Session PTYs | Folders | `.json` | CLI | -|---|---|---|---|---| -| User stops CLI (Ctrl-C / `kill`) | SIGTERM | keep | keep | exits | -| `stop_session` | SIGTERM | keep | keep | runs on | -| `stop_sandbox` | SIGTERM (all) | keep | keep | runs on | -| Retention sweep / `cleanup` | untouched (live skipped) | delete aged | drop entries | runs on | -| `delete_session` | SIGTERM | delete one | drop that entry | runs on | -| `delete_sandbox` | SIGTERM (all) | delete all | delete file | **exits** | - -This is enforced structurally: `Stop` and `StopAll` stay pure "signal the -process" operations containing no disk access, and deletion lives only in the -`delete_session` / `delete_sandbox` branches of the data lane. The CLI-stop path -(`connect.go:193`) calls `StopAll` and returns, so it cannot acquire deletion -behaviour by accident. - -**The drain wait.** `PtyAgent.Stop` sends SIGTERM and returns immediately — -it does not wait for the process to exit: - -```go -if err := a.cmd.Process.Signal(syscall.SIGTERM); err != nil { - return a.cmd.Process.Kill() // only if delivery failed, i.e. already gone -} -return nil -``` - -So today's `Remove` already races: it deletes the session folder while the agent -is still handling SIGTERM and may be flushing files into it. SIGTERM's entire -purpose is to grant that grace period, so deleting the directory mid-cleanup -defeats it. Every delete path therefore stops the PTY, waits on `Runner.Done()` -(closed at `runner.go:159` on process exit) up to a bounded timeout, and only -then deletes: - -```go -r.Stop() // SIGTERM -select { -case <-r.Done(): // clean exit -case <-time.After(sessionDrainTimeout): // hung; reap anyway -} -os.RemoveAll(dir) -``` - -A hung process still gets its folder reclaimed after the timeout, so a stuck -agent cannot block teardown indefinitely. Note that agentfleet never escalates -to `SIGKILL` for a process that *ignores* SIGTERM (the `Kill()` above fires only -when delivery fails), so the timeout is the only backstop. `delete_sandbox` -drains sessions concurrently, bounding total teardown at one timeout rather than -one per session. - -`sessionDrainTimeout` is 5 seconds. - -### 2.6 `delete_sandbox` teardown - -`delete_sandbox` tears the whole thing down and exits the CLI — leaving a TUI -attached to a sandbox that no longer exists is not a useful state. - -Sequence, in `SessionManager`: - -1. `StopAll` — SIGTERM every live session. -2. Drain — wait on each `Runner.Done()` concurrently, bounded by - `sessionDrainTimeout`. -3. Delete every session folder listed in the log. -4. Delete `.json`. -5. Close the log store (§2.6.1). -6. Return `errSandboxDeleted`, which unwinds `DataLane.Run`. - -Then the CLI exits. `dl.Run(ctx)` is launched as a goroutine -(`connect.go:174`), so its return currently signals nothing — `ctx` is never -cancelled and the TUI keeps running. The fix is to cancel on return: - -```go -go func() { - dl.Run(ctx) - stop() // cancel ctx: unblocks tui.Run / <-ctx.Done(), then StopAll -}() -``` - -`stop()` is the existing `signal.NotifyContext` cancel from `connect.go:94`, so -this reuses the exact path a Ctrl-C already takes — `tui.Run` returns, the -deferred `stop()` is a no-op (cancel is idempotent), and `sm.StopAll()` at -`connect.go:193` runs against an already-empty session map. Calling `stop()` -when `Run` returns for any other reason (ctx already cancelled) is equally -harmless. - -#### 2.6.1 Closing the log store - -Deleting `.json` while the retention sweeper is still alive is a -resurrection hazard: a sweep tick between step 4 and the TUI actually exiting -would rewrite the file we just deleted, leaving a log for a sandbox that no -longer exists. - -Cancelling `ctx` first would stop the sweeper but kill the TUI before teardown -finishes, so ordering alone cannot fix it. Instead the store gets an explicit -`Close()` that latches a `closed` flag under the same mutex that guards writes; -every subsequent mutation becomes a no-op. Teardown is then correct regardless -of how the sweeper and the exit path interleave. - -### 2.7 Retention sweeper - -Retention is the backstop for folders left behind by stop and disconnect — -the paths that deliberately do not delete. Explicit deletes reclaim their own -disk immediately (§2.5), so the sweeper exists for the folders nobody ever -explicitly deleted. - -`retask sandbox connect` gains `--retention` (default `30d`; `off` disables). -When enabled, a goroutine sweeps once at startup and then hourly until the -command's context is cancelled. - -Each sweep, for every entry older than the window: delete the folder, drop the -entry, rewrite the log. - -**Live-session guard.** The sweeper skips any session id currently in -`SessionManager.sessions`. Without it, a session running longer than the -retention window would have its own working directory deleted out from under its -PTY. - -`--retention 30d` is not valid `time.ParseDuration` input (no `d` unit), so a -`parseDuration` helper handles the `d` suffix on top of the standard units. - -```go -// parseDuration accepts "30d", "12h", "0". Shared by both flags. -func parseDuration(s string) (d time.Duration, err error) - -// parseRetention wraps it, additionally accepting "off". -func parseRetention(s string) (d time.Duration, enabled bool, err error) -``` - -The two flags share the duration grammar but not their keywords: `off` is valid -only for `--retention` (it is an error for `--older-than`), and `--retention 0` -is an error rather than a silent "reap everything hourly" — disabling is spelled -`off` and only `off`. - -## 3. `retask sandbox cleanup` - -New file: `internal/cmd/sandbox/cleanup.go`, wired into the `sandbox` command's -`AddCommand` block. - -``` -retask sandbox cleanup # every log in cwd, default 30d -retask sandbox cleanup # one sandbox -retask sandbox cleanup --older-than 7d -retask sandbox cleanup --older-than 0 # everything (prompts) -retask sandbox cleanup --older-than 0 --yes # everything, no prompt -retask sandbox cleanup --dry-run # report only -``` - -**Flags:** `--older-than` (default `30d`), `--dry-run`, `--yes`. - -**Vocabulary.** `--retention 30d` and `--older-than 30d` share one duration -grammar. Retention disables via `off` rather than `0`, which frees `0` to -unambiguously mean "delete everything" — the two meanings cannot collide. - -**Scope.** With no argument, every valid log in cwd (files failing the §2.2 -schema check are skipped). With an argument, only `.json`. - -**Confirmation.** `--older-than 0` prompts before deleting, because a separate -process cannot know which sessions are live. `--yes` bypasses for scripts. -Non-zero windows do not prompt. - -**Shared implementation.** The command and the sweeper call one sweep function. -The sweeper passes its live-session set; the command passes an empty set. - -## 4. `help-llm` - -`cmd/retask/main_test.go:83` asserts the hand-maintained manifest matches the -command tree — an undocumented flag fails the build. Update -`internal/cmd/helpcmd/command.go`: - -- Add `retask sandbox cleanup` with `--older-than`, `--dry-run`, `--yes`. -- Add `--retention` to the existing `retask sandbox connect` entry (line 163). - -Also update the `Long` help on `connect` (which documents its flags inline) per -the repo's help-text template. - -## 5. Testing - -| File | Cases | -|---|---| -| `sessionlog_test.go` | round-trip save/load; upsert idempotency; atomic write leaves no temp file; missing file → empty map; foreign JSON (`package.json`) rejected | -| `sessionlog_test.go` | `parseRetention`: `30d`, `12h`, `off`, invalid input | -| `sessionlog_test.go` | sweep: reaps older-than-window, keeps newer, removes entry + folder together, skips live sessions | -| `cleanup_test.go` | multi-log cwd; foreign JSON skipped; `--dry-run` deletes nothing; `--older-than 0` takes all; single-sandbox arg narrows scope | -| `sessionlane_test.go` | `delete_session` deletes folder + drops entry; `delete_sandbox` deletes all folders + the log file; drain waits for `Done()` before deleting; drain gives up after the timeout on a process that ignores SIGTERM | -| `sessionlane_test.go` | **stop does not delete**: `Stop`, `StopAll`, and the CLI-stop path leave folders and log intact | -| `sessionlog_test.go` | `Close()` latches: a sweep after teardown cannot recreate the deleted log file | -| `main_test.go` | existing manifest sync test covers the new command and flags | - -Sweep tests inject a clock and base directory rather than sleeping. Drain tests -use a fake runner exposing a controllable `Done()` channel, so the SIGTERM-race -and timeout cases are deterministic rather than timing-dependent. - -The "stop does not delete" cases are the regression guard for the distinction in -§2.5 — they fail loudly if deletion ever leaks into a stop path. - -## Decisions rejected - -- **`FormatElapsed` hook in agentfleet's `TUIConfig`** — more flexible, but a - larger API change than the format warrants. -- **Adopting orphan folders via mtime** — would have reaped the pre-existing - backlog; rejected in favour of a strict log-only policy. -- **Deferring `delete_session` folder removal to the sweeper** — briefly adopted, - then reversed: an explicit delete should reclaim its disk immediately rather - than leave the folder sitting for up to the retention window. -- **Deleting folders without draining the PTY** — matches today's behaviour, but - destroys the working directory while the agent is still handling SIGTERM. -- **`--after-days 30` (numeric)** — closer to the original phrasing, but splits - the duration vocabulary and cannot express sub-day windows. From f9b7af7e906fa243b59c9eeb3f9bb46f0d677e07 Mon Sep 17 00:00:00 2001 From: nwebbot Date: Fri, 21 Aug 2026 09:17:20 +1000 Subject: [PATCH 10/10] fix(sandbox): rename session log to sandbox_.json Avoids a bare .json sitting in the session working directory next to other JSON files with no indication of what wrote it. Co-Authored-By: Claude Sonnet 5 --- internal/cmd/helpcmd/command.go | 4 ++-- internal/cmd/sandbox/cleanup.go | 2 +- internal/cmd/sandbox/connect.go | 2 +- internal/cmd/sandbox/sessionlog.go | 8 ++++---- internal/cmd/sandbox/sessionlog_test.go | 10 +++++----- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/internal/cmd/helpcmd/command.go b/internal/cmd/helpcmd/command.go index 913e9b9..8f45d32 100644 --- a/internal/cmd/helpcmd/command.go +++ b/internal/cmd/helpcmd/command.go @@ -161,9 +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. Session folders are created in the current directory and recorded in .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 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 .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 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 index f9035eb..3b3f83d 100644 --- a/internal/cmd/sandbox/cleanup.go +++ b/internal/cmd/sandbox/cleanup.go @@ -27,7 +27,7 @@ func newCleanupCommand(gf *flags.Global) *cobra.Command { 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 .json session log are considered; any +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. diff --git a/internal/cmd/sandbox/connect.go b/internal/cmd/sandbox/connect.go index e553528..ae6d380 100644 --- a/internal/cmd/sandbox/connect.go +++ b/internal/cmd/sandbox/connect.go @@ -62,7 +62,7 @@ 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 -.json. Stopping a session, the sandbox, or this command leaves them +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: diff --git a/internal/cmd/sandbox/sessionlog.go b/internal/cmd/sandbox/sessionlog.go index d3fd47e..818f2ec 100644 --- a/internal/cmd/sandbox/sessionlog.go +++ b/internal/cmd/sandbox/sessionlog.go @@ -12,7 +12,7 @@ import ( "time" ) -// sessionLogVersion is the schema version written to .json. +// 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 @@ -26,14 +26,14 @@ type sessionLogEntry struct { CreatedAt time.Time `json:"created_at"` } -// sessionLogData is the on-disk shape of .json. +// 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 /.json. It records when each session +// 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. // @@ -50,7 +50,7 @@ type sessionLog struct { // sessionLogPath returns the log path for a sandbox in baseDir. func sessionLogPath(baseDir, sandboxID string) string { - return filepath.Join(baseDir, sandboxID+".json") + return filepath.Join(baseDir, "sandbox_"+sandboxID+".json") } func newSessionLog(baseDir, sandboxID string) *sessionLog { diff --git a/internal/cmd/sandbox/sessionlog_test.go b/internal/cmd/sandbox/sessionlog_test.go index 346aa2e..63da503 100644 --- a/internal/cmd/sandbox/sessionlog_test.go +++ b/internal/cmd/sandbox/sessionlog_test.go @@ -25,7 +25,7 @@ func TestSessionLogRecordAndLoad(t *testing.T) { 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, "sb-1.json")) + _, err = os.Stat(filepath.Join(dir, "sandbox_sb-1.json")) assert.NoError(t, err) } @@ -104,7 +104,7 @@ func TestSessionLogAtomicWriteLeavesNoTemp(t *testing.T) { names, err := filepath.Glob(filepath.Join(dir, "*")) require.NoError(t, err) require.Len(t, names, 1) - assert.Equal(t, "sb-1.json", filepath.Base(names[0])) + assert.Equal(t, "sandbox_sb-1.json", filepath.Base(names[0])) } func TestSessionLogDestroyDeletesFileAndLatches(t *testing.T) { @@ -113,12 +113,12 @@ func TestSessionLogDestroyDeletesFileAndLatches(t *testing.T) { 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, "sb-1.json")) + _, 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, "sb-1.json")) + _, err = os.Stat(filepath.Join(dir, "sandbox_sb-1.json")) assert.True(t, os.IsNotExist(err), "a closed log must not be recreated") } @@ -240,6 +240,6 @@ func TestSweepOnClosedLogIsNoop(t *testing.T) { 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, "sb-1.json")) + _, err = os.Stat(filepath.Join(dir, "sandbox_sb-1.json")) assert.True(t, os.IsNotExist(err)) }