CB-578 stage C: snapshot a dirty worktree into refs/wip before it can be lost
Adds Worktrees.snapshot(worktreePath, branch, message): stages into a temporary GIT_INDEX_FILE (never the worker's real index/HEAD), writes the tree, commit-trees it onto the worktree's current HEAD, and points refs/wip/<branch> at the result. add -A (never -f) respects .gitignore. SessionManager.release() now snapshots any dirty worktree before the preserve-or-remove decision, regardless of release cause (COMPLETED or SHUTDOWN) — preserving on disk alone is one `worktree remove --force` away from gone. A failing snapshot never escalates: the worktree is still preserved, the pane still stops, and the release listener is still notified. onRelease's listener now receives a ReleaseDetail (terminal, worktree path, branch, snapshot ref) instead of a bare terminal id, so Bridged's WORKER_FAILED wiring can put the same three facts into a failed ticket's detail — a lead can re-dispatch onto the same tree instead of starting from the base commit.
This commit is contained in:
@@ -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<AddCall> addCalls = new CopyOnWriteArrayList<>();
|
||||
private final List<RemoveCall> removeCalls = new CopyOnWriteArrayList<>();
|
||||
private final List<OverlayCall> overlayCalls = new CopyOnWriteArrayList<>();
|
||||
private final List<RepoRootCall> repoRootCalls = new CopyOnWriteArrayList<>();
|
||||
private final List<SnapshotCall> snapshotCalls = new CopyOnWriteArrayList<>();
|
||||
private final Set<String> existingPaths = ConcurrentHashMap.newKeySet();
|
||||
private final Set<String> 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<String> 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<AddCall> 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<SnapshotCall> snapshotCalls() {
|
||||
return List.copyOf(snapshotCalls);
|
||||
}
|
||||
|
||||
public SnapshotCall lastSnapshot() {
|
||||
return snapshotCalls.isEmpty() ? null : snapshotCalls.getLast();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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/<branch>}'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<String> 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<String> 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");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -68,9 +68,12 @@ class SessionManagerTest {
|
||||
*/
|
||||
private static final class RecordingWorktrees implements Worktrees {
|
||||
private final List<String> removeCalls = new java.util.ArrayList<>();
|
||||
private final List<String> snapshotCalls = new java.util.ArrayList<>();
|
||||
private final java.util.Set<String> 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<String> snapshot(String worktreePath, String branch, String message) {
|
||||
snapshotCalls.add(worktreePath);
|
||||
if (snapshotFailure != null) {
|
||||
throw snapshotFailure;
|
||||
}
|
||||
return java.util.Optional.of("wip" + snapshotSeq.incrementAndGet());
|
||||
}
|
||||
|
||||
List<String> removeCalls() {
|
||||
return List.copyOf(removeCalls);
|
||||
}
|
||||
|
||||
List<String> 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<String> 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<String> 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<String> 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"));
|
||||
|
||||
@@ -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<SessionManager.ReleaseDetail> 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<SessionManager.ReleaseDetail> 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");
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user