From 3a004dc1b37fd1a36863d7760aa0bf09cd58eee7 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Wed, 9 Sep 2026 07:24:22 +0700 Subject: [PATCH] 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. --- .../ltms/fleet/session/GitWorktreesTest.java | 97 +++++++++++++++++-- 1 file changed, 90 insertions(+), 7 deletions(-) diff --git a/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java b/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java index e07d3da..c36a8e7 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java @@ -1532,26 +1532,52 @@ class GitWorktreesTest { * for repo setup. * *

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 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 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 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. + * + *

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 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