fix: use buildMtime for daemon stale detection, preserve port on restart#42
Conversation
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 094a7c8f32
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
3d68f26 to
1d2fd15
Compare
1d2fd15 to
de204a2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de204a26fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Instead of comparing daemon.js mtime against startedAt (which is fragile due to filesystem timestamp precision), the daemon now records daemon.js mtime at startup in daemon.json. The CLI compares the current mtime against this stored value — if they differ, the file was rebuilt. Also preserve the original port when auto-restarting so daemons on non-default ports don't silently switch to 8097. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
de204a2 to
7c2373b
Compare
Summary
Follow-up to #41 which introduced daemon auto-restart but had two bugs:
Port lost on restart —
ensureDaemon()restarted the daemon withport = undefined(most CLI commands don't pass a port), silently switching to 8097. Now preserves the originalinfo.port.False restarts in CI — comparing
daemon.jsmtime againststartedAtwas fragile: when build and daemon start happened close together, filesystem timestamp precision could make them appear out of order.New approach
The daemon now records
daemon.js's mtime at startup asbuildMtimeindaemon.json. The CLI compares the file's current mtime against this stored value — if they differ, the file was rebuilt and the daemon restarts. No tolerance or clock comparison needed.Changes
DaemonInfogains optionalbuildMtimefielddaemon.jsmtime todaemon.jsonon startupensureDaemon()compares current mtime vs storedbuildMtimeinfo.portwhen restartingE2E test
New
daemon-auto-restart.test.tsuses the full CLI lifecycle (start/stop/get tree) to verify:buildMtimedoesn't matchbuildMtimematchesTest plan
🤖 Generated with Claude Code