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

A directory in parityOverlay is reported as copied and arrives empty — Files.copy does not recurse #478

New Issue
Open
opened 2026-09-10 15:52:38 +02:00 by ltms · 0 comments
ltms commented 2026-09-10 15:52:38 +02:00
Owner
Copy Link
Copy Source

What I measured

GitWorktrees.overlayParity copies each configured candidate with one call:

Files.copy(src, dst, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES);
copied.add(rel);

Files.copy on a directory creates an empty directory at the destination and returns normally. It does not recurse. So a directory candidate lands in the copied list and the spawn log says:

parity overlay: copied 1 of 1 candidates: .claude

The worker then has an empty .claude/ and nothing anywhere says the contents are missing.

Measured on origin/main:

  • fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java:970 — overlayParity, one Files.copy per candidate, no isDirectory branch. grep -c 'isDirectory' in that method's body: 0. Control: grep -c 'log\.' in the file returns 33, so the file was read.
  • FleetConfig.java:521 — the default is List.of(".env"), a single file. So this is not reachable by default.
  • GitWorktreesTest has five overlayParity tests by name: overlayParityCopiesExactlyTheConfiguredFilesAndNothingElse, overlayParityLogsBothCopiedWhenBothCandidatesArePresent, overlayParityLogsOneCopiedOneAbsent, overlayParityLogsSkipWorktreeConsequenceForATrackedFile, overlayParityWithNoCandidatesLogsNothing. None of them uses a directory candidate — no Files.createDirectory appears in any overlay fixture.

Why it is worth fixing even though the default is a file

The path in is an operator writing a directory into parityOverlay:. That is a plausible edit, not a contrived one: the config comment right above the default at FleetConfig.java:515 discusses .claude/settings.local.json and what a worker must not inherit, so .claude is exactly the kind of thing an operator reaches for.

What makes it a real defect rather than a missing feature is the receipt. The log was written specifically so a spawn's overlay outcome is legible without a filesystem dig — its own comment says so, citing CB-148 point 3 and "a bare count hides which candidates were even considered". A directory defeats that: the count is right, the name is right, and the file the worker needs is not there. An operator reading copied 1 of 1 has been told the opposite of the truth.

This is the same shape as #400 and #408: an operation's normal return is not a measurement of its effect.

The honest framing, and a correction of my own

A worker of mine reported this as "the parity overlay logs success regardless of kind". Read literally that is wrong, and I am recording why so the next reader does not repeat it: a genuine copy failure throws WorktreeException, and an absent source is added to skipped and named in the log with " absent". So the failure paths are handled. The word doing the work in that report was kind — file versus directory — and that part is right.

The fleet01 lead separately disproved a different finding from the same worker, about the ZDOTDIR scrub logging as applied without checking the login shell. I verified their disproof in my own tree: HerdrPeerLauncher.java:1409-1422 reads the login shell, calls isZshShell, and throws IllegalArgumentException naming the shell when it is not zsh. That finding is false and I am not filing it. Two of my worker's three beyond-scope notes did not survive checking; this is the one that did.

Scope

  1. Decide what a directory candidate should mean, and make the code and the log agree with the decision. Two defensible answers, and I am not choosing for the implementer:

    • Copy recursively — matches what an operator writing .claude expects, and keeps the receipt honest.
    • Refuse it — throw the way a copy failure already does, with a message naming the candidate and saying parityOverlay takes files. Cheaper, and it follows the precedent at HerdrPeerLauncher.java:1415, where "the control would silently do nothing" was answered with a refusal rather than a degraded fallback.

    Say which you picked and why in the PR body. If you recurse, the log must count what a reader would count — do not report copied 1 for a directory whose contents are the point.

  2. Whatever you pick, a symlink candidate needs an answer in the same pass, because Files.copy follows it by default and COPY_ATTRIBUTES on a link target is a second surprise. One sentence in a comment is enough if you decide it is out of scope, but do not leave it unexamined.

  3. Do not change the default. List.of(".env") stays.

Acceptance criteria

  1. A test with a directory candidate holding at least one file inside it. It must fail against today's code — paste that failure — and pass after your change. A test that passes both before and after pins nothing.
  2. A test on the log line, not only the filesystem. The existing tests already capture the log (overlayParityLogsOneCopiedOneAbsent is the pattern), so assert what a reader is told about a directory. If you chose to refuse, assert the message names the candidate and says why.
  3. State how many overlayParity tests existed before and after, from grep, not from memory.
  4. Whole suite green, with the total, the failure count and the exit code each read from a file, never from a piped tail.
  5. Say plainly whether you covered the symlink case or deliberately left it, and which.

Found by a worker as a beyond-scope note on #393, verified by me before filing.

## What I measured `GitWorktrees.overlayParity` copies each configured candidate with one call: ```java Files.copy(src, dst, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES); copied.add(rel); ``` `Files.copy` on a **directory** creates an empty directory at the destination and returns normally. It does not recurse. So a directory candidate lands in the `copied` list and the spawn log says: ``` parity overlay: copied 1 of 1 candidates: .claude ``` The worker then has an empty `.claude/` and nothing anywhere says the contents are missing. Measured on `origin/main`: - `fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java:970` — `overlayParity`, one `Files.copy` per candidate, no `isDirectory` branch. `grep -c 'isDirectory' ` in that method's body: 0. Control: `grep -c 'log\.'` in the file returns 33, so the file was read. - `FleetConfig.java:521` — the default is `List.of(".env")`, a single file. **So this is not reachable by default.** - `GitWorktreesTest` has five `overlayParity` tests by name: `overlayParityCopiesExactlyTheConfiguredFilesAndNothingElse`, `overlayParityLogsBothCopiedWhenBothCandidatesArePresent`, `overlayParityLogsOneCopiedOneAbsent`, `overlayParityLogsSkipWorktreeConsequenceForATrackedFile`, `overlayParityWithNoCandidatesLogsNothing`. **None of them uses a directory candidate** — no `Files.createDirectory` appears in any overlay fixture. ## Why it is worth fixing even though the default is a file The path in is an operator writing a directory into `parityOverlay:`. That is a plausible edit, not a contrived one: the config comment right above the default at `FleetConfig.java:515` discusses `.claude/settings.local.json` and what a worker must not inherit, so `.claude` is exactly the kind of thing an operator reaches for. What makes it a real defect rather than a missing feature is the **receipt**. The log was written specifically so a spawn's overlay outcome is legible without a filesystem dig — its own comment says so, citing CB-148 point 3 and "a bare count hides which candidates were even considered". A directory defeats that: the count is right, the name is right, and the file the worker needs is not there. An operator reading `copied 1 of 1` has been told the opposite of the truth. This is the same shape as #400 and #408: **an operation's normal return is not a measurement of its effect.** ## The honest framing, and a correction of my own A worker of mine reported this as "the parity overlay logs success regardless of kind". Read literally that is wrong, and I am recording why so the next reader does not repeat it: a genuine copy failure **throws** `WorktreeException`, and an absent source is added to `skipped` and named in the log with `" absent"`. So the failure paths are handled. The word doing the work in that report was **kind** — file versus directory — and that part is right. The fleet01 lead separately disproved a different finding from the same worker, about the ZDOTDIR scrub logging as applied without checking the login shell. I verified their disproof in my own tree: `HerdrPeerLauncher.java:1409-1422` reads the login shell, calls `isZshShell`, and **throws** `IllegalArgumentException` naming the shell when it is not zsh. That finding is false and I am not filing it. Two of my worker's three beyond-scope notes did not survive checking; this is the one that did. ## Scope 1. Decide what a directory candidate should mean, and make the code and the log agree with the decision. Two defensible answers, and I am not choosing for the implementer: - **Copy recursively** — matches what an operator writing `.claude` expects, and keeps the receipt honest. - **Refuse it** — throw the way a copy failure already does, with a message naming the candidate and saying `parityOverlay` takes files. Cheaper, and it follows the precedent at `HerdrPeerLauncher.java:1415`, where "the control would silently do nothing" was answered with a refusal rather than a degraded fallback. Say which you picked and why in the PR body. If you recurse, the log must count what a reader would count — do not report `copied 1` for a directory whose contents are the point. 2. Whatever you pick, a **symlink** candidate needs an answer in the same pass, because `Files.copy` follows it by default and `COPY_ATTRIBUTES` on a link target is a second surprise. One sentence in a comment is enough if you decide it is out of scope, but do not leave it unexamined. 3. Do not change the default. `List.of(".env")` stays. ## Acceptance criteria 1. A test with a **directory** candidate holding at least one file inside it. It must fail against today's code — paste that failure — and pass after your change. A test that passes both before and after pins nothing. 2. A test on the **log line**, not only the filesystem. The existing tests already capture the log (`overlayParityLogsOneCopiedOneAbsent` is the pattern), so assert what a reader is told about a directory. If you chose to refuse, assert the message names the candidate and says why. 3. State how many `overlayParity` tests existed before and after, from `grep`, not from memory. 4. Whole suite green, with the total, the failure count and the exit code each read from a file, never from a piped tail. 5. Say plainly whether you covered the symlink case or deliberately left it, and which. Found by a worker as a beyond-scope note on #393, verified by me before filing.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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#478
Delete Branch "%!s()"

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?