Skip to content

Commit 54db7c3

Browse files
committed
docs: add workspace recovery spec (interrupted session detection)
Spec for detecting Copilot CLI sessions interrupted by crash/reboot via stale lock file analysis. Adds AttentionInterrupted status, R keybinding for batch resume, and workspace_recovery config toggle. Phase 1 only — dispatch stays read-only, no headless auto-resume. Design validated by dual-model board review. Resolves assessment of tamirdresher/squad-skills for dispatch integration. See #14 for tracking. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 31187c1 commit 54db7c3

1 file changed

Lines changed: 251 additions & 0 deletions

File tree

  • docs/specs/workspace-recovery
Lines changed: 251 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,251 @@
1+
# Workspace Recovery — Interrupted Session Detection
2+
3+
## Overview
4+
5+
Detect Copilot CLI sessions that were interrupted by a crash or reboot and surface them in the dispatch TUI with a distinct visual indicator and a one-key resume shortcut.
6+
7+
## Problem
8+
9+
When a machine crashes or reboots, running Copilot CLI sessions are killed. Dispatch's attention system partially detects these — sessions where the AI had finished responding (last event = `assistant.turn_end`) show yellow "needs input" dots. But sessions where the **AI was mid-generation** (last event = `assistant.turn_start` or `tool.execution`) are classified as idle because `findLivePID()` cannot distinguish "stale lock file (dead PID)" from "no lock file at all." These interrupted sessions are invisible.
10+
11+
### Current Behavior
12+
13+
| Lock File State | Last Event | Current Classification |
14+
|----------------|------------|----------------------|
15+
| No lock file | `assistant.turn_end` (recent) | AttentionWaiting |
16+
| No lock file | `tool.execution` (recent) | **AttentionIdle** |
17+
| Stale lock (dead PID) | `assistant.turn_end` (recent) | AttentionWaiting |
18+
| Stale lock (dead PID) | `tool.execution` (recent) | **AttentionIdle** (bug) |
19+
| Stale lock (dead PID) | `assistant.turn_start` (recent) | **AttentionIdle** (bug) |
20+
21+
### Root Cause
22+
23+
`findLivePID()` in `internal/data/attention.go` returns `int` — either a live PID or `0`. The value `0` conflates two semantically different states:
24+
- No lock file exists (session exited cleanly or was never running)
25+
- Lock file exists but PID is dead (session was killed by crash/reboot)
26+
27+
When the function returns `0`, `classifySession()` falls into the dead-session path which only flags `assistant.turn_end` and `assistant.message` as "waiting" — everything else becomes idle.
28+
29+
## Solution
30+
31+
### Phase 1 (This Spec)
32+
33+
Make `findLivePID` return a tri-state result that distinguishes stale locks from absent locks. Add a new `AttentionInterrupted` status for sessions with stale locks and recent activity. Surface in the TUI with a distinct icon and a one-key `R` resume shortcut.
34+
35+
**Dispatch stays read-only** — no writing to `~/.copilot/session-state/`. No lock file deletion. No headless auto-resume mode. The `ghcs --resume` command handles its own lock cleanup when sessions are actually resumed.
36+
37+
### Phase 2 (Deferred)
38+
39+
`--auto-resume` headless CLI mode for login scripts. Requires extracting launch logic from `tui.Model` into a standalone package. Deferred until Phase 1 adoption is measured.
40+
41+
## Design Decisions (From Board Review)
42+
43+
These decisions were validated by a dual-model board review (Opus 4.6 + Codex 5.3) with Business, Technical, and Legal perspectives.
44+
45+
### D1: Dispatch Stays Read-Only
46+
47+
Dispatch never writes to `~/.copilot/session-state/`. The proposed `CleanStaleLock` function was vetoed by Security and Tech Purist due to:
48+
- TOCTOU race: between `IsProcessAlive(pid)=false` and `os.Remove(lock)`, a new process could claim the PID
49+
- Symlink attack: a replaced lock file could cause dispatch to delete an unrelated file
50+
- Ownership violation: dispatch deleting another tool's files violates least surprise and could interfere with future Copilot CLI self-recovery
51+
52+
Let `ghcs --resume {id}` handle its own locks. Dispatch is a reader, not a writer.
53+
54+
### D2: Separate AttentionInterrupted vs Enhancing AttentionWaiting
55+
56+
Keep them as distinct statuses. "Interrupted" means "process was killed mid-work, needs recovery." "Waiting" means "AI finished, ball is in your court." These require different user actions and have different urgency levels. Collapsing them would lose signal.
57+
58+
### D3: 72-Hour Detection Window
59+
60+
Use `interruptedMaxAge = 72 * time.Hour` for stale-lock sessions, separate from the existing `deadSessionMaxAge = 24 * time.Hour` for sessions without lock files. This ensures Friday afternoon crashes are still visible on Monday morning.
61+
62+
### D4: AttentionInterrupted at Iota Position 4
63+
64+
Place after `AttentionWaiting = 3` to preserve backward compatibility. The TUI sorts by attention status numerically (higher = more urgent), so interrupted sessions correctly sort as highest priority. No code serializes enum values to disk, so the ordering is safe.
65+
66+
### D5: Config Toggle Required
67+
68+
`workspace_recovery` config field (default `true`). Users encountering false positives (e.g., Copilot CLI not cleaning locks on graceful exit) can disable the feature without downgrading dispatch.
69+
70+
### D6: Fix Both Scan Paths
71+
72+
Both `ScanAttentionQuick()` (fast initial render) and `ScanAttention()` (full background scan) must handle stale locks. If only `classifySession()` is updated, interrupted sessions briefly appear as idle then flip to interrupted — a visible flicker bug that undermines trust.
73+
74+
### D7: Defensive Lock File Parsing
75+
76+
The `inuse.{PID}.lock` format is undocumented by the Copilot CLI team with no stability SLA. Mitigate with:
77+
- File size < 32 bytes
78+
- Content is pure ASCII digits
79+
- PID > 0 and < platform max (4194304 on Linux, ~4M on Windows)
80+
- Log warnings via `slog` for unexpected formats (early detection of format changes)
81+
- Graceful degradation: any validation failure = treat as no lock file (AttentionIdle)
82+
83+
## Technical Specification
84+
85+
### 1. pidResult Struct
86+
87+
File: `internal/data/attention.go`
88+
89+
```go
90+
type pidResult struct {
91+
pid int // >0 if live process found, 0 otherwise
92+
hasStale bool // true if lock file exists with dead PID
93+
}
94+
```
95+
96+
Change `findLivePID(dir string) int` to `findSessionPID(dir string) pidResult`.
97+
98+
Logic:
99+
1. Glob `inuse.*.lock` files in session directory
100+
2. If no matches: return `{pid: 0, hasStale: false}`
101+
3. For each lock file:
102+
a. Validate: size < 32 bytes, content is pure digits, PID > 0, PID < platform max
103+
b. If validation fails: log warning, skip file
104+
c. If `platform.IsProcessAlive(pid)`: return `{pid: pid, hasStale: false}`
105+
4. If all lock files had dead PIDs: return `{pid: 0, hasStale: true}`
106+
107+
### 2. AttentionInterrupted Status
108+
109+
File: `internal/data/models.go`
110+
111+
```go
112+
const (
113+
AttentionIdle AttentionStatus = iota // 0 — not running
114+
AttentionStale // 1 — running, quiet
115+
AttentionActive // 2 — AI working
116+
AttentionWaiting // 3 — needs user input
117+
AttentionInterrupted // 4 — crashed/killed mid-work
118+
)
119+
```
120+
121+
File: `internal/data/attention.go`
122+
123+
New constant:
124+
```go
125+
const interruptedMaxAge = 72 * time.Hour
126+
```
127+
128+
Updated `classifySession()` logic when `result.hasStale == true`:
129+
- Read last event from `events.jsonl`
130+
- If no events or read error: return `AttentionIdle`
131+
- If event age > `interruptedMaxAge`: return `AttentionIdle`
132+
- If last event is `assistant.turn_end` or `assistant.message`: return `AttentionWaiting` (already actionable)
133+
- Otherwise: return `AttentionInterrupted`
134+
135+
Updated `ScanAttentionQuick()` logic:
136+
- When `result.hasStale == true`: return `AttentionInterrupted` (preliminary classification, refined by full scan)
137+
- This prevents the idle-to-interrupted flicker
138+
139+
Add structured logging:
140+
```go
141+
slog.Info("attention.interrupted",
142+
"session_id", sessionID,
143+
"stale_pid", stalePID,
144+
"last_event", evt.Type,
145+
"event_age", time.Since(eventTime))
146+
```
147+
148+
### 3. Config Toggle
149+
150+
File: `internal/config/config.go`
151+
152+
```go
153+
type Config struct {
154+
// ... existing fields ...
155+
WorkspaceRecovery bool `json:"workspace_recovery"` // default true
156+
}
157+
```
158+
159+
Default: `true` in `defaultConfig()`.
160+
161+
When `false`: `classifySession()` and `ScanAttentionQuick()` treat `hasStale` the same as `!hasStale` — current behavior, no interrupted detection.
162+
163+
Add to config panel UI with description: "Detect sessions interrupted by crash/reboot".
164+
165+
### 4. TUI Rendering
166+
167+
**Dot/icon**: Use a visually distinct indicator for interrupted — NOT another colored version of the same circle dot. Options:
168+
- `⚡` (lightning bolt) — conveys "interrupted/crashed"
169+
- `▲` (filled triangle) — conveys "warning/attention needed"
170+
- Color: orange or red, distinct from yellow (waiting) and green (active)
171+
172+
**Attention picker** (`attentionpicker.go`): Add "Interrupted (N sessions)" entry with the new icon.
173+
174+
**Status bar**: When interrupted count > 0, show badge: `⚡ N interrupted — press R to resume`
175+
176+
**Help overlay**: Add description for interrupted status and R keybinding.
177+
178+
### 5. Resume Interrupted Keybinding (R)
179+
180+
File: `internal/tui/keys.go`
181+
182+
```go
183+
ResumeInterrupted: key.NewBinding(
184+
key.WithKeys("R"),
185+
key.WithHelp("R", "resume interrupted sessions"),
186+
),
187+
```
188+
189+
File: `internal/tui/model.go`
190+
191+
Handler:
192+
1. Collect all session IDs with `AttentionInterrupted` from `m.attentionMap`
193+
2. Match against loaded sessions to get `Session` objects (need `Cwd` for launch)
194+
3. If none found: set `m.statusInfo = "No interrupted sessions"`; return
195+
4. Batch launch via `m.batchLaunchSessions(sessions, mode)`:
196+
- Force external mode (tab or window — never in-place for batch)
197+
- Existing 50-session cap applies
198+
5. Set `m.statusInfo = fmt.Sprintf("Resumed %d interrupted sessions", len(sessions))`
199+
6. Do NOT delete lock files — dispatch stays read-only
200+
201+
### 6. Test Cases
202+
203+
File: `internal/data/attention_test.go`
204+
205+
| # | Scenario | Expected Result |
206+
|---|----------|----------------|
207+
| 1 | Stale lock + `tool.execution` + recent event | `AttentionInterrupted` |
208+
| 2 | Stale lock + `assistant.turn_start` + recent event | `AttentionInterrupted` |
209+
| 3 | Stale lock + `tool.execution` + old event (>72h) | `AttentionIdle` |
210+
| 4 | Stale lock + `assistant.turn_end` + recent event | `AttentionWaiting` (not interrupted) |
211+
| 5 | Stale lock + `assistant.message` + recent event | `AttentionWaiting` (not interrupted) |
212+
| 6 | No lock + `tool.execution` + recent event | `AttentionIdle` (unchanged) |
213+
| 7 | Multiple stale locks (different dead PIDs) | `AttentionInterrupted` |
214+
| 8 | Stale lock + empty/missing events.jsonl | `AttentionIdle` |
215+
| 9 | Malformed lock file (non-numeric content) | `AttentionIdle` (graceful degradation) |
216+
| 10 | Oversized lock file (>32 bytes) | `AttentionIdle` (graceful degradation) |
217+
| 11 | Lock with negative PID | `AttentionIdle` (graceful degradation) |
218+
| 12 | Config `workspace_recovery = false` + stale lock | `AttentionIdle` (feature disabled) |
219+
| 13 | `ScanAttentionQuick` with stale lock | `AttentionInterrupted` (no flicker) |
220+
221+
File: `internal/config/config_test.go`
222+
223+
| # | Scenario | Expected Result |
224+
|---|----------|----------------|
225+
| 14 | Default config | `WorkspaceRecovery == true` |
226+
| 15 | Explicit `workspace_recovery: false` in JSON | `WorkspaceRecovery == false` |
227+
228+
## Known Risks
229+
230+
| Risk | Severity | Mitigation |
231+
|------|----------|------------|
232+
| Lock file format changes in future Copilot CLI | Medium | Defensive parsing with size/format validation. Graceful degradation to Idle. slog warnings for early detection. |
233+
| PID reuse false negatives | Low | On long-running machines, an unrelated process could reuse a stale PID, making the lock appear "live." Undetectable but very low probability. |
234+
| False positives if Copilot CLI doesn't clean locks on graceful exit | Low | Config toggle lets users disable. Will document in help text. |
235+
| Undocumented dependency on Copilot CLI internals | Medium | Feature is purely additive. Worst case: detection stops working and all sessions show as idle (current behavior). No regression path. |
236+
237+
## Out of Scope
238+
239+
- `--auto-resume` headless CLI mode (Phase 2)
240+
- Login script registration (Task Scheduler / systemd / launchd) (Phase 2)
241+
- Lock file cleanup/deletion (vetoed — dispatch stays read-only)
242+
- Workspace snapshot file (unnecessary — stale lock files are the signal)
243+
- Cross-machine session tracking
244+
- Integration with other squad-skills features (assessed and rejected — wrong domain)
245+
246+
## Origin
247+
248+
This feature was identified during an assessment of [tamirdresher/squad-skills](https://github.com/tamirdresher/squad-skills) for integration into dispatch. Of 22 plugins evaluated:
249+
- **Session Recovery** — already built into dispatch (full TUI browser)
250+
- **Restart Recovery** — inspired this feature (stale lock detection + resume)
251+
- **20 other plugins** — not relevant (Teams/Outlook automation, cross-machine coordination, blog writing, etc.)

0 commit comments

Comments
 (0)