Both defects lived in one method. overlayParity logged every step at debug, so at the default level the copy was silent and nobody could tell which overlay files a member actually got. It also marked a copied tracked file --skip-worktree and said nothing, so a worker editing that file later found git ignoring the change with no error anywhere. The summary now reports the denominator, not a bare count: 'copied 1 of 2 candidates: .env (.envrc absent)'. A bare 'copied 1' is the same under-reporting shape as #113. Neutralised files are named with the consequence in the message itself. No marker file is written into the worktree: acceptance criterion 1 requires the worktree to hold exactly the configured overlay set, so a marker would violate the fix it documents. The copy and mark logic is unchanged — only logging is new. Verified by the lead: baseline 1168 tests, 0 failures. Reverting either log.info to log.debug goes red (2 reds and 1 red, 0 compile errors each), which is the regression that matters since the whole fix is the log level.
This commit is contained in:
@@ -486,25 +486,46 @@ public final class GitWorktrees implements Worktrees {
|
||||
}
|
||||
Path srcRoot = Path.of(repoRoot).toAbsolutePath().normalize();
|
||||
Path dstRoot = Path.of(worktreePath).toAbsolutePath().normalize();
|
||||
List<String> copied = new ArrayList<>();
|
||||
List<String> skipped = new ArrayList<>();
|
||||
List<String> neutralized = new ArrayList<>();
|
||||
for (String rel : overlay) {
|
||||
Path src = srcRoot.resolve(rel).normalize();
|
||||
if (!Files.exists(src)) {
|
||||
log.debug("parity overlay source missing — skipping {}", rel);
|
||||
skipped.add(rel + " absent");
|
||||
continue;
|
||||
}
|
||||
Path dst = dstRoot.resolve(rel).normalize();
|
||||
try {
|
||||
Files.createDirectories(dst.getParent());
|
||||
Files.copy(src, dst, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES);
|
||||
log.debug("copied parity overlay {}", rel);
|
||||
copied.add(rel);
|
||||
} catch (IOException e) {
|
||||
throw new WorktreeException("cannot copy overlay " + rel + ": " + e.getMessage(), e);
|
||||
}
|
||||
if (isTracked(dstRoot, rel)) {
|
||||
exec("git", "-C", worktreePath, "update-index", "--skip-worktree", rel);
|
||||
log.debug("marked overlay --skip-worktree {}", rel);
|
||||
neutralized.add(rel);
|
||||
}
|
||||
}
|
||||
// CB-148 point 3: a bare count ("copied 1") hides which candidates were even considered — the
|
||||
// same shape of under-reporting this repo has been bitten by before. Name the denominator
|
||||
// (every configured candidate), what was actually copied, and — for anything not copied —
|
||||
// why, so a spawn's overlay outcome is legible from the log alone, no filesystem dig required.
|
||||
String detail = copied.isEmpty() ? String.join(", ", skipped)
|
||||
: skipped.isEmpty() ? String.join(", ", copied)
|
||||
: String.join(", ", copied) + " (" + String.join(", ", skipped) + ")";
|
||||
log.info("parity overlay: copied {} of {} candidates: {}", copied.size(), overlay.size(), detail);
|
||||
// CB-134: a neutralized tracked file is otherwise a silent trap — a worker edits it, git
|
||||
// ignores the change with no error, and nothing anywhere said the file could not be
|
||||
// committed from this worktree. Name every file marked --skip-worktree here, with the
|
||||
// consequence stated in the message itself, rather than adding a worktree-local marker
|
||||
// file: acceptance criterion 1 requires the worktree hold exactly the configured overlay
|
||||
// set and nothing else, so an extra marker would itself violate the fix.
|
||||
if (!neutralized.isEmpty()) {
|
||||
log.info("parity overlay marked --skip-worktree (cannot be committed from this worktree): {}",
|
||||
String.join(", ", neutralized));
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@@ -1177,4 +1177,112 @@ class GitWorktreesTest {
|
||||
+ "the refusal must happen before `git worktree add` ever runs");
|
||||
}
|
||||
}
|
||||
|
||||
// ---- fleetd #134 / #148 point 3: overlayParity must report what it did, and marking a tracked
|
||||
// file --skip-worktree must say the file can no longer be committed from this worktree. Drives
|
||||
// overlayParity directly against a real worktree (git worktree add, no GitWorktrees#add) so these
|
||||
// tests are independent of origin/credential-helper provisioning, which is not under test here. ----
|
||||
|
||||
/** A bare worktree, sibling to {@code repo}, created with plain git — the target overlayParity
|
||||
* copies into. Deliberately not {@link GitWorktrees#add}: that method does unrelated
|
||||
* provisioning (origin rewrite, .mcp.json neutralization, credential helper) that would only
|
||||
* add noise to the log assertions below. */
|
||||
private static Path bareWorktree(Path repo, Path wtDir, String branch) throws Exception {
|
||||
git(repo, "worktree", "add", "-q", wtDir.toString(), "-b", branch, "HEAD");
|
||||
return wtDir;
|
||||
}
|
||||
|
||||
/** Acceptance criterion 1: only the configured candidates land in the worktree — nothing else
|
||||
* from the source tree leaks in alongside them. */
|
||||
@Test
|
||||
void overlayParityCopiesExactlyTheConfiguredFilesAndNothingElse(@TempDir Path tmp) throws Exception {
|
||||
Path repo = initRepo(tmp.resolve("repo"));
|
||||
Files.writeString(repo.resolve(".env"), "A=1\n");
|
||||
Files.writeString(repo.resolve(".envrc"), "export A=1\n");
|
||||
Files.writeString(repo.resolve("not-overlaid.txt"), "must not be copied\n");
|
||||
Path wt = bareWorktree(repo, tmp.resolve("wt"), "cb134-exact");
|
||||
|
||||
new GitWorktrees(tmp.resolve("wts").toString())
|
||||
.overlayParity(repo.toString(), wt.toString(), List.of(".env", ".envrc"));
|
||||
|
||||
assertEquals("A=1\n", Files.readString(wt.resolve(".env")));
|
||||
assertEquals("export A=1\n", Files.readString(wt.resolve(".envrc")));
|
||||
assertFalse(Files.exists(wt.resolve("not-overlaid.txt")),
|
||||
"overlayParity must copy only the configured candidates, not the whole source tree");
|
||||
}
|
||||
|
||||
/** Criterion 2: both candidates present — the summary line names both and the denominator. */
|
||||
@Test
|
||||
void overlayParityLogsBothCopiedWhenBothCandidatesArePresent(@TempDir Path tmp) throws Exception {
|
||||
reportingLogger.setLevel(Level.INFO);
|
||||
Path repo = initRepo(tmp.resolve("repo"));
|
||||
Files.writeString(repo.resolve(".env"), "A=1\n");
|
||||
Files.writeString(repo.resolve(".envrc"), "export A=1\n");
|
||||
Path wt = bareWorktree(repo, tmp.resolve("wt"), "cb134-both");
|
||||
|
||||
new GitWorktrees(tmp.resolve("wts").toString())
|
||||
.overlayParity(repo.toString(), wt.toString(), List.of(".env", ".envrc"));
|
||||
|
||||
assertTrue(capturedMessages().contains("parity overlay: copied 2 of 2 candidates: .env, .envrc"),
|
||||
"expected the both-copied summary line, got:\n" + capturedMessages());
|
||||
}
|
||||
|
||||
/** Criterion 2: one candidate present, one absent — the summary must name the copied file, the
|
||||
* denominator, and why the other candidate was not copied. */
|
||||
@Test
|
||||
void overlayParityLogsOneCopiedOneAbsent(@TempDir Path tmp) throws Exception {
|
||||
reportingLogger.setLevel(Level.INFO);
|
||||
Path repo = initRepo(tmp.resolve("repo"));
|
||||
Files.writeString(repo.resolve(".env"), "A=1\n");
|
||||
// .envrc deliberately not created — the absent candidate.
|
||||
Path wt = bareWorktree(repo, tmp.resolve("wt"), "cb134-partial");
|
||||
|
||||
new GitWorktrees(tmp.resolve("wts").toString())
|
||||
.overlayParity(repo.toString(), wt.toString(), List.of(".env", ".envrc"));
|
||||
|
||||
assertTrue(Files.exists(wt.resolve(".env")));
|
||||
assertFalse(Files.exists(wt.resolve(".envrc")));
|
||||
assertTrue(capturedMessages().contains("parity overlay: copied 1 of 2 candidates: .env (.envrc absent)"),
|
||||
"expected the copied/absent summary line, got:\n" + capturedMessages());
|
||||
}
|
||||
|
||||
/** Criterion 3: a tracked candidate is marked --skip-worktree, and that must be named in the log
|
||||
* with the consequence spelled out — a worker editing it afterward finds git ignoring the
|
||||
* change, silently, unless this line told it so beforehand. */
|
||||
@Test
|
||||
void overlayParityLogsSkipWorktreeConsequenceForATrackedFile(@TempDir Path tmp) throws Exception {
|
||||
reportingLogger.setLevel(Level.INFO);
|
||||
Path repo = initRepo(tmp.resolve("repo"));
|
||||
Files.writeString(repo.resolve(".env"), "A=1\n");
|
||||
git(repo, "add", ".env");
|
||||
git(repo, "commit", "-q", "-m", "track env");
|
||||
Path wt = bareWorktree(repo, tmp.resolve("wt"), "cb134-tracked");
|
||||
// Change the source after the worktree checkout, so the overlay copy actually overwrites it.
|
||||
Files.writeString(repo.resolve(".env"), "A=2\n");
|
||||
|
||||
new GitWorktrees(tmp.resolve("wts").toString())
|
||||
.overlayParity(repo.toString(), wt.toString(), List.of(".env"));
|
||||
|
||||
assertEquals("A=2\n", Files.readString(wt.resolve(".env")));
|
||||
assertEquals("", status(wt, ".env"),
|
||||
"the skip-worktree'd file must not show as modified even though its content changed");
|
||||
assertTrue(capturedMessages().contains(
|
||||
"parity overlay marked --skip-worktree (cannot be committed from this worktree): .env"),
|
||||
"expected the skip-worktree consequence line, got:\n" + capturedMessages());
|
||||
}
|
||||
|
||||
/** Criterion 5: null and empty overlay lists return quietly — no exception, no log noise. */
|
||||
@Test
|
||||
void overlayParityWithNoCandidatesLogsNothing(@TempDir Path tmp) throws Exception {
|
||||
reportingLogger.setLevel(Level.INFO);
|
||||
Path repo = initRepo(tmp.resolve("repo"));
|
||||
Path wt = bareWorktree(repo, tmp.resolve("wt"), "cb134-empty");
|
||||
GitWorktrees worktrees = new GitWorktrees(tmp.resolve("wts").toString());
|
||||
|
||||
worktrees.overlayParity(repo.toString(), wt.toString(), null);
|
||||
worktrees.overlayParity(repo.toString(), wt.toString(), List.of());
|
||||
|
||||
assertTrue(reportingAppender.list.isEmpty(),
|
||||
"a null/empty overlay must log nothing, got:\n" + capturedMessages());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user