diff --git a/bridged/src/main/java/dev/ltms/bridged/Bridged.java b/bridged/src/main/java/dev/ltms/bridged/Bridged.java index 797ad3b..412a61b 100644 --- a/bridged/src/main/java/dev/ltms/bridged/Bridged.java +++ b/bridged/src/main/java/dev/ltms/bridged/Bridged.java @@ -428,10 +428,17 @@ public final class Bridged { // CB-516: releasing a worker must fail whatever send was waiting on it. Without this a // torn-down delegation kept reporting PENDING until the 30-minute async timeout, and never // reached /metrics — the delegation was unresolvable and nothing said so. - sessions.onRelease(terminal -> { - messages.abandon(terminal, "the worker session was released before it replied"); - replyInbox.release(terminal); - primaryRegistry.forgetDelegation(terminal); // CB-532: don't leak the lead binding + sessions.onRelease(detail -> { + // CB-578 stage C, acceptance criterion 10: a failed ticket's detail should tell a lead + // where to re-dispatch onto the same tree, not just that the worker vanished. + String reason = "the worker session was released before it replied"; + if (detail.worktreePath() != null) { + reason += "; worktree=" + detail.worktreePath() + " branch=" + detail.branch() + + " snapshot=" + (detail.snapshotRef() != null ? detail.snapshotRef() : "none"); + } + messages.abandon(detail.terminalId(), reason); + replyInbox.release(detail.terminalId()); + primaryRegistry.forgetDelegation(detail.terminalId()); // CB-532: don't leak the lead binding }); // MCP server face (CB-105): bridge_send/bridge_reply/bridge_status, mounted at /mcp. diff --git a/bridged/src/main/java/dev/ltms/bridged/session/GitWorktrees.java b/bridged/src/main/java/dev/ltms/bridged/session/GitWorktrees.java index 052b0c5..4d9e8bb 100644 --- a/bridged/src/main/java/dev/ltms/bridged/session/GitWorktrees.java +++ b/bridged/src/main/java/dev/ltms/bridged/session/GitWorktrees.java @@ -13,6 +13,8 @@ import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.security.SecureRandom; import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; @@ -217,6 +219,53 @@ public final class GitWorktrees implements Worktrees { return Path.of(out.trim()).toAbsolutePath().normalize().toString(); } + /** + * CB-578 stage C. Stages into a temporary index (never the worktree's real one, which + * the worker may still be writing to), writes that index to a tree, commits the tree on top of + * the worktree's current HEAD, and points {@code refs/wip/} at the result: + * + *
+     * GIT_INDEX_FILE=<temp> git -C worktree add -A
+     * tree=$(GIT_INDEX_FILE=<temp> git -C worktree write-tree)
+     * commit=$(git -C worktree commit-tree $tree -p HEAD -m message)
+     * git -C worktree update-ref refs/wip/branch $commit
+     * 
+ * + * {@code add -A} (never {@code -f}) respects {@code .gitignore} exactly as it would in the real + * index — a gitignored file staying ignored is what keeps secrets and local config out of the + * snapshot's tree. The temporary index file is removed afterwards regardless of outcome. + */ + @Override + public Optional snapshot(String worktreePath, String branch, String message) { + if (!Files.exists(Path.of(worktreePath))) { + log.debug("worktree {} already gone — nothing to snapshot", worktreePath); + return Optional.empty(); + } + Path tempIndex; + try { + tempIndex = Files.createTempFile("bridged-wip-index-", ".tmp"); + Files.delete(tempIndex); // git creates it fresh under GIT_INDEX_FILE; a stale empty + // file at that path is otherwise treated as a corrupt index. + } catch (IOException e) { + throw new WorktreeException("cannot create a temporary index for snapshot: " + e.getMessage(), e); + } + Map indexEnv = Map.of("GIT_INDEX_FILE", tempIndex.toAbsolutePath().toString()); + try { + exec(indexEnv, "git", "-C", worktreePath, "add", "-A"); + String tree = exec(indexEnv, "git", "-C", worktreePath, "write-tree").trim(); + String commit = exec("git", "-C", worktreePath, "commit-tree", tree, "-p", "HEAD", "-m", message).trim(); + exec("git", "-C", worktreePath, "update-ref", "refs/wip/" + branch, commit); + log.info("snapshotted worktree {} to refs/wip/{} commit={}", worktreePath, branch, commit); + return Optional.of(commit); + } finally { + try { + Files.deleteIfExists(tempIndex); + } catch (IOException e) { + log.debug("could not delete temporary snapshot index {}: {}", tempIndex, e.getMessage()); + } + } + } + /** Resolve the directory that will hold per-session worktree checkouts. */ private Path resolveRoot(String repoRoot) { if (configuredRoot != null && !configuredRoot.isBlank()) { @@ -239,11 +288,20 @@ public final class GitWorktrees implements Worktrees { * stdout and stderr (merged by redirectErrorStream). */ private String exec(String... command) { + return exec(Map.of(), command); + } + + /** Same as {@link #exec(String...)}, with extra environment variables set on the child process. */ + private String exec(Map extraEnv, String... command) { String out; int code; Process p; try { - p = new ProcessBuilder(command).redirectErrorStream(true).start(); + ProcessBuilder pb = new ProcessBuilder(command).redirectErrorStream(true); + if (extraEnv != null && !extraEnv.isEmpty()) { + pb.environment().putAll(extraEnv); + } + p = pb.start(); } catch (IOException e) { throw new WorktreeException("failed to start " + command[0] + ": " + e.getMessage(), e); } diff --git a/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java b/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java index ad94c60..2cb7a4a 100644 --- a/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java +++ b/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java @@ -54,8 +54,8 @@ public final class SessionManager implements TurnListener { /** CB-520: notified with a terminalId on every acquire; no-op until wired. */ private final List> acquireListeners = new java.util.concurrent.CopyOnWriteArrayList<>(); - /** CB-516: notified with a terminalId on every release; no-op until wired. */ - private final List> releaseListeners = new java.util.concurrent.CopyOnWriteArrayList<>(); + /** CB-516: notified with a {@link ReleaseDetail} on every release; no-op until wired. */ + private final List> releaseListeners = new java.util.concurrent.CopyOnWriteArrayList<>(); /** Backward-compatible constructor: shared-tree sessions, production git seam. */ public SessionManager(PeerLauncher launcher) { @@ -196,14 +196,16 @@ public final class SessionManager implements TurnListener { private void release(String paneId, ReleaseCause cause) { MemberSession removed = registry.remove(paneId); boolean preserveWorktree = cause == ReleaseCause.SHUTDOWN; + String snapshotRef = null; if (removed != null) { try { memberLifecycle.released(removed.terminalId()); log.debug("releasing session pane={} terminal={} state={} cause={}", removed.paneId(), removed.terminalId(), removed.state(), cause); + boolean dirty = removed.worktree() != null && worktrees.hasUncommitted(removed.worktree()); if (preserveWorktree && removed.worktree() != null) { logPreservedForShutdown(removed); - } else if (removed.worktree() != null && worktrees.hasUncommitted(removed.worktree())) { + } else if (dirty) { // CB-576: a release that would otherwise remove the worktree finds it holding // uncommitted work the bridge cannot see. A worker that ends a turn without // committing (normally because it stopped to ask a question or refused the turn) @@ -214,6 +216,13 @@ public final class SessionManager implements TurnListener { + "the worktree holds uncommitted changes that --force remove would destroy", cause, removed.worktree(), removed.paneId(), removed.terminalId()); } + if (dirty) { + // CB-578 stage C: preserving on disk is not saving — the directory is one + // `worktree remove --force`, or an operator tidying up, away from gone. Commit + // its full state to a ref before the preserve-or-remove decision above can be + // undone by anything else, regardless of why this release fired. + snapshotRef = trySnapshot(removed, cause); + } } catch (RuntimeException e) { // CB-581: hasUncommitted shells out to `git status` and can throw on a non-zero // exit. We can no longer tell whether the worktree holds uncommitted work, so fail @@ -228,8 +237,10 @@ public final class SessionManager implements TurnListener { // CB-516/CB-581: a send still waiting on this worker can never be answered now, no // matter what happened above. Tell the listener BEFORE the pane is torn down, so a // blocked caller fails fast with a real reason instead of sitting on a rendezvous - // nothing will ever resolve. - notifyReleased(removed.terminalId()); + // nothing will ever resolve. CB-578 stage C: carry the worktree/branch/snapshot ref + // too, so a failed ticket's detail can point a lead at the same tree to re-dispatch. + notifyReleased(new ReleaseDetail(removed.terminalId(), removed.worktree(), + removed.branch(), snapshotRef)); } } // CB-581: the pane must always stop, even if the dirty check above threw. A session removed @@ -241,6 +252,51 @@ public final class SessionManager implements TurnListener { } } + /** + * Best-effort snapshot of a dirty worktree into {@code refs/wip/} (CB-578 stage C). A + * failure here must never escalate: the caller has already decided to preserve the worktree + * regardless of whether this succeeds, so the only cost of a failed snapshot is a WARN and a + * missing ref — never a lost pane stop or a lost release notification. + */ + private String trySnapshot(MemberSession session, ReleaseCause cause) { + if (session.worktree() == null || session.branch() == null) { + return null; + } + try { + Optional ref = worktrees.snapshot(session.worktree(), session.branch(), + snapshotMessage(session, cause)); + ref.ifPresent(sha -> log.info( + "snapshotted dirty worktree {} to refs/wip/{} commit={} for pane={} terminal={}", + session.worktree(), session.branch(), sha, session.paneId(), session.terminalId())); + return ref.orElse(null); + } catch (RuntimeException e) { + log.warn("snapshot of dirty worktree {} failed for pane={} terminal={} branch={}: the " + + "worktree is still preserved on disk, just not committed to refs/wip/{}: {}", + session.worktree(), session.paneId(), session.terminalId(), session.branch(), + session.branch(), e.toString()); + return null; + } + } + + /** Commit message for a CB-578 stage C snapshot — names the member so an operator can tell runs apart. */ + private String snapshotMessage(MemberSession session, ReleaseCause cause) { + return "CB-578 stage C: snapshot of a released worker\n\n" + + "terminal: " + session.terminalId() + "\n" + + "profile: " + session.profile() + "\n" + + "branch: " + session.branch() + "\n" + + "cause: " + cause; + } + + /** + * Facts about a released session that a listener needs beyond the bare terminal id — enough + * for a caller to point a lead at where to re-dispatch onto the same tree after a failed + * release (CB-578 stage C, acceptance criterion 10). {@code worktreePath} and {@code branch} + * are {@code null} for a shared-tree session; {@code snapshotRef} is {@code null} unless this + * release snapshotted a dirty worktree into {@code refs/wip/}. + */ + public record ReleaseDetail(String terminalId, String worktreePath, String branch, String snapshotRef) { + } + /** * Why a session is being released — governs whether its worktree is preserved or removed. * Worktree removal is reserved for the one case that is genuinely finished; everything else @@ -289,7 +345,7 @@ public final class SessionManager implements TurnListener { * presence view this manager exposes). Wiring it at construction would require breaking that * cycle for one callback. */ - public void onRelease(Consumer listener) { + public void onRelease(Consumer listener) { if (listener != null) { releaseListeners.add(listener); } @@ -315,15 +371,15 @@ public final class SessionManager implements TurnListener { } /** A listener failure must never prevent the teardown it is reacting to. */ - private void notifyReleased(String terminalId) { - if (terminalId == null) { + private void notifyReleased(ReleaseDetail detail) { + if (detail.terminalId() == null) { return; } - for (Consumer listener : releaseListeners) { + for (Consumer listener : releaseListeners) { try { - listener.accept(terminalId); + listener.accept(detail); } catch (RuntimeException e) { - log.warn("release listener failed for terminal {}: {}", terminalId, e.toString()); + log.warn("release listener failed for terminal {}: {}", detail.terminalId(), e.toString()); } } } diff --git a/bridged/src/main/java/dev/ltms/bridged/session/Worktrees.java b/bridged/src/main/java/dev/ltms/bridged/session/Worktrees.java index dd532ce..d7fb0f0 100644 --- a/bridged/src/main/java/dev/ltms/bridged/session/Worktrees.java +++ b/bridged/src/main/java/dev/ltms/bridged/session/Worktrees.java @@ -1,6 +1,7 @@ package dev.ltms.bridged.session; import java.util.List; +import java.util.Optional; /** Seam between {@link SessionManager} and git worktree operations. Tests use a recording fake. */ public interface Worktrees { @@ -27,4 +28,30 @@ public interface Worktrees { /** git -C rev-parse --show-toplevel — the repo root that owns cwd. */ String repoRoot(String cwd); + + /** + * Commit the worktree's full on-disk state — tracked and untracked, respecting + * {@code .gitignore} — to {@code refs/wip/}, so a release that would otherwise leave + * the work as loose, unprotected files has a durable git object to fall back on (CB-578 stage + * C). Built on a temporary index: the worker's own index, working tree, and HEAD are + * never touched, since the worker may still be mid-write. The commit is parented on the + * worktree's current HEAD. + * + *

Never writes under {@code refs/heads/} — the ref must not appear in {@code git branch}, + * must not be pushed by default, and must not be swept by a later {@code git branch -d}. + * + *

Callers are expected to have already confirmed {@link #hasUncommitted} before reaching + * for this; it always stages and commits whatever {@code git add -A} finds, so calling it on + * a clean worktree still produces a (harmless, tree-identical-to-HEAD) commit rather than + * detecting cleanliness itself. + * + * @param worktreePath absolute path of the worktree to snapshot + * @param branch the worktree's own branch — keys {@code refs/wip/} + * @param message the commit message; should name the member, its branch, and the release + * cause so an operator can tell which run produced it + * @return the created commit's sha, or {@link Optional#empty()} if {@code worktreePath} does + * not exist (mirrors {@link #remove} and {@link #hasUncommitted}'s already-gone + * tolerance — a worktree that is gone holds nothing to snapshot) + */ + Optional snapshot(String worktreePath, String branch, String message); } diff --git a/bridged/src/test/java/dev/ltms/bridged/session/FakeWorktrees.java b/bridged/src/test/java/dev/ltms/bridged/session/FakeWorktrees.java index c9b8d9d..f6be1c4 100644 --- a/bridged/src/test/java/dev/ltms/bridged/session/FakeWorktrees.java +++ b/bridged/src/test/java/dev/ltms/bridged/session/FakeWorktrees.java @@ -2,9 +2,11 @@ package dev.ltms.bridged.session; import java.util.Collections; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicLong; /** Recording fake {@link Worktrees} for CB-301-ext acceptance tests (no live git). */ public final class FakeWorktrees implements Worktrees { @@ -22,13 +24,19 @@ public final class FakeWorktrees implements Worktrees { public record RepoRootCall(String cwd) { } + public record SnapshotCall(String worktreePath, String branch, String message) { + } + private final List addCalls = new CopyOnWriteArrayList<>(); private final List removeCalls = new CopyOnWriteArrayList<>(); private final List overlayCalls = new CopyOnWriteArrayList<>(); private final List repoRootCalls = new CopyOnWriteArrayList<>(); + private final List snapshotCalls = new CopyOnWriteArrayList<>(); private final Set existingPaths = ConcurrentHashMap.newKeySet(); private final Set trackedPaths = ConcurrentHashMap.newKeySet(); + private final AtomicLong snapshotSeq = new AtomicLong(); private volatile RuntimeException addFailure; + private volatile RuntimeException snapshotFailure; private volatile boolean dirty = false; private volatile String repoRoot = "/repo"; private volatile String prefix = "/worktrees"; @@ -68,6 +76,12 @@ public final class FakeWorktrees implements Worktrees { return this; } + /** Make subsequent {@link #snapshot} calls throw (simulates a failing git snapshot). */ + public FakeWorktrees failSnapshot(String message) { + this.snapshotFailure = new WorktreeException(message); + return this; + } + @Override public String add(String repoRoot, String branch, String baseRef) { addCalls.add(new AddCall(repoRoot, branch, baseRef)); @@ -112,6 +126,15 @@ public final class FakeWorktrees implements Worktrees { return repoRoot; } + @Override + public Optional snapshot(String worktreePath, String branch, String message) { + snapshotCalls.add(new SnapshotCall(worktreePath, branch, message)); + if (snapshotFailure != null) { + throw snapshotFailure; + } + return Optional.of("wip" + snapshotSeq.incrementAndGet()); + } + public List addCalls() { return List.copyOf(addCalls); } @@ -139,4 +162,12 @@ public final class FakeWorktrees implements Worktrees { public OverlayCall lastOverlay() { return overlayCalls.isEmpty() ? null : overlayCalls.getLast(); } + + public List snapshotCalls() { + return List.copyOf(snapshotCalls); + } + + public SnapshotCall lastSnapshot() { + return snapshotCalls.isEmpty() ? null : snapshotCalls.getLast(); + } } diff --git a/bridged/src/test/java/dev/ltms/bridged/session/GitWorktreesTest.java b/bridged/src/test/java/dev/ltms/bridged/session/GitWorktreesTest.java index 8dd65aa..a90df7a 100644 --- a/bridged/src/test/java/dev/ltms/bridged/session/GitWorktreesTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/session/GitWorktreesTest.java @@ -6,6 +6,7 @@ import org.junit.jupiter.api.io.TempDir; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; +import java.util.Optional; import java.util.concurrent.TimeUnit; import static org.junit.jupiter.api.Assertions.*; @@ -68,6 +69,43 @@ class GitWorktreesTest { return out; } + /** Every pending change in {@code cwd} — the whole-tree porcelain status, unlike {@link #status}. */ + private static String fullStatus(Path cwd) throws Exception { + Process p = new ProcessBuilder("git", "status", "--porcelain") + .directory(cwd.toFile()).redirectErrorStream(true).start(); + String out = new String(p.getInputStream().readAllBytes()); + assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git status timed out"); + return out; + } + + private static String revParse(Path cwd, String ref) throws Exception { + Process p = new ProcessBuilder("git", "-C", cwd.toString(), "rev-parse", ref) + .redirectErrorStream(true).start(); + String out = new String(p.getInputStream().readAllBytes()).trim(); + assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git rev-parse timed out"); + assertEquals(0, p.exitValue(), "git rev-parse " + ref + " failed:\n" + out); + return out; + } + + /** The recursive file list of a commit's tree — used to check what a snapshot actually committed. */ + private static String lsTree(Path cwd, String ref) throws Exception { + Process p = new ProcessBuilder("git", "-C", cwd.toString(), "ls-tree", "-r", "--name-only", ref) + .redirectErrorStream(true).start(); + String out = new String(p.getInputStream().readAllBytes()); + assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git ls-tree timed out"); + assertEquals(0, p.exitValue(), "git ls-tree " + ref + " failed:\n" + out); + return out; + } + + private static String forEachRef(Path cwd, String pattern) throws Exception { + Process p = new ProcessBuilder("git", "-C", cwd.toString(), "for-each-ref", pattern) + .redirectErrorStream(true).start(); + String out = new String(p.getInputStream().readAllBytes()); + assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git for-each-ref timed out"); + assertEquals(0, p.exitValue(), "git for-each-ref " + pattern + " failed:\n" + out); + return out; + } + /** * The heart of CB-525: a provisioned worktree must not inherit the primary's MCP servers. Without * the isolation step the checked-out {@code .mcp.json} carries them in, and a worker navigating @@ -245,4 +283,97 @@ class GitWorktreesTest { assertEquals("", status(Path.of(wt), "opencode.json"), "opencode.json still shows as modified"); assertEquals("", status(Path.of(wt), ".autoenv"), ".autoenv still shows as modified"); } + + /** + * CB-578 stage C, acceptance criterion 1. A dirty worktree — a tracked edit plus a brand-new + * untracked file, exactly the shape lost in CB-576 — must land in {@code refs/wip/}'s + * tree, and that ref must live outside {@code refs/heads} so it never shows up in + * {@code git branch} or gets swept by a branch cleanup. + */ + @Test + void dirtySnapshotCreatesARefWhoseTreeContainsUntrackedAndTrackedChanges(@TempDir Path tmp) throws Exception { + Path repo = initRepo(tmp.resolve("repo")); + GitWorktrees gitWorktrees = new GitWorktrees(tmp.resolve("wts").toString()); + String branch = "cb-578-c-a"; + String wt = gitWorktrees.add(repo.toString(), branch, "HEAD"); + Files.writeString(Path.of(wt).resolve("untracked.txt"), "draft that was never added\n"); + Files.writeString(Path.of(wt).resolve("README.md"), "edited tracked file\n"); + + Optional ref = gitWorktrees.snapshot(wt, branch, "test snapshot"); + + assertTrue(ref.isPresent(), "a dirty worktree snapshot returns a commit sha"); + String tree = lsTree(repo, "refs/wip/" + branch); + assertTrue(tree.contains("untracked.txt"), "snapshot tree must include the untracked file:\n" + tree); + assertTrue(tree.contains("README.md"), "snapshot tree must include the tracked edit:\n" + tree); + String heads = forEachRef(repo, "refs/heads"); + assertFalse(heads.contains("refs/wip/"), "the snapshot ref must not live under refs/heads:\n" + heads); + } + + /** + * CB-578 stage C, acceptance criterion 2. Building the commit through a temporary + * {@code GIT_INDEX_FILE} must leave the worker's own index, working tree, and HEAD exactly as + * they were — the worker may still be mid-write, and staging into the real index would corrupt + * that. + */ + @Test + void snapshotDoesNotTouchTheWorkersOwnIndexWorkingTreeOrHead(@TempDir Path tmp) throws Exception { + Path repo = initRepo(tmp.resolve("repo")); + GitWorktrees gitWorktrees = new GitWorktrees(tmp.resolve("wts").toString()); + String branch = "cb-578-c-b"; + String wt = gitWorktrees.add(repo.toString(), branch, "HEAD"); + Files.writeString(Path.of(wt).resolve("untracked.txt"), "draft that was never added\n"); + Files.writeString(Path.of(wt).resolve("README.md"), "edited tracked file\n"); + String headBefore = revParse(Path.of(wt), "HEAD"); + String statusBefore = fullStatus(Path.of(wt)); + + gitWorktrees.snapshot(wt, branch, "test snapshot"); + + assertEquals(headBefore, revParse(Path.of(wt), "HEAD"), "snapshot must not move HEAD"); + assertEquals(statusBefore, fullStatus(Path.of(wt)), + "snapshot must not change the worker's own index or working tree status"); + } + + /** + * CB-578 stage C, acceptance criterion 3. {@code add -A} (never {@code -f}) respects + * {@code .gitignore}, and that is the only thing keeping a gitignored file (secrets, local + * config) out of a snapshot commit — an untracked file that is NOT ignored must still be + * included, so this isn't just "untracked files are dropped". + */ + @Test + void gitignoredFileIsExcludedButOtherUntrackedFilesAreNot(@TempDir Path tmp) throws Exception { + Path repo = tmp.resolve("repo"); + Files.createDirectories(repo); + git(repo, "init", "-q", "-b", "main"); + git(repo, "config", "user.email", "test@example.invalid"); + git(repo, "config", "user.name", "Test"); + Files.writeString(repo.resolve(".gitignore"), ".env\n"); + Files.writeString(repo.resolve("README.md"), "seed\n"); + git(repo, "add", ".gitignore", "README.md"); + git(repo, "commit", "-q", "-m", "seed"); + + GitWorktrees gitWorktrees = new GitWorktrees(tmp.resolve("wts").toString()); + String branch = "cb-578-c-c"; + String wt = gitWorktrees.add(repo.toString(), branch, "HEAD"); + Files.writeString(Path.of(wt).resolve(".env"), "SECRET=shh\n"); + Files.writeString(Path.of(wt).resolve("untracked.txt"), "draft that was never added\n"); + + Optional ref = gitWorktrees.snapshot(wt, branch, "test snapshot"); + + assertTrue(ref.isPresent()); + String tree = lsTree(repo, "refs/wip/" + branch); + assertFalse(tree.contains(".env"), "a gitignored file must never enter the snapshot:\n" + tree); + assertTrue(tree.contains("untracked.txt"), + "a non-ignored untracked file must still be included:\n" + tree); + } + + /** CB-578 stage C. A clean worktree still produces a valid, if tree-identical, commit — the caller + * (SessionManager) is the one that decides not to call this on a clean worktree. */ + @Test + void snapshotOfAMissingWorktreeReturnsEmptyWithoutThrowing(@TempDir Path tmp) { + GitWorktrees gitWorktrees = new GitWorktrees(tmp.resolve("wts").toString()); + String gone = tmp.resolve("wts").resolve("does-not-exist").toString(); + + assertTrue(gitWorktrees.snapshot(gone, "some-branch", "msg").isEmpty(), + "a missing worktree has nothing to snapshot, and must not throw"); + } } diff --git a/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java b/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java index 41ea200..ffaa64b 100644 --- a/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java @@ -68,9 +68,12 @@ class SessionManagerTest { */ private static final class RecordingWorktrees implements Worktrees { private final List removeCalls = new java.util.ArrayList<>(); + private final List snapshotCalls = new java.util.ArrayList<>(); private final java.util.Set failRemoveFor = new java.util.HashSet<>(); private volatile boolean dirty = false; private volatile RuntimeException hasUncommittedFailure; + private volatile RuntimeException snapshotFailure; + private final java.util.concurrent.atomic.AtomicLong snapshotSeq = new java.util.concurrent.atomic.AtomicLong(); RecordingWorktrees dirty(boolean dirty) { this.dirty = dirty; @@ -87,6 +90,11 @@ class SessionManagerTest { return this; } + RecordingWorktrees failSnapshotWith(RuntimeException e) { + this.snapshotFailure = e; + return this; + } + @Override public String add(String repoRoot, String branch, String baseRef) { return "/wt/" + branch.replace('/', '_'); @@ -117,9 +125,22 @@ class SessionManagerTest { return "/repo"; } + @Override + public java.util.Optional snapshot(String worktreePath, String branch, String message) { + snapshotCalls.add(worktreePath); + if (snapshotFailure != null) { + throw snapshotFailure; + } + return java.util.Optional.of("wip" + snapshotSeq.incrementAndGet()); + } + List removeCalls() { return List.copyOf(removeCalls); } + + List snapshotCalls() { + return List.copyOf(snapshotCalls); + } } private SessionManager sessionManager(FakeHerdr herdr, LongSupplier clock, int contextCap) { @@ -567,7 +588,7 @@ class SessionManagerTest { FakeHerdr herdr = new FakeHerdr(); SessionManager sessions = sessionManager(herdr); java.util.List released = new java.util.concurrent.CopyOnWriteArrayList<>(); - sessions.onRelease(released::add); + sessions.onRelease(detail -> released.add(detail.terminalId())); MemberSession s = sessions.acquire("ltms-local", null, "/caller", null); sessions.release(s.paneId()); @@ -581,7 +602,7 @@ class SessionManagerTest { FakeHerdr herdr = new FakeHerdr(); SessionManager sessions = sessionManager(herdr); java.util.List released = new java.util.concurrent.CopyOnWriteArrayList<>(); - sessions.onRelease(released::add); + sessions.onRelease(detail -> released.add(detail.terminalId())); sessions.release("w9:p404"); // idempotent teardown of something already gone @@ -661,7 +682,7 @@ class SessionManagerTest { RecordingWorktrees worktrees = new RecordingWorktrees(); SessionManager sessions = sessionManager(herdr, worktrees); java.util.List released = new java.util.concurrent.CopyOnWriteArrayList<>(); - sessions.onRelease(released::add); + sessions.onRelease(detail -> released.add(detail.terminalId())); MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null, new WorktreeRequest("cb-581c", null)); worktrees.failHasUncommittedWith(new WorktreeException("git status exited 128")); diff --git a/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java b/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java index 5fa720b..e3c008d 100644 --- a/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java @@ -362,4 +362,86 @@ class WorktreeSessionManagerTest { "the profile's configured cwd must reach repoRoot, not be ignored"); } + // --- CB-578 stage C: snapshot a dirty worktree into git before the preserve-or-remove decision --- + + @Test + void releaseOfACleanWorktreeNeverSnapshots() { + FakeHerdr herdr = new FakeHerdr(); + FakeWorktrees worktrees = new FakeWorktrees().withRepoRoot("/repo").withPrefix("/wt"); + SessionManager sessions = new SessionManager(workerService(herdr), worktrees); + MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null, + new WorktreeRequest("cb-578c-a", null)); + + sessions.release(s.paneId()); + + assertTrue(worktrees.snapshotCalls().isEmpty(), + "acceptance criterion 4: a clean release must create no snapshot ref or commit"); + assertEquals(1, worktrees.removeCalls().size(), "a clean worktree is still removed as before"); + } + + @Test + void releaseOfADirtyWorktreeSnapshotsBeforePreserving() { + FakeHerdr herdr = new FakeHerdr(); + FakeWorktrees worktrees = new FakeWorktrees().withRepoRoot("/repo").withPrefix("/wt") + .withDirty(true); + SessionManager sessions = new SessionManager(workerService(herdr), worktrees); + MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null, + new WorktreeRequest("cb-578c-b", null)); + + sessions.release(s.paneId()); + + assertEquals(1, worktrees.snapshotCalls().size(), + "acceptance criterion 1: a dirty release snapshots the worktree exactly once"); + FakeWorktrees.SnapshotCall call = worktrees.lastSnapshot(); + assertEquals(s.worktree(), call.worktreePath()); + assertEquals(s.branch(), call.branch()); + assertTrue(worktrees.removeCalls().isEmpty(), "the dirty worktree is still preserved, not removed"); + } + + @Test + void releaseNotifiesTheListenerWithWorktreeBranchAndSnapshotRef() { + FakeHerdr herdr = new FakeHerdr(); + FakeWorktrees worktrees = new FakeWorktrees().withRepoRoot("/repo").withPrefix("/wt") + .withDirty(true); + SessionManager sessions = new SessionManager(workerService(herdr), worktrees); + java.util.List released = new java.util.concurrent.CopyOnWriteArrayList<>(); + sessions.onRelease(released::add); + MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null, + new WorktreeRequest("cb-578c-c", null)); + + sessions.release(s.paneId()); + + assertEquals(1, released.size()); + SessionManager.ReleaseDetail detail = released.getFirst(); + assertEquals(s.terminalId(), detail.terminalId()); + assertEquals(s.worktree(), detail.worktreePath(), + "acceptance criterion 6: a failed ticket's detail must carry the worktree path"); + assertEquals(s.branch(), detail.branch(), + "acceptance criterion 6: a failed ticket's detail must carry the branch"); + assertEquals("wip1", detail.snapshotRef(), + "acceptance criterion 6: a failed ticket's detail must carry the snapshot ref"); + } + + @Test + void aFailingSnapshotStillPreservesTheWorktreeStopsThePaneAndNotifies() { + FakeHerdr herdr = new FakeHerdr(); + FakeWorktrees worktrees = new FakeWorktrees().withRepoRoot("/repo").withPrefix("/wt") + .withDirty(true).failSnapshot("git commit-tree exited 128"); + SessionManager sessions = new SessionManager(workerService(herdr), worktrees); + java.util.List released = new java.util.concurrent.CopyOnWriteArrayList<>(); + sessions.onRelease(released::add); + MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null, + new WorktreeRequest("cb-578c-d", null)); + + assertDoesNotThrow(() -> sessions.release(s.paneId()), + "acceptance criterion 5: a failing snapshot must not propagate out of release()"); + + assertTrue(herdr.called("pane.close"), "acceptance criterion 5: the pane must still stop"); + assertTrue(worktrees.removeCalls().isEmpty(), + "acceptance criterion 5: the worktree must still be preserved when the snapshot fails"); + assertEquals(1, released.size(), "acceptance criterion 5: the listener must still be notified"); + assertNull(released.getFirst().snapshotRef(), + "a failed snapshot leaves no ref to report"); + } + }