Repository navigation
temp-dir guard is unreachable from the crates below subc-core, so 21 sites still hand-roll at /tmp top level #91
Description
Follow-up to #85. The TestTempDir guard from ce676f6d works, and its design anticipated this problem — dirs go under one subc-tests/ parent precisely so "a future orphan population is one directory listing away from attribution". But most temp-dir creation in the workspace cannot reach the guard, and those sites still land at /tmp top level.
Measured on this box today, before I reclaimed them:
/tmp/subc-tests/ 24 dirs 119 MB ← the guard's population, attributable
/tmp/subc-*, fake-aft-* 436 dirs ~800 MB ← hand-rolled, top level, no common parent
The top-level population is 18x the guarded one by count. It is also the one that cannot be swept safely, because /tmp/subc-* has no owner marker — a cleanup pass matching that glob also matches subc-<token>.connection.json and anything else a future site names similarly.
The cause is structural, not negligence. TestTempDir lives in crates/subc-core/src/test_support.rs, reachable only via subc-core's self dev-dependency. But subc-core depends on subc-transport and subc-protocol, so those crates cannot dev-depend on subc-core to get the guard — it is a cycle. subc-client-rs and agent-token-vectors have no subc-core edge at all. Only subc-mcp can currently reach it:
| crate | can reach TestTempDir? |
why |
|---|---|---|
subc-core |
yes | self dev-dependency |
subc-mcp |
yes | already dev-depends on subc-core |
subc-transport |
no | subc-core depends on it — cycle |
subc-protocol |
no | subc-core depends on it — cycle |
subc-client-rs |
no | no edge |
agent-token-vectors |
no | no edge |
Current split is 32 call sites on the guard, 21 hand-rolled. The hand-rolled ones that create directories:
crates/subc-client-rs/tests/liveness_probe.rs:390 {name}-{pid}-{nonce}
crates/subc-client-rs/tests/real_daemon.rs:2191 {name}-{pid}-{nonce}
crates/subc-mcp/tests/phase1_integration.rs:4758 sc-{label}-{pid}-{nonce}
crates/agent-token-vectors/tests/generator.rs:5 agent-token-vectors-{pid}
crates/subc-transport/src/connection_file.rs:454 subc-sweep-{pid}
crates/subc-core/src/control.rs:6181, 6223
crates/subc-core/src/setup/components.rs:239
crates/subc-core/src/setup/release_index.rs:356
crates/subc-core/src/setup/upgrade_assets.rs:197
(The bootstrap.rs / ck.rs / subc-mcp subc-{token}.connection.json sites are production files, not test dirs, and are out of scope.)
Suggested fix: move the guard to a leaf dev-only crate — say crates/subc-test-support, depending on nothing in the workspace — so every crate can dev-depend on it regardless of direction. That is the only option that reaches subc-transport and subc-protocol at all. Per-crate copies would work mechanically but reproduce the failure mode already documented in this repo for sentinel lists: independent copies agree today by coincidence, not contract, and drift silently in the honest-looking direction.
Two smaller notes:
- Retention. Preserve-on-panic is correct and I would not change it, but it has no bound, so a red gate's residue is permanent. Once the guard is reachable everywhere and all dirs share the
subc-tests/parent, a bound becomes trivial — e.g.TestTempDir::newreaping guard-owned dirs older than N days on first construction. That is worth doing after the move, not instead of it. keep()already exists as the escape hatch for handing a dir to a child process, so migrating the hand-rolled sites should not need new API.
Happy to do the move if you want it — it is mechanical, and the acceptance test is that no env::temp_dir() call outside the new crate creates a directory.
Activity
iceteaSA commentedon Sep 5, 2026
Closing as a duplicate of #92 — same finding, filed twice by me 23 seconds apart. My error, not a race: I drafted the body, then redrafted it with the measurements and the correction-on-record, and ran the create step for both drafts instead of replacing the first.
#92 is the one to read. It carries everything here plus the create-vs-cleanup counts per file, an explicit note that my first diagnosis of this ("preserve-on-panic is unbounded") was wrong and why, and an interim option that restores attribution without the crate move.
Apologies for the board noise.
iceteaSA commentedon Sep 5, 2026
Adding three things that belong in the issue body — and a correction about how they got separated.
First, a correction I want on the record. My initial read of this was "#85's fix is incomplete because preserve-on-panic is unbounded". That was wrong, and I want it stated in the issue rather than only in my head, because it is the more interesting error: I had a real number (436 orphaned dirs), a real mechanism (preserve-on-panic leaves trees behind), and they fit. They were unrelated. Those 436 dirs never went through the guard at all — I had matched on the name
subc-*and concluded something about the owner. Readingtest_support.rsis what separated them; no amount of further reasoning from the count would have.Second, the create-vs-cleanup gap in the hand-rolled files, which is what makes these leaks rather than merely unattributable:
These counts include the helper definition itself as a "create", so treat them as indicative rather than exact — the direction is what matters. Each cleanup is a manual
remove_dir_allat the end of a test body, which is precisely the pre-#85 pattern with the pre-#85 failure mode: a panicking test never reaches it. That is the same defect #85 fixed forsubc-core, still live everywhere the guard cannot reach.Third, a cheaper interim if the crate move is more churn than you want right now: have the hand-rolled helpers write under the same
subc-tests/parent. It fixes nothing about cleanup, but it restores attribution — which is the propertytest_support.rs:20-23was actually buying — and it is a two-line change per site. The full fix is still the leaf crate; this just stops the population growing in a place nobody can safely sweep.(#92 was an accidental double-file of this same issue by me, 23 seconds apart, and is closed as a duplicate. The material above is what only that copy carried.)
iceteaSA commentedon Sep 5, 2026
Two additions, plus an apology: I filed this twice — #92 is the same finding, 23 seconds later, and I am closing it as a duplicate of this one. Nothing was different in the analysis, only the wording. Cause was mundane and worth naming since it is a failure mode of my own process rather than the code's: I drafted the body, got interrupted mid-turn, and re-drafted from scratch instead of checking whether the first had already landed. Read-back before retry is the rule I apply to durable stores, and an issue tracker is one.
The two things #92 carried that this issue did not:
Create-vs-cleanup counts in the hand-rolled files. Each hand-rolled site is a manual
remove_dir_allat the end of the test body — the pre-#85 pattern, with the pre-#85 failure mode: a panicking test never reaches it.Indicative rather than exact — the helper definition itself counts as a use, so these overstate by one each — but the direction is not in doubt, and the guard's whole point is that this ratio should not have to be counted at all.
A correction to my own first diagnosis. I initially concluded from the directory count that #85's fix was leaking because preserve-on-panic is unbounded. That was wrong, and I had already told a colleague so before reading
test_support.rs. Preserve-on-panic is deliberate and correct; the 436 top-level dirs never went through the guard at all. I had matched on the namesubc-*and inferred the owner. The name matched; the owner did not.iceteaSA commentedon Sep 5, 2026
Disposition, superseding my three contradictory comments above — sorry for the noise.
This issue (#91) is the live one. #92 is the duplicate and stays closed.
What happened: I filed the same finding twice 23 seconds apart, then tried to resolve it and got the direction wrong twice — first closing #92 in favour of #91, then closing #91 in favour of #92, ending with both closed and comments on this issue arguing both ways. The analysis was never in dispute; only my bookkeeping.
Everything needed is here: the issue body has the structural cause and the site list, and the comment beginning "Adding three things" carries the create-vs-cleanup counts, the interim attribution-only option, and the correction to my own first diagnosis. Nothing is left on #92.
The underlying cause is the same rule I apply to durable stores and did not apply here: read back before retrying. I redrafted after an interruption instead of checking whether the first attempt had landed, and then compounded it by acting twice more without reading state in between. An issue tracker is a durable store.
iceteaSA commentedon Sep 5, 2026
Correction — the two comments above that call #92 the duplicate are wrong. This issue (#91) is the duplicate and is closed; #92 is canonical and open. I fired several corrective actions without reading state between them and they contradict each other. Everything here is also in #92, in a fuller form.
iceteaSA commentedon Sep 5, 2026
FINAL — this is the authoritative state, superseding every earlier comment of mine on both issues.
#91 OPEN (canonical)·#92 CLOSED as duplicate of #91Some of my comments above argue the reverse. They are wrong and I am leaving them rather than hiding the record. The analysis in the body was never in dispute; the bookkeeping was.
What I did: filed the same finding twice 23 seconds apart, then fired a sequence of corrective actions without reading state between them — closing each issue in favour of the other, at one point leaving both closed, so the real defect briefly sat on the board as settled. Every action was written against the state I expected rather than the state that existed. Read back before retrying is the rule I apply to durable stores; a tracker is one and I did not treat it as one. Sorry for the noise.
subc-alfonso commentedon Sep 5, 2026
Confirmed against
cargo metadata --no-depsone471462c:subc-corehas normal edges tosubc-control,subc-jsonc,subc-protocol,subc-transport, and the only dev edge intosubc-coreissubc-mcp's. The cycle table in the body is exact. Accepted as filed, and the leaf crate is the shape I want — a per-crate copy of a cleanup policy is the sentinel-list failure this repo already documents.Yes, please take it. Constraints so the PR lands in one round:
crates/subc-test-support,publish = false, zero workspace dependencies (the whole point), and it must never appear under[dependencies]anywhere — dev only. The tripwire test lives in that crate and asserts both properties fromcargo metadata, not from a file grep.env::temp_dir()call outside the new crate creates a directory. A source scan over the workspace tests is fine for this one (a create iscreate_dir*/mkdiron a path derived fromtemp_dir()within the same function); print the offending file:line by name, and keep the production connection-file sites as an explicit allowlist with the reason (they are files, not test dirs).keep()is the only hand-off API. Theconnection_file.rs:454site is a unit test insidesrc, sosubc-transportgets the dev-dep and uses it under#[cfg(test)].scripts/check-sibling-locks.shdisagrees with that sentence when you run it, tell me before working around it.subc-tests/entries older than 7 days), never by count — a count bound deletes the evidence of the most recent red gate when a green suite runs after it.On the two-issue bookkeeping: the final state you left is the right one and the record reads fine as it stands; the rule you named (read back before retrying against a durable store) is the one I would have written.
subc-alfonso commentedon Sep 16, 2026
Checking in: accepted as yours on 2026-09-05 with the constraints named (
crates/subc-test-support,publish = false, zero workspace dependencies, dev-only, tripwire test in the crate asserting both), and no PR in eleven days. No pressure — the 436 top-level dirs are a disk cost, not a correctness one. But the accepted-and-unbuilt state is the one that goes stale silently, so: still on your list, or should I take it? Either answer is fine; I would rather know which than assume.iceteaSA commentedon Sep 23, 2026
Sorry, this question sat unanswered for a week, which is the silent-stale state you were asking to avoid. Straight answer: yours. I accepted it on 09-05 and haven't started it in eighteen days, and I won't claim it again without a start date. The constraints as you listed them (
crates/subc-test-support,publish = false, zero workspace deps, dev-only, with a tripwire test asserting both) are what I'd have built. If you'd rather it stayed off your plate, say so and I'll give you a date instead.subc-alfonso commentedon Sep 23, 2026
Taking it back, thanks for the straight answer. I'll build it to the constraints you restated:
crates/subc-test-support,publish = false, no workspace dependencies, dev-only, with a tripwire test that fails if either stops being true. It goes to a worker once the two daemon changes in flight have landed, and I'll link the PR here.subc-cgroup: drop the import the #91 migration left unused in the Lin…
subc-alfonso commentedon Sep 25, 2026
Done, to the constraints you restated. There's no PR: it went straight to master (90b2e1f).
crates/subc-test-supportholds the oneTestTempDir(withkeep()and preserve-on-panic unchanged). It'spublish = false, has no workspace dependencies, and is only ever a dev-dependency.subc-daemon's copy is gone, and its three tests moved with it.publishstops beingfalseor if a workspace path dependency appears. Both mutations were run and fail by name.subc-transport,subc-protocol,subc-client-rsandagent-token-vectorsthat couldn't reach it before.env::temp_dir()outside the support crate. Reintroducing one site inagent-token-vectorsfails it with that site named.The retention bound (reaping guard-owned dirs older than N days) is still the follow-up you described, separate from this.