fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

t373: pin the production XDG-excludes seam GitWorktreesTest.seedingGitWorktrees builds #380

Closed
agent wants to merge 0 commits from worker/t373-336973-2 into main
pull from: worker/t373-336973-2
merge into: fleet:main
Conversation 0 Commits - Files Changed -
agent commented 2026-09-09 02:25:14 +02:00
Member
Copy Link
Copy Source

fleetd #373 — pin the unpinned production XDG-excludes seam

What was unpinned

GitWorktrees#previouslyEffectiveExcludesFileContent's XDG-fallback branch reads
XDG_CONFIG_HOME/HOME straight in Java (resolveEnv/resolveHome), never through a git
subprocess. That means fleetd #369's subprocess-level hermetic isolation (GIT_CONFIG_GLOBAL,
GIT_CONFIG_SYSTEM, GIT_TERMINAL_PROMPT) cannot reach it — only the gitEnv constructor seam
(fleetd #362 review finding 2) can. A mutation run during the #372/#369 merge found that seam
unpinned: replacing hermeticGitEnv(tmp) with null in GitWorktreesTest.seedingGitWorktrees
left every test in the class green, with or without a poisoned real XDG_CONFIG_HOME.

What I measured myself (re-counted, not taken from the ticket text)

The ticket text (quoting a comment already in the file) said "58 new GitWorktrees(...)
constructions; 5 go through seedingGitWorktrees." I re-ran the counts on this file as it stood
before my change:

grep -c "new GitWorktrees(" GitWorktreesTest.java   -> 59
grep -n "seedingGitWorktrees(" GitWorktreesTest.java -> 4 actual call sites
  (lines 1565, 1621, 1638, 1673 — the definition line and its own internal
  `new GitWorktrees(...)` are not call sites)

So my count is 59 constructions, 4 through seedingGitWorktrees — not 58/5. The "5" and "53"
numbers were already stale in the file's own comment (a claim never re-run, exactly the pattern
this ticket is about) and the ticket text repeated them. I corrected the "5" in that comment to
"4" and added a note that the class-wide count moves every time a test is added, so it should
never be cited without recounting. I did not audit the "53 other" sub-claim further — out of
scope for the two numbers the ticket asked me to re-verify.

The fix

Added seedingGitWorktreesResolvesTheExcludesFileFallbackInsideItsThrowawayDirectory in
GitWorktreesTest.java. It asserts the property (acceptance criterion 2), not the
constructor argument:

  • Builds a GitWorktrees "for seeding" through the exact same construction every other seeding
    test in the class uses (seedingGitWorktrees(root, memberSkillsSource, gitEnv)), with a
    gitEnv whose XDG_CONFIG_HOME points at a throwaway directory this test pre-populates with
    its own marker git/ignore pattern (cb373-xdg-fallback-marker).
  • Seeds a skill through the real add() path (which is what triggers
    previouslyEffectiveExcludesFileContent), then writes a file matching the marker pattern into
    the resulting worktree.
  • Asserts git status --porcelain is empty — this can only be true if the production code
    actually resolved the fallback against this test's own throwaway directory, not the real
    machine's XDG_CONFIG_HOME/HOME.

No externally-set poisoned environment variable is needed. The marker lives only inside a
directory this test controls; if the gitEnv seam is stripped, production code falls back to the
real environment, which does not carry the marker, so the marker file shows up as untracked and
the assertion fails on any machine.

To let the new test go through the actual seedingGitWorktrees construction (rather than a
separate hand-rolled one, which would not have pinned the real gap — the existing
seedSkillsComposesWithTheXdgDefaultExcludesFileWhenNoneIsConfigured test already builds its own
inline instance and would NOT have caught this mutation), I refactored hermeticGitEnv and
seedingGitWorktrees into two-argument overloads:

  • hermeticGitEnv(Path tmp) now delegates to a new hermeticGitEnvAt(Path xdgConfigHome) that
    takes the directory explicitly.
  • seedingGitWorktrees(Path root, String memberSkillsSource, Path tmp) (the existing 4 call
    sites, unchanged) now delegates to a new seedingGitWorktrees(Path root, String memberSkillsSource, Map<String,String> gitEnv), which is the one actual new GitWorktrees(...)
    construction site — same as before, just shared.

This is test-only; no production code was touched. previouslyEffectiveExcludesFileContent
stays private — the new test never needed to reach it directly, only to observe its effect
through git status.

Proof (acceptance criterion — mutate, run, paste, restore)

Mutated the new test's own construction to strip the seam:

GitWorktrees seeding = seedingGitWorktrees(tmp.resolve("wts"), skillsSource.toString(), (Map<String, String>) null);

Ran just that test:

mvn test -Dtest=GitWorktreesTest#seedingGitWorktreesResolvesTheExcludesFileFallbackInsideItsThrowawayDirectory

Real failure:

[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.343 s <<< FAILURE! -- in dev.ltms.fleet.session.GitWorktreesTest
[ERROR] dev.ltms.fleet.session.GitWorktreesTest.seedingGitWorktreesResolvesTheExcludesFileFallbackInsideItsThrowawayDirectory(Path) -- Time elapsed: 0.320 s <<< FAILURE!
org.opentest4j.AssertionFailedError:
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:
?? cb373-xdg-fallback-marker
 ==> expected: <> but was: <?? cb373-xdg-fallback-marker
>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	...
	at dev.ltms.fleet.session.GitWorktreesTest.seedingGitWorktreesResolvesTheExcludesFileFallbackInsideItsThrowawayDirectory(GitWorktreesTest.java:1863)
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
[INFO] BUILD FAILURE

Restored the line to seedingGitWorktrees(tmp.resolve("wts"), skillsSource.toString(), gitEnv)
and re-ran:

[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.286 s -- in dev.ltms.fleet.session.GitWorktreesTest
[INFO] BUILD SUCCESS

Build

Ran mvn clean install in the worktree, unpiped, and read the full output (2324 lines) —
no tail/head/grep hiding a failure:

[INFO] Tests run: 1440, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
[INFO] Total time:  53.226 s

GitWorktreesTest alone: Tests run: 62, Failures: 0, Errors: 0, Skipped: 0 (61 pre-existing + 1
new).

"Find what else has this shape" (per ticket — reported, NOT fixed)

I looked closely at GitWorktrees.java (this ticket's own file) and did a lighter, non-exhaustive
grep of src/main/java for the same shape (a Java-side System.getenv/System.getProperty
read standing in for an isolation/security property, with no test observing what it actually
resolves to). This was not an exhaustive audit of the whole codebase — treat it as a starting
list, not a closed one:

  • GitWorktrees.java itself: execRedacted/exec(..., redactOutput)'s credential-redaction
    guarantee (never copying failing-command stdout that might carry a URL/token into an exception
    message) IS pinned — execRedactedNeverCopiesFailingCommandOutputIntoTheExceptionMessage drives
    the seam directly. Not a gap.
  • OpenCodeLauncher.java:310 — Path.of(System.getProperty("user.home"), ".local", "share", "opencode") resolves the real OS user's opencode session directory straight from the JVM
    property, with a javadoc note (line ~274-279) already flagging it can resolve to "the wrong
    place" when fleetd's own user.home differs from the member's. I did not check whether any
    test asserts on the resolved path itself (vs. mocking around it) — flagging as a candidate, not
    a confirmed finding.
  • ClaudeCodeLauncher.java:604 — seedTrustDialog's unsetConfigDir branch falls back to
    Path.of(System.getProperty("user.home"), ".claude.json"), i.e. the operator's real home file.
    This one is already loud by design (an IllegalStateException refuses the spawn under
    memberHerdrSocket, and a log.warn fires on every occurrence otherwise per the comment at
    line 607-614), so it doesn't obviously have the "deletes silently, stays green" shape the
    ticket describes — noting it only because it's the same raw-property-read pattern.

None of these three were fixed — reporting only, per the ticket.

Files changed

  • fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java (test-only; no production
    code touched)

Build result (verbatim)

[INFO] Tests run: 1440, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
## fleetd #373 — pin the unpinned production XDG-excludes seam ### What was unpinned `GitWorktrees#previouslyEffectiveExcludesFileContent`'s XDG-fallback branch reads `XDG_CONFIG_HOME`/`HOME` straight in Java (`resolveEnv`/`resolveHome`), never through a `git` subprocess. That means fleetd #369's subprocess-level hermetic isolation (`GIT_CONFIG_GLOBAL`, `GIT_CONFIG_SYSTEM`, `GIT_TERMINAL_PROMPT`) cannot reach it — only the `gitEnv` constructor seam (fleetd #362 review finding 2) can. A mutation run during the #372/#369 merge found that seam unpinned: replacing `hermeticGitEnv(tmp)` with `null` in `GitWorktreesTest.seedingGitWorktrees` left every test in the class green, with or without a poisoned real `XDG_CONFIG_HOME`. ### What I measured myself (re-counted, not taken from the ticket text) The ticket text (quoting a comment already in the file) said "58 `new GitWorktrees(...)` constructions; 5 go through `seedingGitWorktrees`." I re-ran the counts on this file as it stood before my change: ``` grep -c "new GitWorktrees(" GitWorktreesTest.java -> 59 grep -n "seedingGitWorktrees(" GitWorktreesTest.java -> 4 actual call sites (lines 1565, 1621, 1638, 1673 — the definition line and its own internal `new GitWorktrees(...)` are not call sites) ``` So my count is **59 constructions, 4 through `seedingGitWorktrees`** — not 58/5. The "5" and "53" numbers were already stale in the file's own comment (a claim never re-run, exactly the pattern this ticket is about) and the ticket text repeated them. I corrected the "5" in that comment to "4" and added a note that the class-wide count moves every time a test is added, so it should never be cited without recounting. I did not audit the "53 other" sub-claim further — out of scope for the two numbers the ticket asked me to re-verify. ### The fix Added `seedingGitWorktreesResolvesTheExcludesFileFallbackInsideItsThrowawayDirectory` in `GitWorktreesTest.java`. It asserts the **property** (acceptance criterion 2), not the constructor argument: - Builds a `GitWorktrees` "for seeding" through the exact same construction every other seeding test in the class uses (`seedingGitWorktrees(root, memberSkillsSource, gitEnv)`), with a `gitEnv` whose `XDG_CONFIG_HOME` points at a throwaway directory this test pre-populates with its own marker `git/ignore` pattern (`cb373-xdg-fallback-marker`). - Seeds a skill through the real `add()` path (which is what triggers `previouslyEffectiveExcludesFileContent`), then writes a file matching the marker pattern into the resulting worktree. - Asserts `git status --porcelain` is empty — this can only be true if the production code actually resolved the fallback against *this test's own throwaway directory*, not the real machine's `XDG_CONFIG_HOME`/`HOME`. **No externally-set poisoned environment variable is needed.** The marker lives only inside a directory this test controls; if the `gitEnv` seam is stripped, production code falls back to the real environment, which does not carry the marker, so the marker file shows up as untracked and the assertion fails on any machine. To let the new test go through the *actual* `seedingGitWorktrees` construction (rather than a separate hand-rolled one, which would not have pinned the real gap — the existing `seedSkillsComposesWithTheXdgDefaultExcludesFileWhenNoneIsConfigured` test already builds its own inline instance and would NOT have caught this mutation), I refactored `hermeticGitEnv` and `seedingGitWorktrees` into two-argument overloads: - `hermeticGitEnv(Path tmp)` now delegates to a new `hermeticGitEnvAt(Path xdgConfigHome)` that takes the directory explicitly. - `seedingGitWorktrees(Path root, String memberSkillsSource, Path tmp)` (the existing 4 call sites, unchanged) now delegates to a new `seedingGitWorktrees(Path root, String memberSkillsSource, Map<String,String> gitEnv)`, which is the one actual `new GitWorktrees(...)` construction site — same as before, just shared. This is test-only; **no production code was touched.** `previouslyEffectiveExcludesFileContent` stays `private` — the new test never needed to reach it directly, only to observe its effect through `git status`. ### Proof (acceptance criterion — mutate, run, paste, restore) Mutated the new test's own construction to strip the seam: ```java GitWorktrees seeding = seedingGitWorktrees(tmp.resolve("wts"), skillsSource.toString(), (Map<String, String>) null); ``` Ran just that test: ``` mvn test -Dtest=GitWorktreesTest#seedingGitWorktreesResolvesTheExcludesFileFallbackInsideItsThrowawayDirectory ``` Real failure: ``` [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.343 s <<< FAILURE! -- in dev.ltms.fleet.session.GitWorktreesTest [ERROR] dev.ltms.fleet.session.GitWorktreesTest.seedingGitWorktreesResolvesTheExcludesFileFallbackInsideItsThrowawayDirectory(Path) -- Time elapsed: 0.320 s <<< FAILURE! org.opentest4j.AssertionFailedError: 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: ?? cb373-xdg-fallback-marker ==> expected: <> but was: <?? cb373-xdg-fallback-marker > at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151) ... at dev.ltms.fleet.session.GitWorktreesTest.seedingGitWorktreesResolvesTheExcludesFileFallbackInsideItsThrowawayDirectory(GitWorktreesTest.java:1863) [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 [INFO] BUILD FAILURE ``` Restored the line to `seedingGitWorktrees(tmp.resolve("wts"), skillsSource.toString(), gitEnv)` and re-ran: ``` [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.286 s -- in dev.ltms.fleet.session.GitWorktreesTest [INFO] BUILD SUCCESS ``` ### Build Ran `mvn clean install` in the worktree, unpiped, and read the full output (2324 lines) — no `tail`/`head`/`grep` hiding a failure: ``` [INFO] Tests run: 1440, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS [INFO] Total time: 53.226 s ``` `GitWorktreesTest` alone: `Tests run: 62, Failures: 0, Errors: 0, Skipped: 0` (61 pre-existing + 1 new). ### "Find what else has this shape" (per ticket — reported, NOT fixed) I looked closely at `GitWorktrees.java` (this ticket's own file) and did a lighter, non-exhaustive grep of `src/main/java` for the same shape (a Java-side `System.getenv`/`System.getProperty` read standing in for an isolation/security property, with no test observing what it actually resolves to). This was not an exhaustive audit of the whole codebase — treat it as a starting list, not a closed one: - `GitWorktrees.java` itself: `execRedacted`/`exec(..., redactOutput)`'s credential-redaction guarantee (never copying failing-command stdout that might carry a URL/token into an exception message) IS pinned — `execRedactedNeverCopiesFailingCommandOutputIntoTheExceptionMessage` drives the seam directly. Not a gap. - `OpenCodeLauncher.java:310` — `Path.of(System.getProperty("user.home"), ".local", "share", "opencode")` resolves the real OS user's opencode session directory straight from the JVM property, with a javadoc note (line ~274-279) already flagging it can resolve to "the wrong place" when fleetd's own `user.home` differs from the member's. I did not check whether any test asserts on the resolved path itself (vs. mocking around it) — flagging as a candidate, not a confirmed finding. - `ClaudeCodeLauncher.java:604` — `seedTrustDialog`'s `unsetConfigDir` branch falls back to `Path.of(System.getProperty("user.home"), ".claude.json")`, i.e. the operator's real home file. This one is already loud by design (an `IllegalStateException` refuses the spawn under `memberHerdrSocket`, and a `log.warn` fires on every occurrence otherwise per the comment at line 607-614), so it doesn't obviously have the "deletes silently, stays green" shape the ticket describes — noting it only because it's the same raw-property-read pattern. None of these three were fixed — reporting only, per the ticket. ### Files changed - `fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java` (test-only; no production code touched) ### Build result (verbatim) ``` [INFO] Tests run: 1440, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ```
agent added 1 commit 2026-09-09 02:25:14 +02:00
t373: pin the production XDG-excludes seam GitWorktreesTest.seedingGitWorktrees builds
CI / contract (pull_request) Successful in 46s
Details
CI / build (pull_request) Successful in 1m32s
Details
3a004dc1b3 
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 closed this pull request 2026-09-09 02:38:10 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 46s
Details
CI / build (pull_request) Successful in 1m32s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#380
Delete Branch "worker/t373-336973-2"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?