Skip to content

Commit 6d1d807

Browse files
committed
planning: add issue 2.1-resume-work-with-existing-worktree
1 parent 317356b commit 6d1d807

2 files changed

Lines changed: 169 additions & 0 deletions

File tree

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
# Plan: Resume Work With Existing Worktree
2+
3+
## Goal
4+
5+
Update `/cat:work` (the `work-prepare` phase implemented in `WorkPrepare.java`) to detect when the
6+
current session already holds the lock on an issue whose worktree directory already exists, and
7+
resume seamlessly by returning a `READY` result with the existing worktree path — instead of
8+
returning an `ERROR`.
9+
10+
## Parent Requirements
11+
12+
None
13+
14+
## Approaches
15+
16+
### A: Handle `ExistingWorktree` in `WorkPrepare.handleNonFoundResult`
17+
18+
- **Risk:** LOW
19+
- **Scope:** 2 files (`WorkPrepare.java`, `WorkPrepareTest.java`)
20+
- **Description:** When `handleNonFoundResult` encounters an `ExistingWorktree` result, check
21+
whether `input.sessionId()` owns the lock. If yes, bypass worktree creation and build the `READY`
22+
JSON directly from the existing worktree. If no, return ERROR as today.
23+
24+
### B: Handle `ExistingWorktree` in `IssueDiscovery` by returning `Found`
25+
26+
- **Risk:** MEDIUM
27+
- **Scope:** 3+ files (changes `IssueDiscovery`'s sealed result hierarchy)
28+
- **Description:** Promote `ExistingWorktree` to a `Found` variant inside `IssueDiscovery`.
29+
Requires modifying the sealed interface and all switch arms that handle it.
30+
31+
> **Selected: Approach A** — lowest risk, minimal surface area, no sealed-interface changes.
32+
> The session ID is available in `WorkPrepare.execute` via `input.sessionId()`, and the lock can
33+
> be checked with `issueLock.check(issueId)`.
34+
35+
## Research Findings
36+
37+
### Current Code Path
38+
39+
1. `WorkPrepare.execute``IssueDiscovery.findNextIssue`
40+
2. `IssueDiscovery` calls `issueLock.acquire(issueId, sessionId, "")`:
41+
- Returns `LockResult.Acquired` when the lock is already held by the same session.
42+
- Returns `LockResult.Locked` when another session holds it (→ `NotExecutable`).
43+
3. After a successful acquire, `IssueDiscovery` checks whether the worktree directory exists:
44+
- If it does → returns `DiscoveryResult.ExistingWorktree`.
45+
- If not → returns `DiscoveryResult.Found`.
46+
4. `WorkPrepare.handleNonFoundResult` handles `ExistingWorktree` today with:
47+
```java
48+
return mapper.writeValueAsString(Map.of(
49+
"status", "ERROR",
50+
"message", "Issue " + existingWorktree.issueId() + " has an existing worktree at: " +
51+
existingWorktree.worktreePath()));
52+
```
53+
54+
### Key Types
55+
56+
- `IssueDiscovery.DiscoveryResult.ExistingWorktree` record fields:
57+
`issueId`, `major`, `minor`, `patch`, `issueName`, `issuePath`, `worktreePath`
58+
- `IssueLock.check(issueId)``LockResult.CheckLocked` (when locked) or `CheckUnlocked`
59+
- `IssueLock.CheckLocked` fields: `sessionId()`, `worktree()`, `ageSeconds()`
60+
- Lock files live at `{projectCatDir}/locks/{issueId}.lock`
61+
62+
### READY JSON contract (from `executeWithLock`)
63+
64+
The `READY` response must include:
65+
```
66+
status, issue_id, major, minor, issue_name, issue_path (worktree-relative),
67+
worktree_path, issue_branch, target_branch, estimated_tokens, percent_of_threshold,
68+
goal, preconditions, approach_selected, lock_acquired,
69+
has_existing_work, existing_commits, commit_summary
70+
```
71+
72+
### `buildIssueBranch` signature
73+
74+
```java
75+
private String buildIssueBranch(String major, String minor, String patch, String issueName)
76+
```
77+
78+
### Helper methods available in `WorkPrepare`
79+
80+
- `buildIssueBranch(major, minor, patch, issueName)` — builds the branch name string
81+
- `estimateTokens(planPath)` — returns token count
82+
- `IssueGoalReader.readGoalFromPlan(planPath)` — reads the `## Goal` section
83+
- `readPreconditionsFromPlan(planPath)` — reads `## Pre-conditions` items
84+
- `ExistingWorkChecker.check(worktreePath, targetBranch)` — checks for existing commits
85+
- `checkTargetBranchCommits(projectDir, targetBranch, issueName, planPath)` — suspicious commit check
86+
- `GitCommands.getCurrentBranch(projectDir.toString())` — reads current branch
87+
88+
## Risk Assessment
89+
90+
- **Risk Level:** LOW
91+
- **Concerns:** The `ExistingWorktree` result carries all fields needed to build the READY response
92+
except `estimatedTokens` and existing-work metadata — these can be recomputed.
93+
- **Mitigation:** Recompute `estimatedTokens`, `goal`, `preconditions`, `existingWork`, and
94+
`suspiciousCommits` from the existing worktree path using the same helpers already used in
95+
`executeWithLock`.
96+
97+
## Files to Modify
98+
99+
- `client/src/main/java/io/github/cowwoc/cat/hooks/util/WorkPrepare.java`
100+
- Modify `handleNonFoundResult` to accept `PrepareInput input` as a parameter.
101+
- When result is `ExistingWorktree`: check the lock owner. If it matches `input.sessionId()`,
102+
call `resumeWithExistingWorktree(input, existingWorktree, projectDir, mapper)`.
103+
- Add private method `resumeWithExistingWorktree` that builds the READY JSON.
104+
- `client/src/test/java/io/github/cowwoc/cat/hooks/test/WorkPrepareTest.java`
105+
- Add `executeReturnsReadyWhenSessionOwnsLockAndWorktreeExists` test.
106+
- Add `executeReturnsErrorWhenDifferentSessionOwnsLockAndWorktreeExists` test (confirm no
107+
regression: another session's existing worktree still returns LOCKED, not ERROR, because the
108+
lock check in `IssueDiscovery` fires first and returns `NotExecutable` before `ExistingWorktree`
109+
can be reached).
110+
111+
## Pre-conditions
112+
113+
- [ ] All dependent issues are closed
114+
115+
## Sub-Agent Waves
116+
117+
### Wave 1
118+
119+
- Write failing tests `executeReturnsReadyWhenSessionOwnsLockAndWorktreeExists` in
120+
`WorkPrepareTest.java` (TDD — write test first, verify it fails, then implement).
121+
- Files: `client/src/test/java/io/github/cowwoc/cat/hooks/test/WorkPrepareTest.java`
122+
- Implement the resume logic in `WorkPrepare.java`:
123+
1. Add `PrepareInput input` parameter to `handleNonFoundResult` (update the single call site in
124+
`execute`).
125+
2. In the `ExistingWorktree` branch of `handleNonFoundResult`:
126+
a. Read the lock: `IssueLock.LockResult check = issueLock.check(existingWorktree.issueId())`.
127+
b. If `check instanceof IssueLock.LockResult.CheckLocked cl && cl.sessionId().equals(input.sessionId())`:
128+
- Call `resumeWithExistingWorktree(input, existingWorktree, projectDir, mapper)`.
129+
c. Otherwise: keep the current ERROR response.
130+
3. Add `private String resumeWithExistingWorktree(PrepareInput input,
131+
IssueDiscovery.DiscoveryResult.ExistingWorktree existing, Path projectDir, JsonMapper mapper)`
132+
which:
133+
- Derives `issueBranch` via `buildIssueBranch(existing.major(), existing.minor(),
134+
existing.patch(), existing.issueName())`.
135+
- Derives `targetBranch` via `GitCommands.getCurrentBranch(projectDir.toString())`.
136+
- Reads `planPath` from `Path.of(existing.issuePath()).resolve("PLAN.md")` — note:
137+
`existing.issuePath()` is the **main workspace** issue directory (not inside the worktree),
138+
so the plan file must be read from the worktree path instead:
139+
`Path.of(existing.worktreePath()).resolve(projectDir.relativize(Path.of(existing.issuePath())).toString()).resolve("PLAN.md")`.
140+
- Calls `estimateTokens(planPath)`.
141+
- Returns OVERSIZED JSON if token limit exceeded.
142+
- Calls `ExistingWorkChecker.check(existing.worktreePath(), targetBranch)`.
143+
- Calls `checkTargetBranchCommits(projectDir, targetBranch, existing.issueName(), planPath)`.
144+
- Calls `IssueGoalReader.readGoalFromPlan(planPath)`.
145+
- Calls `readPreconditionsFromPlan(planPath)`.
146+
- Builds and returns the READY JSON with `worktree_path = existing.worktreePath()`.
147+
- Files: `client/src/main/java/io/github/cowwoc/cat/hooks/util/WorkPrepare.java`
148+
- Run `mvn -f client/pom.xml test` to verify all tests pass.
149+
- Files: (build only)
150+
151+
## Post-conditions
152+
153+
- [ ] `WorkPrepareTest.executeReturnsReadyWhenSessionOwnsLockAndWorktreeExists` passes: when the
154+
current session owns the lock and the worktree directory already exists, `execute` returns
155+
`status = READY` with the existing `worktree_path`.
156+
- [ ] `WorkPrepareTest.executeReturnsLockedWhenIssueIsLockedAndWorktreeExists` still passes (no
157+
regression: another session locking an issue with an existing worktree still returns `LOCKED`).
158+
- [ ] All existing `WorkPrepareTest` tests pass.
159+
- [ ] `mvn -f client/pom.xml test` exits with code 0.
160+
- [ ] E2E: Running `/cat:work resume <issue-id>` (or `/cat:work <issue-id>`) when the same session
161+
already holds the lock and a worktree exists proceeds directly to the implement/confirm/review/merge
162+
phases without prompting to clean up the worktree.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# State
2+
3+
- **Status:** open
4+
- **Progress:** 0%
5+
- **Dependencies:** []
6+
- **Blocks:** []
7+
- **Target Branch:** v2.1

0 commit comments

Comments
 (0)