fix(rpc): sweep stale RPC state and name its directories

[iceteaSA]

Independent of the other open PRs — branched off main at v0.4.3, touches only src/rpc/ plus the two call sites that resolve the directory.

The state tree only ever grew

A dead-pid port file was removed only as a side effect: when a TUI happened to discover a server in that same project directory. Nothing ever removed the directories. On my machine that left 14 project directories, 4 of them completely empty, and the count only goes up — every project you ever opened leaves one behind.

The server now sweeps at startup: dead-pid port-*.json files, then directories left empty. It never touches a live pid, never removes the directory it is about to use, and a sweep failure is logged rather than propagated — collecting old state must not stop a session from starting.

Corrupt port files are collected too. Both the sweep and discovery previously swallowed the parse error, so a truncated file could be neither used nor removed — permanent litter in the mechanism meant to prevent exactly that. Unlinking one is safe because writePortFile writes to a .tmp name and renames, and only port-*.json is considered, so a parse failure means genuinely corrupt rather than mid-write. Discovery stays silent about them since it runs on the TUI polling path, where a per-poll line would be noise.

Directories say what they are, and stop being world-readable

22aebfef4e44976f tells an operator nothing and could collide with a sibling plugin sharing the tree. They are now openai-auth-<hash>, and created 0700 — the port files inside were already 0600, but a world-readable directory still discloses which projects exist. Only directories matching the plugin's own naming are swept or chmod-ed; a configured override path may belong to something else and is left alone.

Resolution deliberately does not consult liveness

Worth flagging, because the first version of this change did and it was wrong.

That version kept using a legacy directory while it still contained a live server — which reads as a considerate migration, but makes the answer depend on when it is asked:

1. old-build server running, port file in the legacy directory, pid alive
2. new server half starts, sees a live legacy entry, writes its port file there
3. that old process exits; its port file is swept as dead-pid
4. the TUI half starts, legacy is now empty, resolves to the new directory — and never finds the server

Step 3→4 is just "the old session was closed after the new one started". The two halves are separate module instances, so when they disagree the TUI silently never finds the server and every modal breaks with no error anywhere.

Both halves now derive the path from the project directory alone, so any two resolutions agree regardless of what exited in between. Plugin changes are restart-gated, so a session cannot be running this resolver against an older build's server in the first place — the state the migration defended against cannot occur. Legacy directories are still swept once nothing live remains in them.

Testing

728 pass / 0 fail (from 714). Build, typecheck and biome clean.

Every production hunk was mutation-proven individually — ten of them, re-run independently by a cross-family reviewer rather than taken from the implementer's report. The mechanic differs by assertion type and is labelled per test: reverse-removal for positive assertions (a dead file is removed), and inverse mutation for absence assertions (a live file is not removed; a foreign directory is not chmod-ed; the active directory is not rmdir'd). Reverse-removal cannot prove an absence assertion — deleting code deepens compliance, so the test can never redden. Those three are the ones worth checking, since they are what stops "collect stale state" from degenerating into "collect state".

The divergence in step 1-4 above was also confirmed by test before being fixed, not argued from reasoning.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Sweeps stale RPC state at startup, switches to named private RPC directories, and hardens port-file writes against concurrent cleanup. Both server and TUI now resolve the same stable path from the project directory, preventing mismatches.

• Bug Fixes

  • Sweep dead port-*.json files and remove emptied project dirs at server start (includes legacy-named dirs); never touch live PIDs or the active dir.
  • Remove corrupt/malformed port files; discovery stays quiet and the sweep continues through bad entries.
  • Port-file write survives a concurrent rmdir with a single ENOENT retry; sweep failures are logged and never block startup.

• New Features

  • RPC dirs are now openai-auth-<hash> and created with 0700; port files remain 0600.
  • Added resolveRpcDir used by both halves; it returns the managed path with secure mode and sweep root. Override paths stay anchored but are not secured or swept.
  • Path resolution ignores liveness; legacy dirs are swept once nothing live remains.

^{Written for commit 08bd18c. Summary will update on new commits.}

[cubic-dev-ai]

All reported issues were addressed across 8 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[ualtinok]

Reviewed against main at v0.4.3. I verified the five things that could turn "collect stale state" into "collect state", and they all hold:

• Unlink is gated on pidAlive and rmdir on resolve(dir) !== active, so a live port file and the active directory are both safe (port-file.ts, sweepRpcState).
• Both halves resolve through resolveRpcDir with no liveness branch, so the loader and TUI cannot disagree. I agree with dropping the migration — the step 1-4 divergence you describe is real and the restart gate makes the migration unnecessary.
• secureDir/sweepRoot are set only on the non-override path, so a configured override is never swept or chmod-ed.
• The tmp name is port-<pid>.json.<pid>.tmp, which the port-*.json filter does not match, so a mid-write file cannot be unlinked.
• The absence tests are paired with a positive assertion in the same test, so a no-op sweep fails them. The corrupt-plus-live case is the one that matters and it gates correctly.

One thing to fix before merge, narrow but silent when it hits.

writePortFile does mkdir then writeFile(tmp). Another project's process sweeping concurrently can rmdir this directory in that window — it is empty, and it is not that process's activeDir, so both guards pass legitimately. writeFile then fails ENOENT, startRpcServer throws, and the loader's catch at index.ts:1470 swallows it. The result is a session with no RPC server at all: every modal silently does nothing, with no error surfaced anywhere. That is the same failure signature the directory-resolution fix was meant to eliminate, arriving by a different route.

Retrying writePortFile once on ENOENT closes it. Please add a test that removes the directory between the mkdir and the write and asserts the port file still lands.

Non-blocking: isUsablePortFileEntry only requires a finite pid, so a {"pid":123} file with no port survives the sweep and is neither usable nor collected. Requiring a finite port too would make it self-cleaning.

[iceteaSA]

Fixed in 08bd18c. 732 pass / 0 fail (from 729). Both points were right.

The mkdir/rmdir window. What makes this one nasty is that neither sweep guard is wrong — activeDir protects the sweeper's own directory and cannot know about a third process mid-mkdir elsewhere, and the directory genuinely is empty at that instant. So the fix belongs on the write side, not on another sweep guard.

I widened the retry beyond your suggestion after looking at where else the window reaches: rename shares it. The .tmp file lives in the same directory, so a sweeper can empty it between writeFile and rename just as easily as between mkdir and writeFile. Retrying only the write would have left a narrower version of the same silent failure. mkdir + writeFile + rename are now retried once as a unit, so the directory is recreated before any of them run again.

Bounded at exactly two attempts — a persistent ENOENT fails fast on the second with the original error preserved, rather than looping against a genuinely broken parent path. There is a test pinning the attempt count, because "retry once" degenerating into "retry forever" is the obvious way this goes wrong later.

The race test needs a seam: a one-line optional beforeWrite hook alongside the existing secureDir option. No production call site sets it, so ?.() never executes outside tests. I considered module-spying fs/promises instead, but the production code uses named imports and spy visibility is not guaranteed under Bun's import binding, so the seam was the honest route rather than a test that only appears to exercise the window. I also rejected a second beforeRename hook — the unit-level retry already covers that window, and it would have doubled the production surface for no extra test power.

The pid-only file. Fixed — isUsablePortFileEntry now requires a finite port as well, matching what discoverPortFile already enforced at :140. The two were inconsistent, which is exactly how a file ends up neither usable nor collectable.

Worth recording: the first draft of that test used a dead pid, so it passed through the pidAlive branch and never exercised the new port gate at all — green while proving nothing. Caught during the per-hunk mutation pass and rewritten with a live pid, so the only way the file can disappear is the check under test. Same class of defect as the one you found on #56.

All hunks reverse-applied individually. The retry-bound test used an inverse mutation (widening the loop to five attempts) since reverse-removal cannot fail an assertion about not retrying more.

[ualtinok]

Verified at 08bd18c3. The create-then-rename unit is now retried once on ENOENT with a bounded second attempt, so a concurrent sweep between mkdir and the write can no longer leave a session with a silently dead RPC server. The beforeWrite hook makes the race directly testable rather than inferred, and both the recovery and the no-infinite-retry cases are covered. isUsablePortFileEntry now requires a finite port as well, so the pid-only litter is self-cleaning.

732 tests pass locally on the branch. Ready to merge from my side.