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

Two memberHerdrSocket tests pass or fail depending on where the repo is checked out #225

New Issue
Closed
opened 2026-09-01 10:41:19 +02:00 by ltms · 1 comment
ltms commented 2026-09-01 10:41:19 +02:00
Owner
Copy Link
Copy Source

Hit while verifying PR #223 (#123). Not caused by that PR — the two failing tests belong to already-merged #213 and #219.

What happened

I built a branch in a scratch worktree under /private/tmp/... instead of my usual checkout. Same commit, same machine, same JDK:

[ERROR] Tests run: 1089, Failures: 0, Errors: 2
  HerdrPeerLauncherAllowListWiringTest.memberHerdrSocketWithZshMemberLoginShellPutsTheScrubOutsideJavaIoTmpdir:470
    » UncheckedIO cannot share generated directory /var/folders/.../fleetd-zdotdir-... with group
      'wheel' — the group must exist, and the fleetd operator (dai.ha) must be a member of it
  OpenCodeLauncherTest.memberHerdrSocketWithWorktreeRootAndGroupPutsConfigDirUnderWorktreeRootAndSharesIt:682
    » UncheckedIO cannot share generated directory /var/folders/.../fleetd-opencode-... with group
      'wheel' — ...

The same commit in /Users/dai.ha/LTMS/... gives 1089 tests, 0 failures, BUILD SUCCESS.

Why

OpenCodeLauncherTest#currentUserGroup (and its twin in the allow-list test):

/** The current process's own primary group — resolvable on whatever host runs this test. */
private static String currentUserGroup() throws IOException {
    PosixFileAttributeView view = Files.getFileAttributeView(Path.of("."), PosixFileAttributeView.class);
    assumeTrue(view != null, "this host's filesystem does not support POSIX group ownership");
    return view.readAttributes().group().getName();
}

The comment and the code disagree. This does not read the process's primary group. It reads the group that owns the current working directory, which is wherever Maven happened to be started. Those coincide in a home checkout and diverge elsewhere:

  • /Users/dai.ha/... → group staff → the operator is a member → chgrp succeeds → green.
  • /private/tmp/... → group wheel → the operator is not a member → chgrp refused → error.

The assumeTrue guard covers only "this filesystem has no POSIX groups". It does not cover "this group exists but the test user is not in it", which is the case that actually bites.

How bad

Not currently breaking anything. Gitea CI is green on main at f9d2ee2 (run 311), so its checkout sits somewhere whose group the CI user belongs to. Both tests also pass in a worker's worktree, since .bridged-worktrees is under /Users/dai.ha and carries staff.

So this is latent. It costs nothing until someone builds from /tmp, a CI runner changes its checkout directory, or a new contributor's machine assigns a different group — and then it presents as two failing tests that have nothing to do with the change under test. It cost me one build to work out today, which is the whole argument for fixing it.

What the fix has to get right

Read the process's real primary group, or pick a group the current user is actually in, rather than inferring one from a directory. If neither can be established, assumeTrue out with a message that says so — an environment-dependent skip is honest; an environment-dependent failure is not.

Whatever is chosen, keep the assertion these tests exist for: that the generated directory ends up group-readable and never group-writable. A fix that makes them skip everywhere would be worse than the current fragility, because the #213/#219 permission behaviour would then be untested on every host.

Acceptance

  • Both tests give the same result from a checkout under /private/tmp and from one under the operator's home.
  • The helper's name and comment match what it actually does.
  • The permission assertions still run on a normal developer machine and on CI — verify by running the suite from both locations, not by reasoning about it.

Related: #213 · #219 / PR #221 (where both tests came from) · #185 (parent).

Hit while verifying PR #223 (#123). Not caused by that PR — the two failing tests belong to already-merged #213 and #219. ## What happened I built a branch in a scratch worktree under `/private/tmp/...` instead of my usual checkout. Same commit, same machine, same JDK: ``` [ERROR] Tests run: 1089, Failures: 0, Errors: 2 HerdrPeerLauncherAllowListWiringTest.memberHerdrSocketWithZshMemberLoginShellPutsTheScrubOutsideJavaIoTmpdir:470 » UncheckedIO cannot share generated directory /var/folders/.../fleetd-zdotdir-... with group 'wheel' — the group must exist, and the fleetd operator (dai.ha) must be a member of it OpenCodeLauncherTest.memberHerdrSocketWithWorktreeRootAndGroupPutsConfigDirUnderWorktreeRootAndSharesIt:682 » UncheckedIO cannot share generated directory /var/folders/.../fleetd-opencode-... with group 'wheel' — ... ``` The same commit in `/Users/dai.ha/LTMS/...` gives **1089 tests, 0 failures, BUILD SUCCESS**. ## Why `OpenCodeLauncherTest#currentUserGroup` (and its twin in the allow-list test): ```java /** The current process's own primary group — resolvable on whatever host runs this test. */ private static String currentUserGroup() throws IOException { PosixFileAttributeView view = Files.getFileAttributeView(Path.of("."), PosixFileAttributeView.class); assumeTrue(view != null, "this host's filesystem does not support POSIX group ownership"); return view.readAttributes().group().getName(); } ``` **The comment and the code disagree.** This does not read the process's primary group. It reads the group that owns the *current working directory*, which is wherever Maven happened to be started. Those coincide in a home checkout and diverge elsewhere: - `/Users/dai.ha/...` → group `staff` → the operator is a member → `chgrp` succeeds → green. - `/private/tmp/...` → group `wheel` → the operator is not a member → `chgrp` refused → error. The `assumeTrue` guard covers only "this filesystem has no POSIX groups". It does not cover "this group exists but the test user is not in it", which is the case that actually bites. ## How bad **Not currently breaking anything.** Gitea CI is green on `main` at `f9d2ee2` (run 311), so its checkout sits somewhere whose group the CI user belongs to. Both tests also pass in a worker's worktree, since `.bridged-worktrees` is under `/Users/dai.ha` and carries `staff`. So this is latent. It costs nothing until someone builds from `/tmp`, a CI runner changes its checkout directory, or a new contributor's machine assigns a different group — and then it presents as two failing tests that have nothing to do with the change under test. It cost me one build to work out today, which is the whole argument for fixing it. ## What the fix has to get right Read the process's **real primary group**, or pick a group the current user is actually in, rather than inferring one from a directory. If neither can be established, `assumeTrue` out with a message that says so — an environment-dependent skip is honest; an environment-dependent failure is not. Whatever is chosen, **keep the assertion these tests exist for**: that the generated directory ends up group-readable and never group-writable. A fix that makes them skip everywhere would be worse than the current fragility, because the #213/#219 permission behaviour would then be untested on every host. ## Acceptance - Both tests give the same result from a checkout under `/private/tmp` and from one under the operator's home. - The helper's name and comment match what it actually does. - The permission assertions still run on a normal developer machine and on CI — verify by running the suite from both locations, not by reasoning about it. Related: #213 · #219 / PR #221 (where both tests came from) · #185 (parent).
ltms commented 2026-09-02 03:03:59 +02:00
Author
Owner
Copy Link
Copy Source

Fixed in PR #230, merged to main (39c7ce7). Both helpers now resolve the process's real primary group with id -gn, and assumeTrue skips only when id itself cannot be resolved.

Verified the way the ticket asked — by running from both locations, not by reasoning about it. The first check I ran was wrong and I am recording that, because it is the trap this ticket is about: I copied the tree with rsync -a, which preserves the group, so the copy came out as staff and the operator was still a member. That run proved the build works from a different path, not from a foreign-group path, which is the only condition that matters here.

Re-run with cp -R, so BSD lets the tree inherit wheel from /private/tmp:

mv2 group: wheel
fleetd copy group: wheel
am I in that group? 0
BUILD SUCCESS
Tests run: 1100, Failures: 0, Errors: 0, Skipped: 0

Skipped: 0 is the part worth keeping. The risk with this fix was that it would make the permission assertions skip everywhere, which would be worse than the original fragility — the #213/#219 behaviour would then be untested on every host. It does not: the assertions still run.

A third copy of the broken helper was created while this was being fixed. The #222 worker copied it into ClaudeCodeLauncherTest before #230 landed; that copy is fixed in PR #229. Three test classes now carry the same helper, which is worth collapsing into one shared test utility the next time someone is in these files.

Fixed in PR #230, merged to `main` (39c7ce7). Both helpers now resolve the process's real primary group with `id -gn`, and `assumeTrue` skips only when `id` itself cannot be resolved. Verified the way the ticket asked — by running from both locations, not by reasoning about it. The first check I ran was wrong and I am recording that, because it is the trap this ticket is about: I copied the tree with `rsync -a`, which **preserves** the group, so the copy came out as `staff` and the operator was still a member. That run proved the build works from a different path, not from a foreign-group path, which is the only condition that matters here. Re-run with `cp -R`, so BSD lets the tree inherit `wheel` from `/private/tmp`: ``` mv2 group: wheel fleetd copy group: wheel am I in that group? 0 BUILD SUCCESS Tests run: 1100, Failures: 0, Errors: 0, Skipped: 0 ``` `Skipped: 0` is the part worth keeping. The risk with this fix was that it would make the permission assertions skip everywhere, which would be worse than the original fragility — the #213/#219 behaviour would then be untested on every host. It does not: the assertions still run. A third copy of the broken helper was created while this was being fixed. The #222 worker copied it into `ClaudeCodeLauncherTest` before #230 landed; that copy is fixed in PR #229. Three test classes now carry the same helper, which is worth collapsing into one shared test utility the next time someone is in these files.
ltms closed this issue 2026-09-02 03:04:04 +02:00
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#225
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?