Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 92 additions & 1 deletion grove-v1-lease-first-implementation-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -831,7 +831,9 @@ main (stable, current semver — releasable)

- Integration branch: `feat/lease-first-v1` (created from `main`).
- PR 1: merged — `feat/lease-first-pr1-schemas-transitions` → `feat/lease-first-v1`.
- PR 2: in progress — `feat/lease-first-pr2-acquire` → `feat/lease-first-v1`.
- PR 2: merged — `feat/lease-first-pr2-acquire` → `feat/lease-first-v1`.
- PR 3: in review — `feat/lease-first-pr3-release` → `feat/lease-first-v1` (PR #34).
- PR 4: next after PR 3 — `feat/lease-first-pr4-destroy` → `feat/lease-first-v1`.

## PR Split

Expand Down Expand Up @@ -923,6 +925,31 @@ PR1 (schemas + transitions)
- Integration tests: path boundary, worktree removal, process safety, idempotent
destroy.

**Carried over from PR 3 review (dedup refactors — do here, not in PR 3):**

- `assertWorktreeSafeForCleanup()` — consolidate `isWorktreeInUse` +
`ownerAlive` + `UnsafeCleanupError` checks currently duplicated in
`lease-release.ts` (`scanProcessSafety`), `pool.ts` (`destroy`, `destroyAll`,
`repair` force-destroy). PR 4 rewrites destroy paths; one helper avoids a
third copy and keeps release/destroy safety semantics aligned.
- `findLeaseByIdOrPath()` in `pool-state.ts` — same lookup in
`lease-release.ts` `beginRelease()` and `pool.ts` `destroy()`. Extract when
destroy is touched.
- `applyLeaseSlotQuarantine()` (or equivalent) — quarantine lease + slot
transition pattern appears in `lease-acquire.ts`, `lease-release.ts`
(`quarantineFailedRelease`, quarantine finalize), `pool.ts` repair, and will
appear again on failed destroy. Single helper reduces drift across lifecycle
stages.
- Merge `resumeCleanupLease()` repair transition + `loadReleasingContext()` into
one locked pass (mirrors `resumeAcquireLease()`). Correct today; cosmetic only.
Low priority unless PR 4 touches `resume-cleanup` again.

**Why deferred from PR 3:** PR 3 scope was release WAL + `ReleaseResult` only.
These refactors are cross-cutting (touch `pool.ts` destroy/repair as well as
`lease-release.ts`), widen the diff, and raise merge-conflict risk with PR 4's
destroy rewrite. No known behavioral bug once PR 3 post-review fixes land (hook
env, enrichment outside lock).

**Defer:** `deleteBranch` on destroy (already in codebase; out of MVP scope).

### PR 5 — Repair and transition enforcement
Expand Down Expand Up @@ -1127,3 +1154,67 @@ Branch: `feat/lease-first-pr2-acquire`
- `packages/grove/test/pool.test.ts`, `grove.integration.test.ts`
- `packages/grove/test/helpers/hook-probe.mjs`
- `grove-v1-lease-first-implementation-plan.md`

## PR 3 Implementation Summary (Completed)

Branch: `feat/lease-first-pr3-release`

- **What was done**

- Added `ReleaseResult` discriminated union (`preserved`, `released`, `quarantined`).
- Implemented `lease-release.ts` with WAL release: all policies enter `releasing`
and persist `pendingCleanup` before side effects.
- Reset: fresh process scan, `git reset --hard`, `git clean -fd` (or `-fdx` with
`cleanIgnored: true`), `RELEASE_RESET_COMPLETE`, slot `RELEASE_TO_POOL`.
- Preserve: clears owner only via `RELEASE_PRESERVE_COMPLETE`; no git reset/clean.
- Quarantine cleanup policy via `RELEASE_START` + `QUARANTINE` finalize.
- Failed reset quarantines lease and slot; preserves `pendingCleanup` for repair.
- `repair({ action: "resume-cleanup" })` via `resumeCleanupLease()` (handles
quarantined and interrupted `releasing` states).
- CLI release/repair commands updated for `ReleaseResult` return type.
- Post-review fixes: shared `buildLeaseHookEnv()` (README hook contract for all
lifecycle hooks), `enrichLeaseReadOnly()` moved outside the state lock in
finalize, single process scan on reset `beginRelease`, lazy `getDefaultBranch`
only when reset omits `resetTo`, `isReleaseResult()` for CLI narrowing,
`quarantineFailedRelease` throws instead of silent no-op.

- **How it was done**

- `releaseLease()` and `resumeCleanupLease()` share `completeRelease()` for
hooks, reset side effects, and transition-driven finalize.
- `toLeaseFirstCleanupIntent()` normalizes reset options with default branch.
- `pool.ts` delegates lease release and resume-cleanup to `lease-release.ts`.
- `buildLeaseHookEnv()` in `lease-view.ts` is the single hook env builder for
acquire, release, and destroy.

- **Why it was done**

- PR 3 delivers durable release with crash-recoverable cleanup intent and typed
results orchestrators need before destroy/repair enforcement in PR 4–5.

- **Deferred to PR 4 (and why)**

- **Process-safety helper** (`assertWorktreeSafeForCleanup`) — duplicated across
release and destroy paths; extract when PR 4 rewrites destroy (see PR 4
section).
- **Lease lookup helper** (`findLeaseByIdOrPath`) — same id-or-path lookup in
release and destroy; extract alongside destroy.
- **Quarantine transition helper** — four similar call sites today; PR 4 adds a
fifth on failed destroy — better to dedupe once destroy lands.
- **`resume-cleanup` double lock** — repair transition and context load are two
locked sections; merge is cosmetic, not a correctness gap.
- **`repair()` return union** (`GroveLease | ReleaseResult | void`) — defer typed
`RepairResult` or action overloads to **PR 5** (repair matrix + mutator
enforcement); `isReleaseResult()` is sufficient for CLI until then.

- **Files worked on**

- `packages/grove/src/lease-release.ts` (new)
- `packages/grove/src/lease-view.ts` — `buildLeaseHookEnv`
- `packages/grove/src/types.ts` — `ReleaseResult`, `isReleaseResult`
- `packages/grove/src/git/worktree.ts` — `cleanIgnored` on reset
- `packages/grove/src/pool.ts` — delegate release/repair cleanup
- `packages/grove/src/index.ts` — export `ReleaseResult`, `isReleaseResult`
- `packages/grove-cli/src/commands/release.ts`, `repair.ts`
- `packages/grove/test/lease.integration.test.ts`
- `grove-v1-lease-first-implementation-plan.md`
8 changes: 5 additions & 3 deletions packages/grove-cli/src/commands/release.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,16 @@ export const releaseCmd = new Command("release")
}
}

const lease = await grove.release(targetPath, releaseOpts);
const result = await grove.release(targetPath, releaseOpts);

if (options.json) {
process.stdout.write(JSON.stringify(lease, null, 2) + "\n");
process.stdout.write(JSON.stringify(result, null, 2) + "\n");
return;
}

console.error(pc.green(`🌳 Lease ${lease.leaseId} released with action ${options.cleanup}.`));
console.error(
pc.green(`🌳 Lease ${result.leaseId} released with action ${options.cleanup} (${result.status}).`),
);
return;
}

Expand Down
16 changes: 12 additions & 4 deletions packages/grove-cli/src/commands/repair.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isReleaseResult } from "@ferueda/grove";
import { Command } from "commander";
import { loadGrove } from "../utils.js";
import { handleError } from "../error-handler.js";
Expand All @@ -17,12 +18,12 @@ export const repairCmd = new Command("repair")
}

const grove = await loadGrove({ repo: options.repo });
const lease = await grove.repair({
const result = await grove.repair({
leaseId,
action: options.action as any,
force: options.force,
});
if (!lease) {
if (!result) {
if (options.json) {
process.stdout.write(JSON.stringify({ status: "destroyed", leaseId }) + "\n");
} else {
Expand All @@ -32,9 +33,16 @@ export const repairCmd = new Command("repair")
}

if (options.json) {
process.stdout.write(JSON.stringify(lease, null, 2) + "\n");
process.stdout.write(JSON.stringify(result, null, 2) + "\n");
} else if (isReleaseResult(result)) {
const state = result.status === "released" ? "released" : result.lease.state;
console.error(
pc.green(`🌳 Lease ${result.leaseId} repaired with action ${options.action}. Result: ${result.status} (${state}).`),
);
} else {
console.error(pc.green(`🌳 Lease ${lease.leaseId} repaired with action ${options.action}. New state: ${lease.state}`));
console.error(
pc.green(`🌳 Lease ${result.leaseId} repaired with action ${options.action}. New state: ${result.state}`),
);
}
} catch (err: unknown) {
handleError(err);
Expand Down
8 changes: 6 additions & 2 deletions packages/grove/src/git/worktree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,19 @@ export async function removeWorktree(repoRoot: string, path: string): Promise<vo
await runGit(repoRoot, ["worktree", "remove", "--force", "--", path]);
}

export async function resetWorktree(path: string, branch: string): Promise<void> {
export async function resetWorktree(
path: string,
branch: string,
options?: { cleanIgnored?: boolean },
): Promise<void> {
let repoRoot = path;
try {
repoRoot = await runGit(path, ["rev-parse", "--show-toplevel"]);
} catch {}
const ref = await branchRef(repoRoot, branch);
await runGit(path, ["checkout", "--detach", "--force", ref]);
await runGit(path, ["reset", "--hard", ref]);
await runGit(path, ["clean", "-fd"]);
await runGit(path, options?.cleanIgnored ? ["clean", "-fdx"] : ["clean", "-fd"]);
}

export async function detachWorktree(path: string): Promise<void> {
Expand Down
2 changes: 2 additions & 0 deletions packages/grove/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import { GroveConfigSchema } from "./schemas.js";
import type { GroveConfig } from "./schemas.js";

export { Grove } from "./pool.js";
export { isReleaseResult } from "./types.js";
export type {
AcquiredSlot,
AcquireLeaseOptions,
ReleaseLeaseOptions,
ReleaseResult,
DestroyLeaseOptions,
RepairLeaseOptions,
GroveLease,
Expand Down
Loading