feat: Stale Worktree Hints#19
Merged
Merged
Conversation
Add idle-staleness affordances built on the merged RecencyOf signal: - wt list: append a "⚠ idle" marker to the Last Active cell in the recent human and --status layouts; emit an additive `idle` boolean JSON field (omitempty) so default JSON output stays stable. - wt delete: annotate idle rows in the interactive menu, add an "All idle (N)" entry (hidden when N=0, defaultIdx 2→3 to keep newest pre-selected), and a --stale[=Nd] selector (pflag NoOptDefVal=7d; = required); --stale is mutually exclusive with positional names and --delete-all (ExitInvalidArgs). - internal/worktree: IsIdle predicate, DefaultIdleThreshold (7d), and ParseIdleThreshold (Nd form) in a new idle.go. Idleness only selects deletion candidates; every removal still routes through the existing per-worktree stash/unpushed/rollback safety flow. Signal is dir-mtime (one definition across list/open/delete, no new git subprocess), framed honestly as "untouched on disk for Nd" — under-reporting is safe-by-direction.
There was a problem hiding this comment.
Pull request overview
Adds an “idle” (stale-on-disk) signal derived from existing worktree recency (dir mtime) and surfaces it in wt list and wt delete to help users safely clean up old worktrees without introducing any new deletion path.
Changes:
- Introduces shared idle detection utilities (
IsIdle,DefaultIdleThreshold,ParseIdleThreshold) ininternal/worktree. - Marks idle worktrees in
wt list(human tables) and exposes an additiveidleJSON field under--status. - Adds stale-aware
wt deleteUX: menu row annotation + “All idle (N)” entry, plus--stale[=Nd]selector routing through existing safety flow.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/internal/worktree/idle.go | Adds idle predicate, default threshold, and Nd parser used by list/delete. |
| src/internal/worktree/idle_test.go | Unit tests for idle boundary conditions and threshold parsing. |
| src/cmd/wt/list.go | Adds Idle *bool, populates it from persisted LastActive, renders ⚠ idle, keeps JSON stable by default. |
| src/cmd/wt/list_test.go | Covers idle marker rendering and JSON idle presence/absence expectations. |
| src/cmd/wt/delete.go | Adds --stale selector, stale-aware menu annotation, and “All idle (N)” option. |
| src/cmd/wt/delete_test.go | Tests stale menu behavior, selector behavior, mutexes, and empty-match behavior. |
| fab/changes/260530-5fyu-stale-worktree-hints/spec.md | Feature spec documenting requirements and decisions. |
| fab/changes/260530-5fyu-stale-worktree-hints/plan.md | Implementation plan and acceptance checklist. |
| fab/changes/260530-5fyu-stale-worktree-hints/intake.md | Intake notes updated to match final scope/decisions. |
| fab/changes/260530-5fyu-stale-worktree-hints/.status.yaml | Pipeline status/metrics update. |
| fab/changes/260530-5fyu-stale-worktree-hints/.history.jsonl | Change history log updated. |
| docs/memory/wt-cli/recency-ordering-contract.md | Updates contract for delete menu default index and idle annotation. |
| docs/memory/wt-cli/list-status-contract.md | Documents idle pointer field + ⚠ idle rendering behavior. |
| docs/memory/wt-cli/idle-staleness-contract.md | New cross-command contract for idle/stale behavior and invariants. |
| docs/memory/index.md | Adds the new idle/staleness contract to the memory index. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+50
to
+59
| numPart := strings.TrimSuffix(s, "d") | ||
| days, err := strconv.Atoi(numPart) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("invalid threshold %q: expected a day-suffixed integer like 7d or 30d", s) | ||
| } | ||
| if days <= 0 { | ||
| return 0, fmt.Errorf("invalid threshold %q: day count must be a positive integer like 7d or 30d", s) | ||
| } | ||
|
|
||
| return time.Duration(days) * 24 * time.Hour, nil |
Owner
Author
There was a problem hiding this comment.
Fixed — ParseIdleThreshold now rejects day counts that overflow int64 time.Duration (cap ~106751d) instead of wrapping to a negative threshold that would make IsIdle select every worktree; added overflow-guard tests. (4c90d0c)
Comment on lines
+534
to
+538
| // formatThreshold renders a whole-day duration back as an Nd string for the | ||
| // informational empty-state message, matching the --stale=Nd input form. | ||
| func formatThreshold(d time.Duration) string { | ||
| return fmt.Sprintf("%dd", int(d.Hours())/24) | ||
| } |
Owner
Author
There was a problem hiding this comment.
Fixed — formatThreshold now uses integer duration division (d / (24*time.Hour)) instead of float d.Hours()/24 truncation. (4c90d0c)
- ParseIdleThreshold: reject day counts that overflow int64 time.Duration (a wrapped-negative threshold would make IsIdle select every worktree — dangerous for --stale deletion). Add overflow guard + tests. - formatThreshold: use integer duration division instead of float d.Hours()/24 truncation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Meta
Pipeline: intake ✓ → spec ✓ → apply ✓ → review ✓ → hydrate ✓ → ship ✓ → review-pr
Summary
Worktrees accumulate, and without a stale signal users have no prompt to clean up branches they finished with weeks ago — they clutter
wt list/wt openmenus and consume disk. This change turns the recency data (from the merged recency-aware listing change) into a cleanup signal: a non-destructive idle marker inwt listand an idle-aware selector inwt delete. Idleness only ever selects candidates — every removal still routes through the existing rollback-safe per-worktree safety flow.Changes
IsIdlepredicate (now - RecencyOf(wt) > threshold),DefaultIdleThreshold(7d), andParseIdleThreshold(Ndform) in the newsrc/internal/worktree/idle.go. Per Constitution V the predicate lives ininternal/worktree;cmd/only consumes it.wt listidle marker: appends⚠ idleto the existingLast Activecolumn in the recent human and--statuslayouts — no newos.Stat, reusing the already-persisted recency key. JSON mode emits an additiveidleboolean field (omitempty), keeping default JSON output stable per the audience-split contract.wt delete: the interactive menu annotates idle rows (feature-x (feat-x) — 41d, idle) and adds an "All idle (N)" entry beside "All (N worktrees)" (hidden when N=0;defaultIdxshifts 2→3 to keep the newest worktree pre-selected). A--stale[=Nd]selector (pflagNoOptDefVal=7d;=required) pre-selects idle candidates. Both route through the existinghandleDeleteMultipleflow — no new deletion path.Design notes
Signal choice (dir-mtime + honest framing): the idle signal reuses the single dir-mtime
RecencyOfdefinition rather than inventing a parallel signal. This costs zero newgitsubprocesses and keeps one signal definition acrosslist/open/delete. dir-mtime under-reports staleness (a worktree touched by a build/fab synclooks fresh), so it is framed honestly as "idle / untouched on disk for Nd" — not "you haven't worked here." A cleaner per-staleness signal (e.g. last-commit-date) was rejected for this change: it costs agitsubprocess per worktree and a second divergent definition, for accuracy the safety flow already backstops.Safety invariant: idleness is never the sole delete gate. The existing per-worktree
HasUnpushedCommits/HasUncommittedChanges/ rollback flow runs on every removal. mtime under-reporting is safe-by-direction — it can hide an idle worktree but never expose an unsafe one. The main worktree is structurally excluded (the menu already skipsctx.RepoRoot). The--stale↔positional-names and--stale↔--delete-allmutexes (exitExitInvalidArgs) convert the silent--stale 30dparse trap into a loud error, matching the existing--path↔--statusidiom.Test coverage
internal/worktree/idle_test.go: idle boundary cases (just-under, just-over, exactly-at), zero-recency idle, andParseIdleThresholdvalidNdforms plus rejects (non-dsuffix, non-integer,0d, negative, empty).cmd/wt/list_test.go(viaos.Chtimes): 4-col and 5-col idle markers (old marked, fresh and main not), name/branch modes show no marker, JSONidleabsent in default/--jsonand present+boolean under--status.cmd/wt/delete_test.go(controlled mtimes, non-interactive / EOF-cancel paths only — no real interactive side effects): menu idle annotation, "All idle (N)" entry +defaultIdxshift, no entry when none idle,--stalesubset deletion,--stale=Ndoverride, zero-match message + exit 0, positional/--delete-allmutexes, invalid threshold, and main never targeted.