[aslom]

Two pre-existing test bugs in remote-worker, both found while chasing a red proto job on #620. Neither is related to that PR.

1. A data race in internal/guestagent (f040f7a3)

proto fails go test -race ./... with:

Read at ... by goroutine 433:
  testing.(*common).log()
  guestagent.(*EgressForwarder).logf()    egress.go:52
  guestagent.(*EgressForwarder).handle()  egress.go:61
Previous write at ... by goroutine 110:
  testing.tRunner.func1()                testing.go:2023

tRunner.func1 is the test finishing, so a forwarder goroutine is calling t.Logf on a *testing.T that has already returned.

startForwarder passes t.Logf straight to the forwarder. Its t.Cleanup closes the listener, which ends Serve — but Serve spawns each connection with a bare go f.handle(c) (egress.go:46) and nothing joins those goroutines. A handle still mid-request calls f.logf, which on a failed Dial it always does.

The fix gates the logger on the test lifetime: a mutex-guarded flag flipped off by a t.Cleanup registered before the listener's, so cleanup's LIFO order closes the listener first and only then stops logging. The production logf is left alone — it is correct, and falls back to log.Printf when no Logf is set. This was the test handing it a logger with a shorter lifetime than the goroutines using it.

Not reproduced locally, and worth saying plainly: my sandbox forbids Unix socket bind, so the five affected tests cannot run there at all, and the one TCP-only test on the same path passes 20 consecutive -race runs both with and without the change. The fix follows from the stack trace and from go f.handle(c) having no join. CI is the test.

2. Hardcoded /tmp for the socket test directories (ac7bedad)

Fourteen call sites across eleven test files, plus the conformance harness, used os.MkdirTemp("/tmp", ...). The reason was real — a Unix socket path is capped at 104 bytes by sockaddr_un.sun_path on macOS, and Go's default TMPDIR there is long enough to blow it — but hardcoding trades one failure for another: wherever /tmp is not writable, all of them fail at mkdir before testing anything. It also meant the Go tests ignored TMPDIR, the one knob the rest of the repo honours.

Adds internal/shortdir, which tries TMPDIR, then /tmp, then Go's default, and takes the first that is both short enough and writable. A directory that leaves under 40 bytes for a socket name is skipped rather than used, so an over-long TMPDIR costs a fallback instead of an EINVAL from bind that explains nothing. When no candidate works, the failure names every one it tried and why.

New(t, prefix) registers its own cleanup for tests; Make(prefix) is the same logic without the testing dependency, for egress/conformance, which is not a test.

No behaviour change where /tmp is writable: there TMPDIR is usually /tmp or shorter, so the first candidate wins. Verified by the failure mode moving from mkdir /tmp/ectl...: operation not permitted to bind .../c.sock: operation not permitted — the directory is now created, and only the sandbox's socket ban remains, which no path change can address.

Checks

• gofmt -l . clean; go vet ./... clean
• internal/shortdir has its own tests (4; two skip where there is no writable fallback, which is itself the condition the package exists for)
• Every remaining local test failure is this sandbox's socket-bind ban, not a code failure

---

🤖 Assisted-By Claude Code

[ingpaolodettori-dev]

The race fix in startForwarder is correct. The gate's t.Cleanup is registered before the listener's, so LIFO order closes the listener first and only then stops logging. Holding mu across t.Logf means no line can slip in after the flag flips. The shortdir refactor is a good idea, but its length check is applied to the wrong thing, and on macOS it brings back the exact failure the hardcoded /tmp was there to avoid.

must-fix: Make checks len(base)+40 <= 104 on the parent directory. The real socket path is base + "/" + prefix + <up to 10 random digits> + <caller's suffix>. On darwin the default TMPDIR is about 48 bytes (/var/folders/xx/<28 chars>/T). That passes the check and is now preferred over /tmp. collisionLauncher then binds <dir>/firecracker/vm-6/root/run/firecracker.socket: 48+1+7+10+45 ≈ 111 bytes, which is over 104. So occupyJail fails with EINVAL on a default macOS setup, where the old /tmp base worked. That contradicts the PR's "no behaviour change where /tmp is writable". TestNewLeavesRoomForASocketName also applies budget to the returned dir rather than the base, so the package and its own test disagree. With a TMPDIR of about 60 bytes, Make accepts it and the test fails. Suggested fix: check the created dir (len(dir)+need) and let callers pass the suffix length they need (e.g. New(t, prefix, len("/firecracker/vm-6/root/run/firecracker.socket"))). Alternatively, prefer /tmp ahead of a TMPDIR that leaves less room than the caller needs.

CI is all green (linux runners with TMPDIR unset take /tmp, so the issue doesn't show there). Both commits are signed off.

Author: aslom (MEMBER — maintainer)
Areas reviewed: Go (tests + new internal/shortdir package, egress conformance harness)
Agent/IDE config (.claude/.vscode): none
Commits: 2 commits, all signed-off: yes
CI status: passing

[ingpaolodettori-dev]

must-fix: this checks the parent against a fixed 40-byte allowance, but the socket lives at base/<prefix><≤10 digits>/<suffix>, and suffixes in this suite go up to 45 bytes (/firecracker/vm-6/root/run/firecracker.socket, collision test). On darwin's default TMPDIR (about 48 bytes, which passes 48+40≤104) the collision socket comes to about 111 bytes, so bind fails with EINVAL where /tmp used to work. Check the created dir instead (len(dir)+need > MaxSocketPath → remove it and try the next candidate), with need supplied by the caller or at least sized for the longest suffix in the suite.

[ingpaolodettori-dev]

suggestion: this asserts budget against the returned dir, while Make only enforces it against the base, so a TMPDIR of 60–64 bytes passes Make and fails this test. Once the check in Make moves to the created dir (see shortdir.go:53), these two agree. A case with a TMPDIR just under the old threshold would pin it.

[ingpaolodettori-dev]

suggestion: this is the caller that breaks on macOS. The comment above ("a short /tmp path rather than t.TempDir()") is now inaccurate, and the 45-byte /firecracker/<id>/root/run/firecracker.socket suffix doesn't fit in shortdir's 40-byte budget once TMPDIR is long. Pass the needed length, or update the comment along with the shortdir fix.

[ingpaolodettori-dev]

nit: the comment still says "a short-named dir directly under /tmp". It's now TMPDIR-or-/tmp via shortdir. The same stale wording is in socket_wait_unix_test.go ("A short /tmp base") and the shortUnixSocketDir comment in fcapi_test.go.

[ingpaolodettori-dev]

nit: shortdir.New already registers os.RemoveAll as a cleanup, so this one (and its twins at :197, :243, plus proxy_test.go:209, control_test.go:69, egress_test.go:57, sdnotify_test.go:24, and the defers in main_test.go:51 / gates_egress_unix_test.go:91) is now redundant. Harmless, but dropping them makes it clear who owns the cleanup.

[aslom]

Thank you — the must-fix was real, and the arithmetic in it is exactly right. Addressed in 8cbf053b, rebased onto 25154730 (so x/net 0.62.0 from #648 is in). Three commits now.

must-fix — shortdir.go:53, the length check was on the wrong thing

Reproduced before changing anything. The socket path is base + "/" + prefix + <≤10 digits> + <caller's suffix>, and Make measured only base against a flat 40. On a default darwin TMPDIR:

base   /var/folders/xx/<28 chars>/T              46   46+40 = 86  <= 104, so ACCEPTED
dir    <base>/vmpool1234567890                   63
+      /firecracker/vm-6/root/run/firecracker.socket  45
                                                ----
total                                            108  > 104       EINVAL

So occupyJail fails where the hardcoded /tmp worked, and my "no behaviour change where /tmp is writable" claim in ac7bedad was wrong. Your diagnosis of why CI stayed green — Linux runners leave TMPDIR unset and take /tmp — is also right, and is the reason 15 green checks were not evidence of anything here.

Took your suggested fix rather than the alternative. New and Make now take need, and the check moved to the directory that actually exists:

dir, err := os.MkdirTemp(base, prefix)
if err != nil { /* tried = ...; continue */ }
if len(dir)+need > MaxSocketPath {
    _ = os.RemoveAll(dir)
    tried = append(tried, /* dir, its length, the total, and need */)
    continue
}
return dir, nil

Creating first and measuring the real path is what makes this correct: MkdirTemp's random suffix is up to 10 digits and the prefix is the caller's, so no arithmetic on base alone can predict the result.

I did not take the alternative (prefer /tmp ahead of a too-short TMPDIR), because it only moves the threshold — it still has to guess how much room the caller wants in order to decide what "too short" means, and the needs here span 7 bytes (/c.sock) to 45. Ordering cannot fix a missing input. With need supplied, the existing TMPDIR → /tmp → os.TempDir() order gives your alternative's behaviour as a consequence: a TMPDIR without room for this caller is skipped and /tmp is used.

All 15 call sites pass len() of the literal suffix they append, so the number cannot drift from the path the test binds:

base := shortdir.New(t, "fcguard", len("/firecracker/vm-6/root/run/firecracker.socket"))
dir  := shortdir.New(t, "ectl", len("/vm1.sock"))

The error message when nothing fits now names the created directory, its length, the total with the caller's suffix, and need, so the next person gets the sum rather than having to redo it.

suggestion — shortdir_test.go:58, the test disagreed with the package

Correct, and it was the sharper half of the finding: TestNewLeavesRoomForASocketName asserted budget against the returned dir while Make enforced it against the base, so the two could not both be right. Now that Make checks the created dir, they assert the same thing, and the test loops over both a 7-byte and a 45-byte need.

Added the case you asked for — a TMPDIR sized to sit just under the old threshold. TestNewFallsBackForABigNeedWhereASmallOneFits builds one base and asks for two needs: the small one must come from TMPDIR (proving the directory is usable, so the test is not just measuring an unusable path) and the large one must fall back. The base-only check could not pass that, because it never saw need at all.

Both new tests were run against the old implementation and both fail — one accepts a TMPDIR with no room for its caller, the other accepts an impossible 105-byte need — so neither passes vacuously.

suggestion — launcher_firecracker_collision_unix_test.go:30, inaccurate comment

Fixed, and it now records the 45-byte figure and the fact that the suffix length is passed to shortdir.New, so the next reader does not have to re-derive why this call site is the tight one.

nit — stale "/tmp" wording

All four fixed: listen_test.go, socket_wait_unix_test.go, fcapi_test.go's shortUnixSocketDir, and the newHarness comment in egress/conformance/conformance.go, which had the same problem.

nit — redundant os.RemoveAll cleanups

Dropped all nine you listed, so shortdir.New's own t.Cleanup is the single owner. That orphaned the os import in sdnotify_test.go and guestagent/egress_test.go; both removed.

I left the defers that are not on a shortdir directory alone — e.g. main_test.go's cmd.Process.Kill and the listener closes — since those still own something.

Verification, and what I could not verify here

Stated plainly, since the last round's "CI is the test" turned out to matter:

• go vet ./... and gofmt clean; golangci-lint run ./... reports nothing in internal/shortdir (the 68 issues it finds are pre-existing and it is not wired into CI or pre-commit).
• Full remote-worker go test ./... A/B against main, back to back: main fails 110, this branch 103 — a strict subset, the 7 being tests main cannot run at all because they hardcode /tmp. No new failures.
• Every failure in both runs is bind: operation not permitted — my sandbox forbids bind(2), so those tests die before asserting. That is also why I verified the fix in a scratch copy of the package with the /tmp candidate pointed at a writable short path: it is the only way to exercise the fallback branch here, and it is how I confirmed the two new tests fail without the fix (6 pass with it, 2 fail without).
• I cannot run the darwin bind that the must-fix is ultimately about, so the EINVAL itself is reasoned from the byte counts above, not observed. The path arithmetic is checkable by hand and the shortdir unit tests now pin the decision that precedes the bind.

Worth noting for the record: this is the second finding on this PR that CI could not have caught, both for the same structural reason — the Linux runners take /tmp, so the darwin path length is never exercised. Happy to add a job that runs these with a long TMPDIR if you think it is worth the minutes; it would have caught this one.

[ingpaolodettori-dev]

Follow-up review @ 8cbf053. The rebase is clean (range-diff = for the two earlier commits), so only 8cbf053 was reviewed.

All items from the f040f7a review are addressed:

• must-fix (shortdir.go:53): Make now measures the created directory against the caller's need and falls back if it doesn't fit. All 15 call sites pass len() of their suffix, so darwin's default TMPDIR now falls back to /tmp for the 45-byte collision socket.
• test/package disagreement: fixed. The new regression test and the error-path test both look good.
• stale comments / redundant os.RemoveAll cleanups: fixed everywhere, including conformance.go.

One small suggestion inline: an off-by-one at exactly 104 bytes on darwin (the NUL terminator).

On the red proto job: I couldn't read the log, but gofmt and vet are clean, and an isolated go test -race ./... at this head passed every package. The only failure I could reproduce is a pre-existing flake in internal/egress TestCapRefusalsAreBoundedPerVM ("no refusal was ever in flight"). It fails about 1 run in 5 under -count=50 -race both here and at the parent commit. The test samples capRefusalsInFlight right after each write without waiting for the handler. A re-run should clear it; it's probably worth its own issue.

[ingpaolodettori-dev]

suggestion: off by one on darwin. sun_path there is 104 bytes including the NUL, and Go's syscall (SockaddrUnix.sockaddr in syscall_bsd.go) rejects len(name) >= 104 with EINVAL, so the longest bindable path is 103. > MaxSocketPath accepts exactly 104. Use >= (or make the constant the usable length, 103, and say so in its comment). TestNewFallsBackForABigNeedWhereASmallOneFits sizes its base so the small-need dir can come to exactly 104 and expects it accepted, so its arithmetic needs the same -1.

[aslom]

Thank you — the must-fix was real, and the arithmetic in it is exactly right. Addressed in 8cbf053b, with two follow-ups (31ccf033, 91cc8db4) that came out of chasing it properly. Rebased onto 25154730, so x/net 0.62.0 from #648 is in. Five commits.

must-fix — shortdir.go:53, the length check was on the wrong thing

Reproduced before changing anything. The socket path is base + "/" + prefix + <≤10 digits> + <caller's suffix>, and Make measured only base against a flat 40. On a default darwin TMPDIR:

base   /var/folders/xx/<28 chars>/T              46   46+40 = 86  <= 104, so ACCEPTED
dir    <base>/vmpool1234567890                   63
+      /firecracker/vm-6/root/run/firecracker.socket  45
                                                ----
total                                            108  > 104       EINVAL

So occupyJail fails where the hardcoded /tmp worked, and my "no behaviour change where /tmp is writable" claim in ac7bedad was wrong. Your diagnosis of why CI stayed green — Linux runners leave TMPDIR unset and take /tmp — is also right, and is the reason 15 green checks were not evidence of anything here.

Took your suggested fix rather than the alternative. New and Make now take need, and the check moved to the directory that actually exists:

dir, err := os.MkdirTemp(base, prefix)
if err != nil { /* tried = ...; continue */ }
if len(dir)+need > MaxSocketPath {
    _ = os.RemoveAll(dir)
    tried = append(tried, /* dir, its length, the total, and need */)
    continue
}
return dir, nil

Creating first and measuring the real path is what makes this correct: MkdirTemp's random suffix is up to 10 digits (measured, not assumed — 3000 calls, max 10) and the prefix is the caller's, so no arithmetic on base alone can predict the result.

I did not take the alternative (prefer /tmp ahead of a too-short TMPDIR), because it only moves the threshold — it still has to guess how much room the caller wants in order to decide what "too short" means, and the needs here span 7 bytes (/c.sock) to 45. Ordering cannot fix a missing input. With need supplied, the existing TMPDIR → /tmp → os.TempDir() order gives your alternative's behaviour as a consequence: a TMPDIR without room for this caller is skipped and /tmp is used.

The error message when nothing fits now names the created directory, its length, the total with the caller's suffix, and need, so the next person gets the sum rather than having to redo it.

The part of this I got wrong four times — 91cc8db4

Worth reporting in full, because it says something about the shape of the fix rather than just about me.

Having added need, I then had to supply it at 15 call sites. Four of them were undersized, and I found them one at a time, each after claiming the audit was complete:

call site	declared	longest socket actually bound	short by
internal/egress/control	len("/vm1.sock") = 9	vm-doomed.sock → 15	6
cmd/moca-egress	len("/c.sock") = 7	vm.sock → 8	1
internal/egress	len("/vm-1.sock") = 10	vm-quiet.sock → 14	4
egress/conformance	len("/c.sock") = 7	vm-a.sock (cases.go) → 10	3

All four bind real listeners. None of them fails on a short base — on CI's /tmp they come to 28–34 bytes against the 104 limit — so nothing catches them until someone runs with a long TMPDIR, which is the exact case this package exists to handle.

The internal/egress one is the instructive case: that need was correct when written. It became wrong when #641 added vm-quiet/vm-noisy, long after. need lives at the call site; the names bound under it live hundreds of lines away, often in another file, and grow when someone adds a test. That is not a thing to re-audit by hand, so 91cc8db4 adds internal/shortdir/callers_test.go, which reads the module's own source, finds every shortdir.New/Make, and checks its need against the longest socket name that scope binds.

Two design notes, since the test has judgement in it:

• Scoping is per-file with a package fallback. Both shapes occur: a directory used by one test is bounded by that test (cmd/moca-egress's notify dir only ever holds /notify, and charging it the package maximum was a false positive), while a helper in one file with names in another needs the whole package — as does a file joining a variable id, which is how conformance.go reaches vm-a via id+".sock".
• Socket detection is a deliberately narrow name heuristic (.sock, .socket, v.sock_<port>, plus the one plain notify), not dataflow. These names reach bind through locals, helper parameters and shell scripts, so following them properly wants a type-checked SSA pass rather than a regexp. The two errors are asymmetric: a missed socket costs one call site's coverage, while a regular file mistaken for a socket fails correct code — which is how the first draft flagged vmpool's jailer-was-spawned marker file.

Verified against all four: each reverted individually makes the test fail; the clean tree passes. It also floors at 14 call sites, so a refactor that breaks the scan fails loudly instead of passing vacuously — the first draft silently walked ".." from internal/shortdir, which is internal/, and missed cmd/moca-egress entirely while reporting a confident "12 sites checked".

suggestion — shortdir_test.go:58, the test disagreed with the package

Correct, and it was the sharper half of the finding: TestNewLeavesRoomForASocketName asserted budget against the returned dir while Make enforced it against the base, so the two could not both be right. Now that Make checks the created dir, they assert the same thing, and the test loops over both a 7-byte and a 45-byte need.

Added the case you asked for — a TMPDIR sized to sit just under the old threshold. TestNewFallsBackForABigNeedWhereASmallOneFits builds one base and asks for two needs: the small one must come from TMPDIR (proving the directory is usable, so the test is not just measuring an unusable path) and the large one must fall back. The base-only check could not pass that, because it never saw need at all.

Both new tests were run against the old implementation and both fail — one accepts a TMPDIR with no room for its caller, the other accepts an impossible 105-byte need — so neither passes vacuously.

suggestion — launcher_firecracker_collision_unix_test.go:30, inaccurate comment

Fixed, and it now records the 45-byte figure and the fact that the suffix length is passed to shortdir.New, so the next reader does not have to re-derive why this call site is the tight one.

nit — stale "/tmp" wording

All four fixed: listen_test.go, socket_wait_unix_test.go, fcapi_test.go's shortUnixSocketDir, and the newHarness comment in egress/conformance/conformance.go, which had the same problem.

nit — redundant os.RemoveAll cleanups

Dropped all nine you listed, so shortdir.New's own t.Cleanup is the single owner. That orphaned the os import in sdnotify_test.go and guestagent/egress_test.go; both removed.

I checked the teardown ordering this changes, since it is not obviously neutral: the defers I removed ran before any t.Cleanup, whereas shortdir.New's runs after. The new order removes the directory strictly after the listeners close, which is the safer of the two. I left alone the defers that own something other than the directory (main_test.go's cmd.Process.Kill, the srv.Closes).

The red proto on 8cbf053b is a pre-existing flake, not this PR

Flagging this since it will be sitting red on the PR. The failure is:

--- FAIL: TestCapRefusalsAreBoundedPerVM (0.00s)
    proxy_test.go:527: no refusal was ever in flight: the test did not exercise the refusal path
FAIL	github.com/rossoctl/moca/remote-worker/internal/egress	48.915s

Every package this PR touches passed in that same run, internal/shortdir included.

The same test is failing on main. Run 40708539112 at c916d731 (the #646 merge, which is not in this branch's history) fails identically, at proxy_test.go:530 — same assertion, different line because main has moved:

--- FAIL: TestCapRefusalsAreBoundedPerVM (0.00s)
    proxy_test.go:530: no refusal was ever in flight: the test did not exercise the refusal path

The test arrived in 87cc3fe9 (#641) and this branch does not touch it — this branch's entire diff to proxy_test.go is the two lines of newHarness that call shortdir.New (the need argument, and the redundant cleanup removed). The mechanism looks timing-dependent by construction: it writes CONNECT and then immediately polls capRefusalsInFlight, so peak == 0 means the refusal completed between the write and the first poll on a slow runner, rather than the bound being violated. The (0.00s) is consistent with that — it never got as far as measuring anything.

Happy to open a separate issue, or to take a fix in this PR if you would rather not have it bouncing: polling with a deadline, or holding the refusals open so they cannot complete early, would both close the window. I have left it alone for now since it is #641's test and not this PR's subject.

Verification, and what I could not verify here

Stated plainly, since the last round's "CI is the test" turned out to matter:

• gofmt and go vet ./... clean, checked on linux/amd64 in a golang:1.26-alpine container (Go 1.26.4, the version go.work pins) as well as on darwin — the host here is arm64 darwin, so a darwin-only check was not evidence for CI.
• internal/shortdir (all 6 tests), egress/conformance and internal/egress pass under -race on that Linux container at this head.
• A/B under -race on Linux, this commit's parent (0ebc871b, same rebased base) against the branch: internal/vmpool ok/ok, internal/egress ok/ok, internal/egress/control ok/ok. No regression from these commits.
• The shortdir tests were also run under CI's own conditions (TMPDIR unset, /tmp writable): all 6 pass, none skipped.
• I cannot run bind(2) in my sandbox, so the darwin EINVAL the must-fix is ultimately about is reasoned from the byte counts above, not observed. The path arithmetic is checkable by hand, and the shortdir unit tests now pin the decision that precedes the bind. The fallback branch was exercised in a scratch copy of the package with the /tmp candidate pointed at a writable short path — the only way to reach it here.

One structural note for the record: both findings on this PR were things CI could not catch, for the same reason — the Linux runners take /tmp, so neither the darwin path length nor an undersized need is ever exercised. callers_test.go closes the second of those on every platform. For the first, a job running remote-worker's tests with a long TMPDIR would have caught this must-fix; happy to add one if you think it earns the minutes.

[aslom]

Thank you — the must-fix was real, and the arithmetic in it is exactly right. Addressed in 8cbf053b, with two follow-ups (31ccf033, 91cc8db4) that came out of chasing it properly. Rebased onto 25154730, so x/net 0.62.0 from #648 is in. Five commits.

must-fix — shortdir.go:53, the length check was on the wrong thing

Reproduced before changing anything. The socket path is base + "/" + prefix + <≤10 digits> + <caller's suffix>, and Make measured only base against a flat 40. On a default darwin TMPDIR:

base   /var/folders/xx/<28 chars>/T              46   46+40 = 86  <= 104, so ACCEPTED
dir    <base>/vmpool1234567890                   63
+      /firecracker/vm-6/root/run/firecracker.socket  45
                                                ----
total                                            108  > 104       EINVAL

So occupyJail fails where the hardcoded /tmp worked, and my "no behaviour change where /tmp is writable" claim in ac7bedad was wrong. Your diagnosis of why CI stayed green — Linux runners leave TMPDIR unset and take /tmp — is also right, and is the reason 15 green checks were not evidence of anything here.

Took your suggested fix rather than the alternative. New and Make now take need, and the check moved to the directory that actually exists:

dir, err := os.MkdirTemp(base, prefix)
if err != nil { /* tried = ...; continue */ }
if len(dir)+need > MaxSocketPath {
    _ = os.RemoveAll(dir)
    tried = append(tried, /* dir, its length, the total, and need */)
    continue
}
return dir, nil

Creating first and measuring the real path is what makes this correct: MkdirTemp's random suffix is up to 10 digits (measured, not assumed — 3000 calls, max 10) and the prefix is the caller's, so no arithmetic on base alone can predict the result.

I did not take the alternative (prefer /tmp ahead of a too-short TMPDIR), because it only moves the threshold — it still has to guess how much room the caller wants in order to decide what "too short" means, and the needs here span 7 bytes (/c.sock) to 45. Ordering cannot fix a missing input. With need supplied, the existing TMPDIR → /tmp → os.TempDir() order gives your alternative's behaviour as a consequence: a TMPDIR without room for this caller is skipped and /tmp is used.

The error message when nothing fits now names the created directory, its length, the total with the caller's suffix, and need, so the next person gets the sum rather than having to redo it.

The part of this I got wrong four times — 91cc8db4

Worth reporting in full, because it says something about the shape of the fix rather than just about me.

Having added need, I then had to supply it at 15 call sites. Four of them were undersized, and I found them one at a time, each after claiming the audit was complete:

call site	declared	longest socket actually bound	short by
internal/egress/control	len("/vm1.sock") = 9	vm-doomed.sock → 15	6
cmd/moca-egress	len("/c.sock") = 7	vm.sock → 8	1
internal/egress	len("/vm-1.sock") = 10	vm-quiet.sock → 14	4
egress/conformance	len("/c.sock") = 7	vm-a.sock (cases.go) → 10	3

All four bind real listeners. None of them fails on a short base — on CI's /tmp they come to 28–34 bytes against the 104 limit — so nothing catches them until someone runs with a long TMPDIR, which is the exact case this package exists to handle.

The internal/egress one is the instructive case: that need was correct when written. It became wrong when #641 added vm-quiet/vm-noisy, long after. need lives at the call site; the names bound under it live hundreds of lines away, often in another file, and grow when someone adds a test. That is not a thing to re-audit by hand, so 91cc8db4 adds internal/shortdir/callers_test.go, which reads the module's own source, finds every shortdir.New/Make, and checks its need against the longest socket name that scope binds.

Two design notes, since the test has judgement in it:

• Scoping is per-file with a package fallback. Both shapes occur: a directory used by one test is bounded by that test (cmd/moca-egress's notify dir only ever holds /notify, and charging it the package maximum was a false positive), while a helper in one file with names in another needs the whole package — as does a file joining a variable id, which is how conformance.go reaches vm-a via id+".sock".
• Socket detection is a deliberately narrow name heuristic (.sock, .socket, v.sock_<port>, plus the one plain notify), not dataflow. These names reach bind through locals, helper parameters and shell scripts, so following them properly wants a type-checked SSA pass rather than a regexp. The two errors are asymmetric: a missed socket costs one call site's coverage, while a regular file mistaken for a socket fails correct code — which is how the first draft flagged vmpool's jailer-was-spawned marker file.

Verified against all four: each reverted individually makes the test fail; the clean tree passes. It also floors at 14 call sites, so a refactor that breaks the scan fails loudly instead of passing vacuously — the first draft silently walked ".." from internal/shortdir, which is internal/, and missed cmd/moca-egress entirely while reporting a confident "12 sites checked".

suggestion — shortdir_test.go:58, the test disagreed with the package

Correct, and it was the sharper half of the finding: TestNewLeavesRoomForASocketName asserted budget against the returned dir while Make enforced it against the base, so the two could not both be right. Now that Make checks the created dir, they assert the same thing, and the test loops over both a 7-byte and a 45-byte need.

Added the case you asked for — a TMPDIR sized to sit just under the old threshold. TestNewFallsBackForABigNeedWhereASmallOneFits builds one base and asks for two needs: the small one must come from TMPDIR (proving the directory is usable, so the test is not just measuring an unusable path) and the large one must fall back. The base-only check could not pass that, because it never saw need at all.

Both new tests were run against the old implementation and both fail — one accepts a TMPDIR with no room for its caller, the other accepts an impossible 105-byte need — so neither passes vacuously.

suggestion — launcher_firecracker_collision_unix_test.go:30, inaccurate comment

Fixed, and it now records the 45-byte figure and the fact that the suffix length is passed to shortdir.New, so the next reader does not have to re-derive why this call site is the tight one.

nit — stale "/tmp" wording

All four fixed: listen_test.go, socket_wait_unix_test.go, fcapi_test.go's shortUnixSocketDir, and the newHarness comment in egress/conformance/conformance.go, which had the same problem.

nit — redundant os.RemoveAll cleanups

Dropped all nine you listed, so shortdir.New's own t.Cleanup is the single owner. That orphaned the os import in sdnotify_test.go and guestagent/egress_test.go; both removed.

I checked the teardown ordering this changes, since it is not obviously neutral: the defers I removed ran before any t.Cleanup, whereas shortdir.New's runs after. The new order removes the directory strictly after the listeners close, which is the safer of the two. I left alone the defers that own something other than the directory (main_test.go's cmd.Process.Kill, the srv.Closes).

The red proto on 8cbf053b was a pre-existing flake — green on re-run

Flagging it because it is in the PR's history. The failure was:

--- FAIL: TestCapRefusalsAreBoundedPerVM (0.00s)
    proxy_test.go:527: no refusal was ever in flight: the test did not exercise the refusal path
FAIL	github.com/rossoctl/moca/remote-worker/internal/egress	48.915s

Every package this PR touches passed in that same run, internal/shortdir included.

The same test failed on main. Run 40708539112 at c916d731 (the #646 merge, which is not in this branch's history) failed identically, at proxy_test.go:530 — same assertion, different line because main has moved:

--- FAIL: TestCapRefusalsAreBoundedPerVM (0.00s)
    proxy_test.go:530: no refusal was ever in flight: the test did not exercise the refusal path

The test arrived in 87cc3fe9 (#641) and this branch does not touch it — this branch's entire diff to proxy_test.go is the two lines of newHarness that call shortdir.New (the need argument, and the redundant cleanup removed). The mechanism looks timing-dependent by construction: it writes CONNECT and then immediately polls capRefusalsInFlight, so peak == 0 means the refusal completed between the write and the first poll on a slow runner, rather than the bound being violated. The (0.00s) is consistent with that — it never got as far as measuring anything.

proto then passed on 91cc8db4 (run 40712180471) with internal/egress green at 47.7s and internal/shortdir green at 1.1s, which is the same test code — only the three commits above differ, none of which touch proxy_test.go's cap-refusal path. A test that fails and then passes on code that did not change is a flake by definition; combined with the identical failure on main, that is as close to settled as this gets without a fix.

Happy to open a separate issue, or to take a fix in this PR if you would rather not have it bouncing: polling with a deadline, or holding the refusals open so they cannot complete early, would both close the window. I have left it alone for now since it is #641's test and not this PR's subject.

Verification, and what I could not verify here

Stated plainly, since the last round's "CI is the test" turned out to matter:

• gofmt and go vet ./... clean, checked on linux/amd64 in a golang:1.26-alpine container (Go 1.26.4, the version go.work pins) as well as on darwin — the host here is arm64 darwin, so a darwin-only check was not evidence for CI.
• internal/shortdir, egress/conformance and internal/egress pass under -race on that Linux container at this head — and, more to the point, all 15 checks are green on 91cc8db4, proto included, with every remote-worker package passing.
• A/B under -race on Linux, this commit's parent (0ebc871b, same rebased base) against the branch: internal/vmpool ok/ok, internal/egress ok/ok, internal/egress/control ok/ok. No regression from these commits.
• The shortdir tests were also run under CI's own conditions (TMPDIR unset, /tmp writable): all 6 pass, none skipped.
• I cannot run bind(2) in my sandbox, so the darwin EINVAL the must-fix is ultimately about is reasoned from the byte counts above, not observed. The path arithmetic is checkable by hand, and the shortdir unit tests now pin the decision that precedes the bind. The fallback branch was exercised in a scratch copy of the package with the /tmp candidate pointed at a writable short path — the only way to reach it here.

One structural note for the record: both findings on this PR were things CI could not catch, for the same reason — the Linux runners take /tmp, so neither the darwin path length nor an undersized need is ever exercised. callers_test.go closes the second of those on every platform. For the first, a job running remote-worker's tests with a long TMPDIR would have caught this must-fix; happy to add one if you think it earns the minutes.