t373: pin the production XDG-excludes seam GitWorktreesTest.seedingGitWorktrees builds
fleetd #362 review finding 2 protects GitWorktrees#previouslyEffectiveExcludesFileContent's Java-side XDG_CONFIG_HOME/HOME read (it never goes through a git subprocess, so no GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM isolation reaches it) with a gitEnv constructor seam. A mutation run during the #372/#369 merge found that seam unpinned: stripping hermeticGitEnv(tmp) from seedingGitWorktrees left every test green, poisoned XDG_CONFIG_HOME or not. Adds seedingGitWorktreesResolvesTheExcludesFileFallbackInsideItsThrowawayDirectory, which asserts the property directly (a GitWorktrees built for seeding resolves the fallback inside its own throwaway directory) using a self-contained marker instead of relying on an externally poisoned env var. Refactors hermeticGitEnv/seedingGitWorktrees into two-argument overloads (one taking an explicit XDG_CONFIG_HOME / gitEnv) so the new test can pre-populate the marker before construction while still going through the same production construction every other seeding test uses; no behavior change for the 4 existing call sites.
This commit is contained in:
@@ -1532,26 +1532,52 @@ class GitWorktreesTest {
|
||||
* for repo setup.
|
||||
*
|
||||
* <p>Scope, measured on the fleetd #369 merge and narrower than an earlier version of this
|
||||
* comment claimed: this protects the 5 {@link #seedingGitWorktrees} sites plus — through
|
||||
* comment claimed: this protects the {@link #seedingGitWorktrees} call sites plus — through
|
||||
* {@link #gitProcessBuilder} — every {@code git} subprocess the TEST itself starts. It does
|
||||
* NOT cover the other 53 {@code new GitWorktrees(...)} constructions in this file, which pass
|
||||
* NOT cover the {@code new GitWorktrees(...)} constructions elsewhere in this file that pass
|
||||
* no env override, so a production instance built that way still inherits the JVM's real
|
||||
* environment. Stripping this override from {@code seedingGitWorktrees} leaves the class green
|
||||
* both with and without the poison command above, so that half is currently unpinned.
|
||||
* environment. (Re-measured for fleetd #373, on this file as it stands here: 4 call sites go
|
||||
* through {@link #seedingGitWorktrees(Path, String, Path)} — not 5, an earlier count this
|
||||
* comment and fleetd #373's own ticket text both repeated without re-running it — out of 59
|
||||
* total {@code new GitWorktrees(...)} occurrences, one of which is the shared construction
|
||||
* inside {@link #seedingGitWorktrees(Path, String, Map)} itself. This class-wide count moves
|
||||
* every time a test is added, so treat any number here as a snapshot, not a fact to cite
|
||||
* without recounting.) Stripping the {@code gitEnv} override from a {@link
|
||||
* #seedingGitWorktrees} call site leaves the class green both with and without the poison
|
||||
* command above for that call site's OWN test, so that half was unpinned until fleetd #373
|
||||
* added {@link #seedingGitWorktreesResolvesTheExcludesFileFallbackInsideItsThrowawayDirectory}
|
||||
* below, which asserts the property directly instead of relying on a poisoned real machine.
|
||||
*/
|
||||
private static Map<String, String> hermeticGitEnv(Path tmp) {
|
||||
return hermeticGitEnvAt(tmp.resolve("hermetic-xdg-config-home-" + System.nanoTime()));
|
||||
}
|
||||
|
||||
/** Same isolation as {@link #hermeticGitEnv(Path)}, with an explicit {@code XDG_CONFIG_HOME}
|
||||
* instead of a fresh nanoTime-unique one under {@code tmp} — used by
|
||||
* {@link #seedingGitWorktreesResolvesTheExcludesFileFallbackInsideItsThrowawayDirectory}
|
||||
* (fleetd #373) so it can pre-populate that directory with a marker BEFORE the production
|
||||
* {@link GitWorktrees} instance reads it, something the random per-call name from
|
||||
* {@link #hermeticGitEnv(Path)} makes impossible to predict from outside. */
|
||||
private static Map<String, String> hermeticGitEnvAt(Path xdgConfigHome) {
|
||||
return Map.of(
|
||||
"GIT_CONFIG_GLOBAL", "/dev/null",
|
||||
"GIT_CONFIG_SYSTEM", "/dev/null",
|
||||
"GIT_TERMINAL_PROMPT", "0",
|
||||
"XDG_CONFIG_HOME", tmp.resolve("hermetic-xdg-config-home-" + System.nanoTime()).toString());
|
||||
"XDG_CONFIG_HOME", xdgConfigHome.toString());
|
||||
}
|
||||
|
||||
/** {@link GitWorktrees}'s full test seam, with a {@code memberSkillsSource} and no other
|
||||
* overrides — the shape every seeding test below needs, isolated via {@link #hermeticGitEnv}. */
|
||||
private static GitWorktrees seedingGitWorktrees(Path root, String memberSkillsSource, Path tmp) {
|
||||
return new GitWorktrees(root.toString(), null, _ -> {}, null, null, memberSkillsSource,
|
||||
hermeticGitEnv(tmp));
|
||||
return seedingGitWorktrees(root, memberSkillsSource, hermeticGitEnv(tmp));
|
||||
}
|
||||
|
||||
/** Same shape as {@link #seedingGitWorktrees(Path, String, Path)}, taking an already-built
|
||||
* {@code gitEnv} directly rather than computing one via {@link #hermeticGitEnv(Path)} — lets
|
||||
* fleetd #373's test drive the exact production construction a real member spawn uses, with a
|
||||
* {@code gitEnv} it has already pre-populated a marker into. */
|
||||
private static GitWorktrees seedingGitWorktrees(Path root, String memberSkillsSource, Map<String, String> gitEnv) {
|
||||
return new GitWorktrees(root.toString(), null, _ -> {}, null, null, memberSkillsSource, gitEnv);
|
||||
}
|
||||
|
||||
/** Acceptance criterion 2 (part 1): a worktree with no {@code .claude/} at all gets the skill
|
||||
@@ -1784,6 +1810,63 @@ class GitWorktreesTest {
|
||||
+ "after skill seeding ran — got:\n" + porcelain);
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #373. Pins the production seam that fleetd #362 review finding 2 protects: {@link
|
||||
* GitWorktrees#previouslyEffectiveExcludesFileContent}'s XDG-fallback branch reads {@code
|
||||
* XDG_CONFIG_HOME}/{@code HOME} straight in Java, not through a {@code git} subprocess, so
|
||||
* {@code gitEnv} — the constructor seam every {@link #seedingGitWorktrees} instance in this
|
||||
* class is built with — is the ONLY thing that can isolate it. A mutation run during the
|
||||
* fleetd #372/#369 merge found this unpinned: replacing {@code hermeticGitEnv(tmp)} with
|
||||
* {@code null} in {@link #seedingGitWorktrees(Path, String, Path)} left every test in this
|
||||
* class green — the 56 tests that would fail against a real machine's poisoned {@code
|
||||
* XDG_CONFIG_HOME} were fixed by fleetd #369's subprocess-level isolation, but none of them
|
||||
* looks at what THIS Java-side read resolves, so deleting the override stays invisible.
|
||||
*
|
||||
* <p>This test asserts the PROPERTY, not the constructor argument: a {@link GitWorktrees}
|
||||
* built for seeding — through the very same {@link #seedingGitWorktrees(Path, String, Map)}
|
||||
* construction every other seeding test in this class goes through — must resolve the
|
||||
* excludes-file fallback inside its own throwaway {@code gitEnv}-supplied directory. It needs
|
||||
* NO externally-set poisoned environment variable: the marker pattern below is written ONLY
|
||||
* inside a throwaway {@code XDG_CONFIG_HOME} this test controls directly (bypassing {@link
|
||||
* #hermeticGitEnv(Path)}'s unpredictable nanoTime-named directory, via {@link
|
||||
* #hermeticGitEnvAt}, so the marker can be in place before the production instance ever reads
|
||||
* it), reachable ONLY through the {@code gitEnv} seam. If that seam is stripped, the
|
||||
* production code instead falls back to resolving the REAL {@code XDG_CONFIG_HOME}/{@code
|
||||
* HOME} of the machine running the test — which does not carry this marker — so the marker
|
||||
* file below shows up as untracked and the assertion fails on any machine, with no poison
|
||||
* command required. See the PR body for the pasted failure from actually running that
|
||||
* mutation (removing the {@code gitEnv} override from this test's own construction).
|
||||
*/
|
||||
@Test
|
||||
void seedingGitWorktreesResolvesTheExcludesFileFallbackInsideItsThrowawayDirectory(@TempDir Path tmp)
|
||||
throws Exception {
|
||||
Path xdgConfigHome = tmp.resolve("cb373-xdg-config-home");
|
||||
Files.createDirectories(xdgConfigHome.resolve("git"));
|
||||
Files.writeString(xdgConfigHome.resolve("git").resolve("ignore"), "cb373-xdg-fallback-marker\n");
|
||||
Map<String, String> gitEnv = hermeticGitEnvAt(xdgConfigHome);
|
||||
|
||||
Path repo = initRepo(tmp.resolve("repo"));
|
||||
Path skillsSource = tmp.resolve("skills-src");
|
||||
writeSkill(skillsSource, "implementer", "IMPLEMENTER SKILL\n");
|
||||
GitWorktrees seeding = seedingGitWorktrees(tmp.resolve("wts"), skillsSource.toString(), gitEnv);
|
||||
|
||||
String wt = seeding.add(repo.toString(), "cb-373-xdg-seam", "HEAD");
|
||||
assertEquals("IMPLEMENTER SKILL\n",
|
||||
Files.readString(Path.of(wt, ".claude", "skills", "implementer", "SKILL.md")),
|
||||
"fixture check — the skill really was seeded, so previouslyEffectiveExcludesFileContent ran");
|
||||
|
||||
Files.writeString(Path.of(wt, "cb373-xdg-fallback-marker"),
|
||||
"would only be invisible to git status if the fallback resolved THIS throwaway "
|
||||
+ "XDG_CONFIG_HOME rather than the real machine's\n");
|
||||
|
||||
String porcelain = fullStatus(Path.of(wt));
|
||||
assertEquals("", porcelain,
|
||||
"the marker pattern lives only in this test's throwaway XDG_CONFIG_HOME; git "
|
||||
+ "status must still be empty, proving the production seam resolved the "
|
||||
+ "excludes-file fallback through the gitEnv seam rather than the JVM's "
|
||||
+ "real environment — got:\n" + porcelain);
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #369, acceptance criterion 4 — make the fix hard to undo by accident. Every git
|
||||
* subprocess this class starts is required to go through {@link #gitProcessBuilder}, the one
|
||||
|
||||
Reference in New Issue
Block a user