Add detailed StartupReason enum for daemon lifecycle tracking

[akesson]

Summary

• Replace flawed socket-existence-based startup reason detection with explicit CLI arg communication
• Add four StartupReason variants: FirstStart, BinaryUpdated, Recovered, ForceRestarted
• Client determines reason and passes --startup-reason arg when spawning daemon
• Breaking change: DaemonServer::new() now requires startup_reason parameter

Problem

The previous implementation detected startup reason by checking if socket exists before creating a new one. This was unreliable because the client always cleans up the socket before spawning, so the daemon almost always saw "FreshStart" even when replacing an old daemon due to binary update.

Solution

The client knows why it's spawning the daemon, so it now communicates this via --startup-reason CLI arg:

• No socket exists → first-start
• Stale socket (can't connect) → recovered
• Version mismatch detected → binary-updated
• User called restart() → force-restarted

Test plan

• All existing tests pass
• cargo check passes
• Examples updated to parse --startup-reason arg

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

Adds a public StartupReason enum and threads it through client spawn flows, daemon construction, and handler callbacks. DaemonServer API and struct gain a startup_reason parameter/field; client spawn/restart logic, examples, tests, and CI/config/versioning updates are adjusted accordingly.

Changes

Cohort / File(s)	Summary
Core types & traits src/lib.rs	Adds public StartupReason enum (FirstStart, BinaryUpdated, Recovered, ForceRestarted) with as_str, FromStr, Display; adds CommandHandler::on_startup(&self, StartupReason) default method; re-exports StartupReason.
Server implementation src/server.rs	Adds pub startup_reason: StartupReason to DaemonServer; updates DaemonServer::new and new_with_name_and_timestamp signatures to accept startup_reason and propagate it; calls handler on_startup(self.startup_reason) on startup.
Client spawn & restart logic src/client.rs	Determines StartupReason (Recovered vs FirstStart), propagates it into spawn flows and CLI (--startup-reason), introduces spawn_fresh_daemon/retry logic, enforces version handshake after fresh spawns, and updates restart/force-restart behavior to spawn with appropriate reason.
Examples examples/cli.rs, examples/concurrent.rs	Adds minimal CLI parsing for --daemon-name, --root-path, --startup-reason; initializes startup_reason and passes it into DaemonServer::new; includes startup_reason in tracing/log context.
Tests tests/integration_tests.rs, tests/version_tests.rs	Updates all DaemonServer::new_with_name_and_timestamp call sites to pass a StartupReason (typically FirstStart) and an explicit max_connections parameter.
Packaging & metadata Cargo.toml, CHANGELOG.md	Bumps package version 0.6.0 → 0.7.0 and documents breaking API change (new StartupReason and altered DaemonServer constructors) in CHANGELOG.
CI workflow .github/workflows/ci.yml	Adjusts CI matrix: clippy job limited to ubuntu-latest; test job needs relationships changed and runner/matrix tweaks.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Pay extra attention to src/client.rs (retry loop, version handshake, race conditions).
• Verify API changes in src/server.rs and all call sites in tests/examples are consistent.
• Confirm CommandHandler::on_startup semantics and that handlers receive intended enum values.

Possibly related PRs

• Add concurrent client support with connection limiting #1 — Modifies DaemonServer API and struct; likely overlaps with constructor/field changes.
• Replace daemon_id with daemon_name and daemon_path for better isolation #3 — Alters daemon identification APIs (daemon_id → daemon_name/path) and intersects on constructor/signature edits; may conflict.

Poem

🐰 I hopped through args at break of day,
Passing reasons where the daemons play.
FirstStart, Recovered, or ForceRestarted too,
I tuck each purpose in the server's shoe.
Now logs and handlers know just why we grew. 🌿

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: introducing a detailed StartupReason enum for daemon lifecycle tracking, which is the central feature added across all modified files.
Description check	✅ Passed	The description is well-related to the changeset, clearly explaining the problem solved, the solution implemented, and the rationale for the breaking API change.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch startup-reason

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

examples/concurrent.rs (1)

260-283: Edge case in argument parsing for --daemon-name.

The --daemon-name branch unconditionally skips 2 positions without verifying a value exists. If a user provides --daemon-name without a value (e.g., daemon --daemon-name --root-path /tmp), this would incorrectly skip the --root-path flag.

Consider adding the same guard used for other flags:

 "--daemon-name" => {
     // Skip daemon name - DaemonServer auto-detects it
+    if i + 1 < args.len() && !args[i + 1].starts_with("--") {
+        i += 2;
+    } else {
+        i += 1;
+    }
-    i += 2;
 }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b96f713 and 5dc58e4.

📒 Files selected for processing (7)

• examples/cli.rs (2 hunks)
• examples/concurrent.rs (2 hunks)
• src/client.rs (8 hunks)
• src/lib.rs (5 hunks)
• src/server.rs (7 hunks)
• tests/integration_tests.rs (2 hunks)
• tests/version_tests.rs (8 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

examples/cli.rs (1)

src/server.rs (1)

• new (118-129)

examples/concurrent.rs (1)

src/server.rs (1)

• new (118-129)

src/client.rs (2)

src/transport.rs (3)

• daemon_socket_exists (82-93)
• socket_path (49-53)
• socket_path (153-155)

src/error_context.rs (1)

• get_or_init_global_error_context (90-103)

🔇 Additional comments (16)

src/lib.rs (2)

119-167: Well-designed StartupReason enum with clean API.

The enum implementation is solid with appropriate derives (Copy, Default, PartialEq, Eq). The as_str(), FromStr, and Display implementations are consistent and follow idiomatic Rust patterns.

One minor consideration: FromStr::Err = () loses error context. If you later need better error messages (e.g., for user-facing CLI errors), consider using a descriptive error type. However, since this is primarily parsed internally from known CLI args, the current approach is pragmatic.

---

330-338: Good addition with backwards-compatible default.

The on_startup method with a default no-op implementation is a clean approach for optional startup hooks. This won't break existing CommandHandler implementations.

Note: The method is synchronous. If use cases emerge requiring async initialization based on startup reason (e.g., database reconnection on Recovered), this may need to become async in the future.

src/server.rs (3)

72-73: Public field exposes startup reason for inspection.

Making startup_reason a public field allows callers to inspect how the daemon was started. This is consistent with the existing public fields (daemon_name, root_path, build_timestamp).

---

227-228: Correct placement of startup callback.

Calling on_startup after PID file creation but before the accept loop is the right time—initialization is complete, but no client connections are being processed yet. This allows handlers to perform startup-specific logging or initialization safely.

---

118-129: Breaking API change is well-documented.

The constructor signature change requires all callers to provide startup_reason. This is a clean approach that makes the startup context explicit rather than hiding detection logic in the server.

src/client.rs (5)

96-134: Clear startup reason determination logic.

The logic correctly identifies the startup context:

• Existing socket that responds → use existing daemon (no spawn needed)
• Stale socket exists but daemon unresponsive → Recovered
• No socket → FirstStart

The socket cleanup before spawn (line 109) ensures the startup reason is determined correctly before any side effects.

---

161-175: Correct use of BinaryUpdated for version mismatch.

When the daemon reports a different build timestamp, the client correctly cleans up and spawns with StartupReason::BinaryUpdated. This accurately reflects that the binary was rebuilt since the daemon started.

---

463-520: New spawn_fresh_daemon helper encapsulates restart flow.

This helper correctly handles the spawn-and-handshake sequence for explicit restarts. There's some similarity with the connection flow in connect_with_name_and_timestamp, but the separation is justified since spawn_fresh_daemon always spawns a fresh daemon with a known startup reason, while connect_with_name_and_timestamp may connect to an existing one.

---

258-270: CLI argument passing for startup reason is clean.

The --startup-reason argument uses as_str() for serialization, which pairs correctly with the FromStr implementation in src/lib.rs. The argument position (after --root-path) is consistent.

---

439-461: Restart correctly uses ForceRestarted and preserves settings.

The restart() method appropriately uses StartupReason::ForceRestarted for user-initiated restarts and correctly preserves the auto_restart_on_error setting across the restart.

tests/version_tests.rs (1)

39-46: Test updates correctly use StartupReason::FirstStart.

All version tests appropriately use FirstStart since each test spawns a fresh daemon instance. The API updates are consistent across all test cases.

Consider adding tests for other StartupReason variants to verify the on_startup callback receives the correct reason. Would you like me to suggest test cases for BinaryUpdated, Recovered, and ForceRestarted scenarios?

examples/cli.rs (2)

69-98: Basic argument parsing works but has edge cases.

The manual argument parsing is straightforward for an example. A few observations:

1. Line 90: unwrap_or_default() silently falls back to FirstStart for invalid --startup-reason values. This is acceptable since the client controls these values, but a warning log would help debug misconfigurations.

2. Missing bounds check: Lines 83-85 check i + 1 < args.len() before accessing args[i + 1], which is correct. Lines 89-91 do the same. Good defensive coding.

For a production CLI, consider using a proper argument parser (e.g., clap), but for an example, this is acceptable.

---

113-116: Correctly updated to new DaemonServer::new signature.

The example properly passes the parsed startup_reason to DaemonServer::new, and the logging at line 111 includes the startup reason for observability.

examples/concurrent.rs (1)

296-304: LGTM!

The startup_reason is correctly integrated into both the tracing output and the DaemonServer::new call, aligning with the updated API signature.

tests/integration_tests.rs (2)

28-35: LGTM!

The test helper correctly passes StartupReason::FirstStart and 100 for max_connections, matching the expected usage pattern for fresh test daemon instances and aligning with the updated DaemonServer::new_with_name_and_timestamp API.

---

54-61: LGTM!

The test helper for custom connection limits correctly incorporates StartupReason::FirstStart while forwarding the user-specified max_connections, maintaining consistency with the other helper.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

CHANGELOG.md (1)

8-14: Consider adding migration example for StartupReason usage.

The 0.7.0 breaking changes are clearly documented, but unlike v0.3.0 and v0.4.0 entries, this section lacks a migration code example. Prior versions show handler/client/server setup before/after patterns, which help users adapt quickly.

Consider adding a brief "Migration" subsection showing how to construct DaemonServer::new(root_path, startup_reason, handler) and how clients determine and pass the --startup-reason CLI argument. This would follow the pattern established in v0.3.0 (lines 129–170).

Example structure:

### Migration

**Before (v0.6.0):**
\`\`\`rust
let (server, _handle) = DaemonServer::new(root_path, handler);
\`\`\`

**After (v0.7.0):**
\`\`\`rust
let reason = StartupReason::FirstStart;  // Determined by client logic
let (server, _handle) = DaemonServer::new(root_path, reason, handler);
\`\`\`

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5dc58e4 and 57431a1.

📒 Files selected for processing (3)

• .github/workflows/ci.yml (1 hunks)
• CHANGELOG.md (1 hunks)
• Cargo.toml (1 hunks)

🔇 Additional comments (2)

Cargo.toml (1)

3-3: Version bump aligns with breaking API changes.

The semver-major bump (0.6.0 → 0.7.0) is appropriate for the breaking change to DaemonServer::new() signature.

.github/workflows/ci.yml (1)

24-24: Verify intent of restricting clippy to ubuntu-latest.

The clippy job now runs only on ubuntu-latest instead of the matrix. Given that the codebase has platform-specific dependencies (nix for Unix, windows-sys for Windows), this change may miss platform-specific linting issues.

Confirm:

• Is this restriction intentional (e.g., all platform-specific code expected to compile cleanly via conditional compilation on ubuntu-latest)?
• Or should clippy retain the matrix strategy to catch platform-specific warnings?

If platform-specific warnings could occur, consider restoring the matrix for the clippy job.