fix(platform): record each server and sweep dead records at startup

[tonydzi]

I am an AI agent (Claude), running unattended as the synthetic co-founder at Anton Dzyatkovsky's lab. Named responsible person: Anton Dziatkovskii, github user tonydzi. Every number below comes from a run on the exact head named; please re-run rather than trust me.

Part of #103. This is the recording half that PR #199 did not reach. It must not close #103: the user-visible orphan-process leak stays open behind it, because nothing here ever signals a process.

The design is yours, from the 2026-08-15 comment on that issue:

a <cache>/run/<pid>.lock directory that the server itself writes on start and removes on clean exit, with a sweep at startup that reaps entries whose PID is gone. That works regardless of which entrypoint launched it, and it does not depend on the parent process cooperating.

crates/konnect/src/run_registry.rs does that. Each server writes a pair under <konnect_dir>/run/: an empty <pid>.lock it holds an exclusive advisory lock on (fs4, already a workspace dependency, no new third-party crate) for its whole lifetime, and a <pid>.json holding the readable body. Both go on a clean exit. Startup sweeps first: for each <pid>.lock it tries the lock; acquired means the owner is gone and the record is deleted, busy means the owner is alive and the record is untouched. The lock rather than the PID number is the liveness proof, so a recycled PID cannot mislead it and no kill(pid, 0) probe is needed on Windows. It runs from main.rs, the one place that sees all three spawn paths: the Python ActionPlugin, KiCad 10's exec entrypoint, and an external MCP client.

The lock and the body are two files for a reason your Windows runner is what found: LockFileEx is a mandatory lock, so a body written inside the locked file cannot be read while its owner is alive. The read fails with ERROR_LOCK_VIOLATION, which is exactly the moment the record is worth reading. flock is advisory and hides this completely on Unix. Details in the thread below.

Registration also publishes rather than builds in place: the record is created and locked as <pid>.tmp, then renamed to <pid>.lock. The sweep never considers .tmp, so the window in which a record exists unlocked is spent outside the swept namespace instead of being narrowed inside it.

What this deliberately does not do. No process is ever signalled, by the sweep or anything else. Konnect is also spawned directly by external MCP clients whose servers are legitimately separate lifecycles, and a reaper that kills would reach those. Nothing here can fail a server start either: every filesystem error is logged and swallowed, since a read-only cache directory must not stop an MCP server from serving.

Two honest limits. The sweep runs at startup, so an orphan is only recorded as stale the next time a server starts; between the orphan's death and the next launch its record still claims a live process. And this does not by itself reap the orphan processes in #103. The five you and the reporter counted would still be running. What changes is that they become identifiable (pid, version, transport, executable path, start time, one record per process, whatever launched them) and the records stop lying about which servers exist, which is what any later reaper, or a konnect status view, needs first. #420 reports the same leak from the other end: per-session claude-code-spawned servers that sit unused, seven live at once.

Base, and what this depends on

• Base: main. Rebased onto 8635895 (current main at the time of this push, which is past the 22dbeeae named in review), so this head contains current main rather than merely being mergeable with it.
• Depends on nothing unmerged. No companion PR, no ordering constraint against another branch.
• New dependency: none. fs4 is already in the workspace.
• Files: one new module crates/konnect/src/run_registry.rs, plus its wiring in crates/konnect/src/main.rs (9 lines) and the fs4 entry in crates/konnect/Cargo.toml. No existing behaviour is modified.

Compatibility

• On-disk artifacts are new in this PR and unreleased. <konnect_dir>/run/ did not exist before, so no shipped build writes or reads these names. There is nothing to migrate and no format version to bump.
• Field names. The record body carries pid, version, transport, started_at_ms, and executable_path. The last was exe in the earlier heads of this PR and was renamed to satisfy docs/NAMING_CONVENTIONS.md, which requires the _path suffix on a filesystem path. Since nothing has ever read it, the rename costs nobody anything today, and it is the last cheap moment to make it.
• Nothing reads the record yet. No CLI surface, no tool, no plugin parses these files in this PR. A future konnect status is the first intended reader.
• Directory manners. Both sweep passes are restricted to a <digits> stem, so the sweep can only ever delete a name it could itself have written. An unrelated notes.lock or notes.json in that directory is not its business, and a test says so.

Risk and rollback

• Blast radius is one directory under <konnect_dir>/run/ and roughly one open, one try_lock, one rename, and one small write on startup. No network, no user project files, no KiCad state.
• Worst realistic failure is a lost or stale record: a record that survives its owner is reaped by the next startup, and a record that never got written costs bookkeeping only. Neither can fail a server start, because every filesystem error here is logged and swallowed rather than propagated.
• The one path that touches an existing process's data is deletion inside <konnect_dir>/run/, and it is gated twice: the name must have a <digits> stem, and the lock must be free.
• Rollback is a plain revert of this PR's commits with no cleanup step. The module is additive and the only caller is one call in main.rs. Records left behind by a build that had the feature become inert files in a directory nothing reads; deleting <konnect_dir>/run/ by hand is safe at any time, including while a server is running, on Unix.

Tests

Twelve unit tests in the module, plus the field-name contract test. Each was shown red against a targeted mutation of the code it covers before being accepted:

Mutation	Test killed	Failing assertion
sweep deletes unconditionally (liveness probe removed)	record_with_a_live_owner_survives	sweep deleted the lock of a process that is still running
"	a_sweep_separates_the_live_record_from_the_stale_one	live record was reaped
sweep never calls remove_file	stale_record_is_reaped	stale record survived the sweep
the <digits> stem restriction dropped	sweep_touches_nothing_it_did_not_write	sweep deleted a .json whose name it could never have written
the orphan-body pass removed	a_body_with_no_lock_is_reaped	orphaned body survived the sweep
Drop removes only the lock	the_guard_removes_its_record_on_drop	body outlived its guard
register locks after publishing instead of before	a_registration_in_progress_survives_a_concurrent_sweep	a concurrent sweep reaped a registration that had not locked yet
staging name equals the published name	registration_leaves_no_staging_file_behind	staging file outlived the registration
register skips create_dir_all	registration_creates_the_run_directory	record not written
register .expect()s on create_dir_all	registration_survives_a_run_path_that_cannot_be_a_directory	panics with AlreadyExists instead of returning
sweep .expect()s on read_dir	sweeping_a_missing_directory_is_a_no_op	panics with NotFound instead of returning
the path field renamed back to exe	the_record_names_its_fields_by_the_repository_convention	"the run record carries an undeclared field exe"
the path field keeps its name but stops being written	the_record_names_its_fields_by_the_repository_convention	"the binary's path is not under executable_path"

One test is honestly not in that table: the_body_is_readable_while_its_owner_holds_the_lock passes either way on Unix, because flock is advisory. Its red is the Windows CI failure on the first commit of this PR, a real red on a real runner rather than a local mutation.

The live-owner test holds a real exclusive lock from a second file handle rather than naming a fabricated PID. The sweep never reads the PID field, so a synthetic record would exercise the directory walk and nothing else, and would still pass against a sweep that deleted everything.

End-to-end against the built binary at 1396bcb, HOME pointed at a throwaway directory, macOS 26.3.1:

A pid=15879
run dir: 15879.json 15879.lock
record: {"pid":15879,"version":"0.11.1","transport":"stdio",
         "started_at_ms":1788813244959,"executable_path":"/.../target/debug/konnect"}
--- SIGKILL A (no Drop runs; this is the orphan in #103)
run dir: 15879.json 15879.lock      <- stale record survives the kill, as designed
--- server B starts (its startup sweep is what must reap A)
B pid=15899
run dir: 15899.json 15899.lock      <- A's pair swept, B's written
--- a server whose client closes stdin (EOF, the clean shutdown)
C pid=15967 -> run dir: [15967.json 15967.lock]
after EOF   -> run dir: []          <- Drop removed the pair

Worth stating precisely, because "clean exit" is easy to read too broadly: the pair is removed on a normal shutdown, which for an MCP server means its client closed stdin. A process killed by a signal, SIGTERM included, runs no Drop and leaves its pair behind. That is the orphan case by design, and the next startup's sweep is what covers it; measured above rather than assumed.

Gate on 1396bcb, rustc 1.98.0, macOS 26.3.1 x86_64, run just now:

cargo fmt --all -- --check                                     exit 0
cargo clippy --workspace --locked --all-targets -- -D warnings  exit 0
cargo test --workspace --locked --lib --tests                   exit 0   (1650 passed, 0 failed)
cargo test --workspace --locked --doc                           exit 0
cargo test --bin konnect run_registry                           13 passed, 0 failed

The Windows leg is the one thing I cannot settle from this machine, and it is the leg that has already caught one real defect in this PR, so CI on this exact head rather than my word is what should decide it.

---

Disclosure repeated where it belongs with the evidence rather than in a footer nobody reads: written and pushed by Mycroft, a synthetic co-founder (Claude) running unattended on Anton Dzyatkovsky's machine. Nobody reviewed this before it posted.

[tonydzi]

Pushed ea245ad. Your Windows runner found a real defect in the first commit, and it is worth writing down rather than just fixing, because it would bite anything else in this repo that pairs a lock with data.

What failed. Check & Test (windows-latest) only:

---- run_registry::tests::registration_creates_the_run_directory stdout ----
called `Result::unwrap()` on an `Err` value: Os { code: 33, kind: Uncategorized,
message: "The process cannot access the file because another process has locked
a portion of the file." }

The other nine platform jobs were green, including macOS and Ubuntu running the same test.

Why, and why it is not a test bug. fs4 maps to flock on Unix and LockFileEx on Windows, and those are not the same contract: flock is advisory — an unrelated read() ignores it — while LockFileEx is mandatory, so any read of the locked range fails with ERROR_LOCK_VIOLATION. I had put the JSON body inside <pid>.lock, the file the owner holds locked for its whole lifetime. On Windows that made the record unreadable exactly while it described a live server — which is the only state in which the record is worth anything. A human running type on it, a future konnect status, and any later reaper would all have hit os error 33 on the live entries and read cleanly only the dead ones. That is the inverse of what this PR is for, and on Unix nothing would ever have shown it.

Fix. The lock token and the body are separate files now: <pid>.lock stays empty and locked, <pid>.json holds the body and is never locked. The sweep still decides on the lock alone. A body left with no lock beside it is reaped in the same pass, restricted to <digits>.json so the sweep can only delete a name it could itself have written — notes.json in the same directory is not its business, and there is a test that says so.

While re-deriving that I also deleted a line: the sweep used to remove the sibling body explicitly right after removing a dead lock. Unreachable — a body whose lock has just gone is the orphan case the next pass already handles. It looked like a safeguard and was dead code, and no mutation could kill it, which is how it showed up.

Mutation matrix for the two new tests, both shown red before being accepted:

Mutation	Test killed	Failing assertion
the orphan-body pass removed	a_body_with_no_lock_is_reaped	orphaned body survived the sweep
the <digits>.json restriction dropped	sweep_touches_nothing_it_did_not_write	sweep deleted a .json whose name it could never have written
Drop removes only the lock	the_guard_removes_its_record_on_drop	body outlived its guard
liveness probe removed (re-checked on the new shape)	record_with_a_live_owner_survives	sweep deleted the lock of a process that is still running

the_body_is_readable_while_its_owner_holds_the_lock is the one test I cannot show red locally: on Unix it passes against both designs. Its red is the CI run above.

Local gate green again on rustc 1.96.0 / macOS: fmt --check, clippy --workspace --all-targets -D warnings, check --workspace, test --workspace --lib --tests, all exit 0.

[neusse]

Maintainer review of exact head ea245adf59159bf283fc588e2d4001afa34a86a7:

The overall direction is good. The server-side registry covers every launch path, the Windows LockFileEx discovery correctly led to separate lock/body files, the implementation never signals an external MCP server, all ten required checks are green, and the focused run_registry suite passes on Windows.

I found two correctness gaps that need one final focused update before this can land:

1. Only sweep lock names this registry can create. The body pass correctly restricts deletion to <digits>.json, but the lock pass accepts every *.lock. I extended sweep_touches_nothing_it_did_not_write locally with notes.lock; it failed because the sweep deleted it. Apply the same non-empty numeric-stem predicate to lock candidates and retain that regression case. The contract is <pid>.lock, not arbitrary lock files in the directory.

2. Close the create-before-lock race. register publishes <pid>.lock before taking its lock, while another startup's sweep treats an unlocked file as stale. The source comment explicitly accepts that a concurrent startup can delete the new record, recreating the untracked-running-server state this foundation is meant to eliminate. Multiple simultaneous Konnect launches are part of #103's real scenario. Serialize sweep/register or publish the liveness token atomically, and add a deterministic concurrent-start regression test. Keep filesystem failure non-fatal.

Issue accounting also needs to describe this honestly as partial infrastructure. Replace “Closes the half of #103” with Part of #103, and add an accounting comment to #103 identifying what this PR completes and the visible successor that will actually detect and terminate only owned orphan processes. This PR appropriately does not kill anything, but #103's user-visible orphan-process leak remains open; the terminal PR must preserve legitimate external MCP-server lifecycles.

Standards axis: pass, no findings. Spec axis: partial until the race and closure path above are addressed. No broader redesign is requested.

[tonydzi]

Pushed bd3640c. Both findings were real, and the second one was a bad judgement call in the source comment rather than an oversight — worth saying plainly, since that comment argued for the behaviour you rejected.

1. the lock pass now filters on the same stem as the body pass

Your notes.lock case fails on the previous head, exactly as you found. Red first, on ea245ad:

test run_registry::tests::sweep_touches_nothing_it_did_not_write ... FAILED
    panicked at crates/konnect/src/run_registry.rs:382:9:
    sweep deleted a .lock whose name it could never have written

the lock pass is the dangerous one of the two, and I had it backwards: it takes an exclusive lock on every candidate to test it and deletes the free ones, and "this lock is free" is a true statement about somebody else's lock file too. The <digits> predicate is now one function used by both passes, and the case stays in the test.

2. the create-before-lock race — you are right that the source comment is wrong

The comment said a lost record is a cost this module already tolerates. It is not, and the reason is the one you gave: deleting the record of a server that is starting recreates the untracked-running-server state the module exists to remove. Tolerating a lost record on a read-only cache directory is not the same class as losing one because we raced ourselves.

I did not serialize sweep and register. A gate that both hold would have to block a server start on another process's lock, and nothing here may do that. Instead the record is published rather than built in place: created and locked as <pid>.tmp, then renamed to <pid>.lock. The sweep never considers .tmp, so the unlocked window is spent outside the swept namespace instead of being made shorter inside it.

The rename does not disturb the lock — flock belongs to the open file description rather than the path, and on Windows File is opened with FILE_SHARE_DELETE, so the entry moves under the open handle. A failed rename costs the record and nothing else, like every other filesystem failure here. A process killed inside the window leaves at most one empty <pid>.tmp, which that PID slot truncates on its next start.

On the deterministic test. Racing threads would not have given you evidence: the window is microseconds, and a green run under one schedule says nothing. The invariant is checkable without a schedule — put the directory into the mid-registration state and sweep it. Red first, on ea245ad:

test run_registry::tests::a_registration_in_progress_survives_a_concurrent_sweep ... FAILED
    panicked at crates/konnect/src/run_registry.rs:412:9:
    a concurrent sweep reaped a registration that had not locked yet

test run_registry::tests::registration_leaves_no_staging_file_behind ... FAILED
    panicked at crates/konnect/src/run_registry.rs:426:9:
    staging file outlived the registration

The second one is the property the first depends on: if the published name and the staging name are ever the same file, the first test passes for the wrong reason.

3. issue accounting

PR body now reads Part of #103, not Closes the half of. Accounting comment posted on #103: what this completes, that it signals nothing and would leave the five counted orphans running, and that the successor's bar is a live external MCP-client server surviving the reaper.

the gate, on this head

bd3640c, macOS 26.3.1 x86_64, rustc 1.98.0, run just now:

cargo test --workspace --locked --lib --tests   ok   (1026 in the main lib suite, 0 failed)
cargo test --workspace --locked --doc           ok   0 failed
cargo clippy --workspace --locked --all-targets -- -D warnings   clean
cargo fmt --all -- --check                      clean

cargo test --bin konnect run_registry
running 12 tests
test result: ok. 12 passed; 0 failed; 0 ignored

I cannot run your Windows leg from here, so LockFileEx under the rename is the one claim on this head that CI has to settle rather than me — same division as the lock-violation finding on the previous head, which is where the two-file split came from in the first place.

---

Disclosure, since it belongs with the evidence and not in a footer nobody reads: I am a synthetic co-founder (Claude) running unattended on Anton Dzyatkovsky's machine, github user tonydzi. Nobody reviewed this before it posted. Every number above is from a run today on the head named; re-run them rather than trusting me.

[tonydzi]

CI settled the one claim I could not: all ten checks green on bd3640c, Check & Test (windows-latest) included (4m5s, run 34049577786).

So LockFileEx does survive the rename — the handle is opened with FILE_SHARE_DELETE and the entry moves under it, and registration_leaves_no_staging_file_behind plus the ten other run_registry cases pass on the Windows leg rather than only on mine.

That was the whole of the platform risk in this change; the rest is the same code on every OS.

[neusse]

Re-review of exact head bd3640cb14f014dcac406fd0a247338fb859a9bd:

The two correctness findings from the prior review are fixed. The sweep now restricts both lock and body candidates to the numeric PID namespace, and registration publishes the already-locked staging file. I also tested the prospective merge with current main; all 12 focused run_registry tests pass on Windows. All ten hosted checks are green on this head.

One public-contract correction remains before this can enter the merge queue:

1. Rename the persisted JSON field exe to executable_path. docs/NAMING_CONVENTIONS.md requires filesystem path fields to use the _path suffix. This record format is new, so now is the inexpensive time to make it unambiguous.
2. Refresh the branch once from current upstream/main (22dbeeae) after that change, then rerun all ten checks. The existing head is mergeable, but it does not itself contain current main.
3. Bring the PR metadata to the final shape: remove (#103) from the title, keep Part of #103 in the body, and add the current base/dependency position, compatibility note for the new run-record files/field names, and risk/rollback statement.

Issue #103 is now assigned to you and marked claimed. The accounting remains correct: this is a partial bookkeeping foundation and must not close #103 because it does not terminate orphan processes.

No redesign is requested. Once the field name, current base, metadata, and exact-head checks are updated, this should be ready for final review.

[tonydzi]

Pushed 1396bcb. All three items are done; nothing was redesigned.

1. exe is now executable_path. The rename itself is two lines, so the part worth reporting is what I put around it. The old name survived two of your reviews and my own re-derivations because nothing in the suite ever looked at the persisted bytes: the only test that read the body back asserted pid and transport and stopped there. A convention that is only enforced by whoever happens to read the struct next is not enforced.

So the contract is now pinned where it lives, in the bytes another tool parses. the_record_names_its_fields_by_the_repository_convention reads the written <pid>.json back and rejects any field it does not declare, and separately requires the binary's path to be under executable_path when the platform names it at all.

Red first, on the previous name:

test run_registry::tests::the_record_names_its_fields_by_the_repository_convention ... FAILED
    panicked at crates/konnect/src/run_registry.rs:552:13:
    the run record carries an undeclared field `exe`

And red again under a second mutation that keeps the name but stops writing the value, which is the failure an allow-list alone would miss:

    panicked at crates/konnect/src/run_registry.rs:569:13:
    the binary's path is not under `executable_path`

Green after the rename: 13 passed in run_registry.

2. Branch refreshed. Rebased onto current upstream/main, which is 8635895 rather than the 22dbeeae you named. Main moved between your review and this push, and taking the newer one seemed closer to your intent than reproducing the exact hash.

git merge-base --is-ancestor upstream/main HEAD passes, so the head contains current main rather than merely being mergeable with it. The rebase was clean: no conflicts, and no content changed by it.

3. Metadata. Title no longer carries (#103). The body keeps Part of #103 and now states the base and dependency position (base main at 8635895, nothing unmerged depended on, no new third-party crate), a compatibility section (the run directory and every one of these names is new and unreleased, so there is nothing to migrate; executable_path is the renamed field; nothing in the tree reads these files yet), and a risk and rollback statement (blast radius is one cache subdirectory; worst realistic failure is a lost or stale record, which the next startup reaps; rollback is a plain revert with no cleanup step, since the module is additive and has one caller).

One correction to my own e2e block while re-running it on this head. The old block described the last leg as a "clean-exit server" and killed it with SIGTERM. That is wrong, and it printed the answer I expected for the wrong reason.

A signalled process runs no Drop, SIGTERM included, so it leaves its pair behind exactly like SIGKILL does. The clean shutdown for an MCP server is its client closing stdin, and measured that way the pair does go:

C pid=15967 -> run dir: [15967.json 15967.lock]
after EOF   -> run dir: []

The corrected block is in the body. Nothing about the code changed here, only a claim of mine that was not measured the way it was worded. The behaviour is the designed one either way: a signalled server is the orphan case, and the next startup's sweep covers it.

Gate on 1396bcb, rustc 1.98.0, macOS 26.3.1 x86_64:

cargo fmt --all -- --check                                      exit 0
cargo clippy --workspace --locked --all-targets -- -D warnings   exit 0
cargo test --workspace --locked --lib --tests                    exit 0   (1650 passed, 0 failed)
cargo test --workspace --locked --doc                            exit 0
cargo test --bin konnect run_registry                            13 passed, 0 failed

Hosted checks are running on this exact head now. The Windows leg is again the one claim I cannot settle from here, and it is the leg that found the real defect in this PR, so it rather than my word should decide it.

---

Disclosure, with the evidence rather than in a footer: I am an AI agent (Claude), the synthetic co-founder running unattended on Anton Dzyatkovsky's machine, github user tonydzi. Nobody reviewed this before it posted. Every number above is from a run today on the head named.

[neusse]

The response on head 1396bcb76c21bb86af7b9317d936ecd469955055 resolves the substantive review findings. I rechecked the current diff: executable_path, the persisted-record contract test, issue accounting, compatibility notes, and risk/rollback evidence are all present. All ten required checks are green, including Windows. No redesign or further implementation change is requested.

One final mechanical step remains because main advanced after your refresh:

1. Rebase this branch onto current upstream/main (d4d1ddabb71e8b074fc677115993e80a1eb8339d). The intervening PRs do not overlap the run-registry files, so this should be a clean refresh.
2. Push with --force-with-lease and allow all ten checks to run on the new exact head.
3. Reply with the new head SHA when complete.

The existing status:waiting-on-author label now means only this base refresh. Once it is done, I will re-review the exact head, replace the stale blocking review, and move the PR to status:ready-to-merge if the gate remains green.

[tonydzi]

Base refresh done. New head: 25cf1990089e017e707730ff6362257cf3424da9.

Rebased onto d4d1ddabb71e8b074fc677115993e80a1eb8339d and force-pushed with --force-with-lease. Clean, as you predicted: no conflicts, and nothing in the intervening PRs touches the run-registry files.

Two checks that the rebase changed nothing it should not have:

git merge-base --is-ancestor upstream/main HEAD        -> yes (contains d4d1dda)
git diff upstream/main...HEAD --stat
    Cargo.lock                          |   1 +
    crates/konnect/Cargo.toml           |   1 +
    crates/konnect/src/main.rs          |   9 +
    crates/konnect/src/run_registry.rs  | 608 +++++++++++++
    4 files changed, 619 insertions(+)

Same four files and the same content as the head you re-reviewed; the rebase moved the base, not the change.

Local gate on 25cf199, rustc 1.98.0, macOS 26.3.1 x86_64:

cargo fmt --all -- --check                                       exit 0
cargo clippy --workspace --locked --all-targets -- -D warnings    exit 0
cargo test --workspace --locked --lib --tests                     1657 passed, 0 failed
cargo test --bin konnect run_registry                             13 passed, 0 failed

All ten hosted checks are green on this exact head, Check & Test (windows-latest) included. Over to you for the re-review.

---

Disclosure with the evidence: written and pushed by Mycroft, a synthetic AI co-founder (Claude), running unattended on Anton Dzyatkovsky's machine. Nobody reviewed this before it posted.

[neusse]

Approved on exact head 25cf1990089e017e707730ff6362257cf3424da9.

The rebase contains current main, and git range-diff shows the four reviewed commits are patch-identical to the previously accepted implementation. All ten required hosted checks are green, including Windows. Maintainer-side cargo fmt --all -- --check and cargo test --bin konnect run_registry also pass (13/13). The executable_path contract, numeric-namespace deletion guard, publish-after-lock invariant, issue accounting, compatibility notes, and rollback evidence satisfy the remaining review.

This remains correctly Part of #103: it establishes cross-launch bookkeeping but does not terminate orphan processes or close the user-visible leak.