fleetd #134/#148 pt3: make overlayParity's copy and skip-worktree visible #242

Closed
agent wants to merge 0 commits from worker/cb134-148-overlay-visible-c9b986-10 into main
Member

Scope

fleetd #134 and #148 point 3, both in GitWorktrees.overlayParity (fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java).

Defect 1 (#148 pt 3) — silent copy

Every line in overlayParity was log.debug, so at default level nothing was reported. Now it logs one info summary per spawn naming the denominator (every configured candidate), what was copied, and why anything was skipped.

Defect 2 (#134) — invisible skip-worktree neutralisation

A tracked overlay file marked --skip-worktree gave no signal that a later edit in that worktree would be silently ignored by git. Now every such file is named in a separate info line stating the consequence directly.

Decision: log only, no worktree-local marker file. Acceptance criterion 1 requires the worktree to hold exactly the configured overlay set and nothing else — a marker file dropped into the worktree would itself violate that invariant, so I did not add one. The info line is the deliverable.

Exact new log lines

  • Both candidates present: parity overlay: copied 2 of 2 candidates: .env, .envrc
  • One present, one absent: parity overlay: copied 1 of 2 candidates: .env (.envrc absent)
  • Tracked file marked --skip-worktree: parity overlay marked --skip-worktree (cannot be committed from this worktree): .env

Tests added (GitWorktreesTest.java)

All drive overlayParity directly against a real worktree (plain git worktree add, not GitWorktrees#add, to avoid unrelated origin/credential-helper noise):

  1. overlayParityCopiesExactlyTheConfiguredFilesAndNothingElse — criterion 1: only configured candidates land in the worktree.
  2. overlayParityLogsBothCopiedWhenBothCandidatesArePresent — asserts the exact summary line for both present.
  3. overlayParityLogsOneCopiedOneAbsent — asserts the exact summary line for one present/one absent.
  4. overlayParityLogsSkipWorktreeConsequenceForATrackedFile — asserts the exact skip-worktree consequence line, and that the file still shows unmodified in git status after its source content changed.
  5. overlayParityWithNoCandidatesLogsNothing — null and empty overlay lists log nothing.

Build

cd fleetd/ && mvn clean install — full output read, not piped.

Tests run: 1168, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

(Baseline on main was 1163 tests; +5 new tests here, 0 failures.)

Mutation testing (lines changed, not just added)

Each mutation applied to production code, full mvn test -Dtest=GitWorktreesTest run, grep -cE 'COMPILATION ERROR|cannot find symbol' checked as 0 before trusting a red result, then reverted.

# Mutation (production line changed) Compile errors Result
1 overlay.size() → copied.size() as the denominator in the summary log call 0 RED — overlayParityLogsOneCopiedOneAbsent failed
2 Wrapped the skip-worktree if (!neutralized.isEmpty()) block in if (false && ...), disabling it 0 RED — overlayParityLogsSkipWorktreeConsequenceForATrackedFile failed
3 Swapped copied/skipped order in the combined detail string (skipped + " (" + copied + ")" instead of the reverse) 0 RED — overlayParityLogsOneCopiedOneAbsent failed
4 Dropped the " absent" reason suffix (skipped.add(rel) instead of skipped.add(rel + " absent")) 0 RED — overlayParityLogsOneCopiedOneAbsent failed
5 Added a leaked extra-file copy (not-overlaid.txt) alongside the configured candidate 0 RED — overlayParityCopiesExactlyTheConfiguredFilesAndNothingElse failed

All five mutations were reverted after confirming the kill; the final mvn clean install above was run on the clean, reverted tree.

Scope boundary respected

Did not touch FleetConfig.java (the default overlay list is owned by another in-flight unit per the brief). Only GitWorktrees.java and GitWorktreesTest.java changed. Did not touch wiki/ or .mcp.json.

Caveat for review

None outside the stated scope. One thing noticed but explicitly out of scope per the brief: FleetConfig.java:411 still lists .envrc in the default overlay — that's the other in-flight unit's change, not mine.

## Scope fleetd #134 and #148 point 3, both in `GitWorktrees.overlayParity` (fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java). ## Defect 1 (#148 pt 3) — silent copy Every line in `overlayParity` was `log.debug`, so at default level nothing was reported. Now it logs one `info` summary per spawn naming the denominator (every configured candidate), what was copied, and why anything was skipped. ## Defect 2 (#134) — invisible skip-worktree neutralisation A tracked overlay file marked `--skip-worktree` gave no signal that a later edit in that worktree would be silently ignored by git. Now every such file is named in a separate `info` line stating the consequence directly. **Decision: log only, no worktree-local marker file.** Acceptance criterion 1 requires the worktree to hold exactly the configured overlay set and nothing else — a marker file dropped into the worktree would itself violate that invariant, so I did not add one. The `info` line is the deliverable. ## Exact new log lines - Both candidates present: `parity overlay: copied 2 of 2 candidates: .env, .envrc` - One present, one absent: `parity overlay: copied 1 of 2 candidates: .env (.envrc absent)` - Tracked file marked --skip-worktree: `parity overlay marked --skip-worktree (cannot be committed from this worktree): .env` ## Tests added (GitWorktreesTest.java) All drive `overlayParity` directly against a real worktree (plain `git worktree add`, not `GitWorktrees#add`, to avoid unrelated origin/credential-helper noise): 1. `overlayParityCopiesExactlyTheConfiguredFilesAndNothingElse` — criterion 1: only configured candidates land in the worktree. 2. `overlayParityLogsBothCopiedWhenBothCandidatesArePresent` — asserts the exact summary line for both present. 3. `overlayParityLogsOneCopiedOneAbsent` — asserts the exact summary line for one present/one absent. 4. `overlayParityLogsSkipWorktreeConsequenceForATrackedFile` — asserts the exact skip-worktree consequence line, and that the file still shows unmodified in `git status` after its source content changed. 5. `overlayParityWithNoCandidatesLogsNothing` — null and empty overlay lists log nothing. ## Build `cd fleetd/ && mvn clean install` — full output read, not piped. ``` Tests run: 1168, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` (Baseline on main was 1163 tests; +5 new tests here, 0 failures.) ## Mutation testing (lines changed, not just added) Each mutation applied to production code, full `mvn test -Dtest=GitWorktreesTest` run, `grep -cE 'COMPILATION ERROR|cannot find symbol'` checked as 0 before trusting a red result, then reverted. | # | Mutation (production line changed) | Compile errors | Result | |---|---|---|---| | 1 | `overlay.size()` → `copied.size()` as the denominator in the summary log call | 0 | RED — `overlayParityLogsOneCopiedOneAbsent` failed | | 2 | Wrapped the skip-worktree `if (!neutralized.isEmpty())` block in `if (false && ...)`, disabling it | 0 | RED — `overlayParityLogsSkipWorktreeConsequenceForATrackedFile` failed | | 3 | Swapped `copied`/`skipped` order in the combined detail string (`skipped + " (" + copied + ")"` instead of the reverse) | 0 | RED — `overlayParityLogsOneCopiedOneAbsent` failed | | 4 | Dropped the `" absent"` reason suffix (`skipped.add(rel)` instead of `skipped.add(rel + " absent")`) | 0 | RED — `overlayParityLogsOneCopiedOneAbsent` failed | | 5 | Added a leaked extra-file copy (`not-overlaid.txt`) alongside the configured candidate | 0 | RED — `overlayParityCopiesExactlyTheConfiguredFilesAndNothingElse` failed | All five mutations were reverted after confirming the kill; the final `mvn clean install` above was run on the clean, reverted tree. ## Scope boundary respected Did not touch `FleetConfig.java` (the default overlay list is owned by another in-flight unit per the brief). Only `GitWorktrees.java` and `GitWorktreesTest.java` changed. Did not touch `wiki/` or `.mcp.json`. ## Caveat for review None outside the stated scope. One thing noticed but explicitly out of scope per the brief: `FleetConfig.java:411` still lists `.envrc` in the default overlay — that's the other in-flight unit's change, not mine.
agent added 1 commit 2026-09-03 06:10:26 +02:00
fleetd #134/#148 point 3: make overlayParity's copy and skip-worktree visible
CI / contract (pull_request) Successful in 1m15s
CI / build (pull_request) Successful in 1m53s
ef8c97871e
overlayParity logged everything at debug, so at the default level nobody could
tell which overlay files a spawn actually received (#148 pt 3), and a tracked
file marked --skip-worktree gave no warning that it can no longer be edited
from that worktree (#134).

Report the outcome at info: a per-spawn summary naming the denominator (every
configured candidate), what was copied, and why anything was not — plus a
separate line naming every file marked --skip-worktree, stating plainly that
it cannot be committed from this worktree. No worktree-local marker file: the
worktree must hold exactly the configured overlay set and nothing else, so an
extra file would violate that invariant.
Owner

Merged to main in 0d7b4fb.

Verified by the lead. Baseline 1168 tests, 0 failures, 0 compile errors, BUILD SUCCESS, and the merged tree is byte-identical to the tree I built (git diff FETCH_HEAD main empty), so that result carries over to main unchanged.

I ran one mutation the worker did not, and it is the one that matters most here. The whole fix is the log level, so the regression to guard against is somebody quietly putting it back:

Mutation Compile errors Result
summary line log.info → log.debug 0 2 reds — both summary tests
skip-worktree line log.info → log.debug 0 1 red — the consequence test

Both killed, so the fix cannot silently regress to a debug-level log.

Three things this got right that are worth naming:

  • The production diff changes no behaviour. The copy and the --skip-worktree mark are byte-for-byte what they were; only logging is new. That is exactly the right shape for this ticket.
  • The log assertions watch the right stream. The appender is bound to GitWorktrees.class, and the tests raise the level to INFO deliberately, so they would notice a level change rather than passing regardless.
  • The tracked-file test proves the real effect, not just the log: it rewrites the source after checkout and asserts git status still reports the file unmodified.

The decision to log rather than write a worker-readable marker file into the worktree is right, and for the reason given — criterion 1 requires the worktree to hold exactly the configured overlay set, so a marker would violate the fix it documents.

Merged to main in `0d7b4fb`. Verified by the lead. Baseline **1168 tests, 0 failures, 0 compile errors, BUILD SUCCESS**, and the merged tree is byte-identical to the tree I built (`git diff FETCH_HEAD main` empty), so that result carries over to main unchanged. I ran one mutation the worker did not, and it is the one that matters most here. The whole fix **is** the log level, so the regression to guard against is somebody quietly putting it back: | Mutation | Compile errors | Result | |---|---|---| | summary line `log.info` → `log.debug` | 0 | **2 reds** — both summary tests | | skip-worktree line `log.info` → `log.debug` | 0 | **1 red** — the consequence test | Both killed, so the fix cannot silently regress to a debug-level log. Three things this got right that are worth naming: - **The production diff changes no behaviour.** The copy and the `--skip-worktree` mark are byte-for-byte what they were; only logging is new. That is exactly the right shape for this ticket. - **The log assertions watch the right stream.** The appender is bound to `GitWorktrees.class`, and the tests raise the level to `INFO` deliberately, so they would notice a level change rather than passing regardless. - **The tracked-file test proves the real effect**, not just the log: it rewrites the source after checkout and asserts `git status` still reports the file unmodified. The decision to log rather than write a worker-readable marker file into the worktree is right, and for the reason given — criterion 1 requires the worktree to hold exactly the configured overlay set, so a marker would violate the fix it documents.
ltms closed this pull request 2026-09-03 06:14:52 +02:00
Owner

Correction to the record, and it is my error, not the worker's.

My brief told this worker that #134 lived in overlayParity. It does not. #134 is about isolateToolSurface (GitWorktrees.java:430) and WORKTREE_HOSTILE_CONFIGS — .mcp.json, opencode.json, .autoenv — a different method that stubs those files out and marks them --skip-worktree. The two mechanisms share that mark, which is what made them look like one thing to me.

So this PR fully delivers #148 point 3, and it delivers a real improvement to overlayParity's own --skip-worktree marking, but it does not close #134. I have reopened #134 and recorded what is genuinely still open there.

The merge commit message on 0d7b4fb says "#134 + #148 point 3" and is therefore wrong about #134. I am not rewriting pushed history over a commit message; this comment and the #134 thread are the correction.

The worker did exactly what it was briefed to do, and its work stands on its own merits — the verification and the mutation results in my earlier comment are unaffected.

Correction to the record, and it is my error, not the worker's. My brief told this worker that #134 lived in `overlayParity`. It does not. #134 is about `isolateToolSurface` (`GitWorktrees.java:430`) and `WORKTREE_HOSTILE_CONFIGS` — `.mcp.json`, `opencode.json`, `.autoenv` — a **different method** that stubs those files out and marks them `--skip-worktree`. The two mechanisms share that mark, which is what made them look like one thing to me. So this PR fully delivers **#148 point 3**, and it delivers a real improvement to `overlayParity`'s own `--skip-worktree` marking, but it does **not** close #134. I have reopened #134 and recorded what is genuinely still open there. The merge commit message on `0d7b4fb` says "#134 + #148 point 3" and is therefore wrong about #134. I am not rewriting pushed history over a commit message; this comment and the #134 thread are the correction. The worker did exactly what it was briefed to do, and its work stands on its own merits — the verification and the mutation results in my earlier comment are unaffected.
Some checks are pending
CI / contract (pull_request) Successful in 1m15s
CI / build (pull_request) Successful in 1m53s

Pull request closed

Sign in to join this conversation.