Repository navigation
fix(core): don't mistake a stale daemon PID file for a running daemon (V2-1655) - #235
Open
Nic-dorman wants to merge 3 commits into
Open
Nic-dorman wants to merge 3 commits into
Nic-dorman wants to merge 3 commits into
Conversation
… (V2-1655) A daemon that dies without cleaning up (a reboot, an OOM kill, SIGKILL) leaves daemon.pid and daemon.port behind. Once the PID is reused by an unrelated process, `daemon info` and `daemon start` saw a live PID and treated it as the daemon: `info` reported running while `status` (which asks the daemon over HTTP) said not running, and `start` refused to start a new daemon, so the hint `status` prints led nowhere. `info` and `start` now classify the PID file through one helper that also requires the process to be identifiable as the daemon, the check `stop` already applied before signalling it. `start` removes stale files before spawning; otherwise its port poll would read the dead daemon's port straight back. macOS now checks the executable name via proc_pidpath instead of trusting any live PID. The `info` help text and README no longer claim its output is always JSON: it has been human-readable unless --json is passed since 920f872, which is the convention for every command. Adds unit tests for the reused-PID case and an e2e skill step (5.5). Closes V2-1655 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rustdoc's private_intra_doc_links lint fails the Documentation job under -D warnings. Describe the check inline instead of linking to `recorded_daemon`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Nic-dorman
force-pushed
the
nic/v2-1655-ant-node-daemon-infostart-trust-a-stale-pid-file-report
branch
from
October 23, 2026 18:52
c714134 to
da95adb
Compare
MaidSafe-Hermes
requested changes
Oct 23, 2026
MaidSafe-Hermes
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES — 2/2 seats reviewed (sol-1 gpt-6.1-sol xhigh; glm-1 z-ai/glm-5.3). The seats disagree; I confirmed the platform-identity gap below against the frozen head. CI checks were all passing at review time.
- P2 —
ant-core/src/node/daemon/client.rs:496-516(also Windows:519-545): the macOS and Windows validators accept any live process whose executable basename isant/ant.exe; unlike the Linux validator, they do not verify daemon-specific arguments. A stale PID reused by anotherantCLI process therefore classifies asLive, soinfocan again reportrunning: trueandstartcan refuse recovery. Please verify that the process is running the daemon command (or use another daemon-specific identity signal) on these platforms, and add a regression test using a same-executable, non-daemon process. The current test only uses the test harness executable, so it does not cover this case.
Coverage: 2/2 seats reviewed. sol-1 returned CHANGES_REQUIRED; glm-1 returned APPROVE but noted the executable-name limitation. The process-identity issue is reproducible from the platform checks in the diff; it blocks approval.
…d Windows too Review of #235: the macOS and Windows checks accepted any live process whose executable is named `ant`. A stale PID reused by another `ant` command, such as a long `ant file upload`, was still reported as the running daemon, and `start` still refused to start. Each platform now requires the process to be `ant` running `daemon run`, the arguments `daemon start` passes to the detached child. macOS and Windows read the arguments through `sysinfo`, which ant-core already uses for node adoption. The executable-name checks stay as they were: they work for another user's process and for an elevated one. Where the arguments can't be read, `sysinfo` reports none, and the name check alone decides. Linux keeps its /proc reading but uses the same rule, which is stricter than the previous "any `daemon` argument". New tests copy the test binary as `ant` and run it as `node daemon run` (accepted), as `file upload` (rejected), and as `node daemon status` behind a reused PID (`info` reports it not running). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Community report: after an unclean daemon exit,
ant node daemon infosaid the daemon was running (with a console URL that refused connections),statussaid it wasn't, andstartrefused to start a new one.Cause. A daemon that dies without cleaning up (a reboot, an OOM kill, SIGKILL) leaves
daemon.pidanddaemon.portbehind.infoandstartdecided "running" fromkill(pid, 0)alone. Once the PID was reused by an unrelated process, they treated it as the daemon.statusasks the daemon over HTTP, so it disagreed, and its "Start it with:ant node daemon start" hint went nowhere. Onlystopchecked that the PID really is the daemon.Fix.
infoandstartclassify the PID file through one helper (recorded_daemon): live and identifiable as the daemon, which is the checkstopalready applies before signalling.startremoves stale PID/port files before spawning. Otherwise its port poll reads the dead daemon's port straight back as the new daemon's.proc_pidpath, as Windows does), where it previously trusted any live PID. This also tightensstopon macOS.info's help text and README no longer say its output is "always JSON". It has been human-readable unless--jsonis passed since 920f872, which is the convention for every command. The e2e skill already usesinfo --json.statusandstoplogic are unchanged.Every distribution (install.sh, install.ps1, npm, release archives, the Linux .deb) names the binary
ant/ant.exe, which is what the identity check requires.Linear issue
Closes V2-1655
Risk tier
Compatibility
daemon.pid/daemon.portfiles; stale ones are now removed bystart)ant-coreitems added or changed;DaemonInfo.runningis now false for a stale PID file, which is what it documents.Semver impact
Test evidence
Unit tests (
cargo test -p ant-core --lib node::daemon::client): 12/12 pass on Windows and Linux. The new tests use the test process's own PID as the "reused" PID. The two tests for the reported case (info_with_a_reused_pid_is_not_running,start_clears_files_left_by_a_dead_daemon) fail with the old PID-only check restored, and pass with the fix.Lint:
cargo clippy --all-targets --all-features -- -D warningsclean on Linux;cargo fmt --checkclean.Reporter's scenario on Linux, real binaries, isolated
XDG_DATA_HOME: start a daemon,kill -9it, write a live unrelated PID intodaemon.pid.origin/maine47ea7ainfoon a genuine daemonrunning: truerunning: trueinfoafter kill -9 + reused PIDrunning: truerunning: falsestatusrunning: falserunning: falsestartalready_running: true, pid = reused PIDinfo/statusafterwardsmacOS is not run locally; the
proc_pidpathpath is compiled and tested by CI's macos-latest jobs.The e2e skill gains step 5.5, "Stale PID file recovery", which covers the same scenario.
Prod compat smoke (2026-10-23), re-run at the rebased head
da95adb5againstmainccd4106d. Run on Linux (dev2, stable 1.99.1) with--lockedrelease builds and isolatedXDG_DATA_HOME/XDG_CONFIG_HOME.mainand is fixed here:main, afterkill -9and a reused PID,inforeports the decoysleepPID as the daemon.startprintsalready running (PID <decoy>)andstatussays not running.infosays not running andstartlaunches a new daemon.infoandstatusthen agree on its PID, andstopexits cleanly.stoponmainalready catches the reuse:PID … is alive but does not appear to be the ant daemon (possible PID reuse). Stale PID file removed.This PR givesinfoandstartthe same check.ant file downloadof a public canary file (3cba93c8…, 5,277,908 B) with this build succeeded in 14.6 s, and the byte count matches.New dependency
none (
libcis already a dependency;proc_pidpathis from it)ADR
n/a
Mitigation / rollback
Revert the commit. If a renamed
antbinary fails the identity check, the impact is limited toinfo/startseeing its daemon as stale;stophas always refused such a daemon.🤖 Generated with Claude Code