Release v0.8.0: Add env var passing and theme detection

[akesson]

Summary

• Add per-command environment variable passing from client to daemon
• Add terminal theme detection (dark/light mode) with 50ms timeout

Breaking Changes

• CommandHandler::handle() signature changed: terminal_info: TerminalInfo → ctx: CommandContext
• Access terminal info via ctx.terminal_info

New Features

• EnvVarFilter for exact-match filtering of env var names
• CommandContext struct bundling terminal info + env vars
• DaemonClient::with_env_filter() builder method
• Theme enum (Dark, Light) and TerminalInfo.theme field

Test plan

• All 52 tests pass
• Pre-push hooks pass (fmt, clippy, nextest)

🤖 Generated with Claude Code

[coderabbitai]

📝 Walkthrough

Walkthrough

Releases v0.8.0 with breaking API changes: TerminalInfo is wrapped into a new CommandContext carrying terminal info and filtered env vars; EnvVarFilter is added; terminal theme detection (Theme) integrated; DaemonClient gains with_env_filter(); examples and tests updated.

Changes

Cohort / File(s)	Summary
Version & manifest CHANGELOG.md, Cargo.toml	Bumped package to 0.8.0, added changelog entry, and added terminal-colorsaurus = "0.4" dependency.
Core public API src/lib.rs	Added EnvVarFilter (filtering APIs) and CommandContext (wraps TerminalInfo + env_vars); updated prelude/exports; changed CommandHandler::handle to accept CommandContext.
Daemon client src/client.rs	Added private env_var_filter field, with_env_filter() builder, preserve filter across spawn/restart, compute filtered env vars and send CommandContext when executing commands.
Server & transport src/server.rs, src/transport.rs	SocketMessage::Command now carries context: CommandContext; server destructures context and passes CommandContext to handlers; logging updated to use context.terminal_info and env var counts.
Terminal & theme src/terminal.rs	Added public Theme enum (Dark/Light), TerminalInfo.theme: Option<Theme> with serde(default), runtime detect_theme() (50ms timeout) integrated into TerminalInfo::detect().
Examples examples/common/mod.rs, examples/concurrent.rs	Updated example handler signatures to accept CommandContext instead of TerminalInfo.
Tests & test harness src/tests.rs, tests/* (e.g., tests/integration_tests.rs, tests/version_tests.rs)	Updated test handlers and message construction to use CommandContext; extended TerminalInfo usage with theme; added tests for EnvVarFilter behaviors; adjusted socket/message tests to expect context.

Possibly related PRs

• Add terminal info support with theme detection (v0.4.0) #5: Modifies TerminalInfo/Theme and SocketMessage::Command payload—overlaps on terminal/theme and message-shape changes.
• Release v0.5.0: Add daemon force-stop functionality #6: Touches DaemonClient implementation and related configuration/builder methods, directly related to with_env_filter additions.
• Release v0.3.0: API simplification and exit code support #4: Changes the CommandHandler::handle API surface; intersects with the trait signature updates to handle.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main changes: adding environment variable passing and theme detection for version 0.8.0 release.
Description check	✅ Passed	The description is comprehensive and directly related to the changeset, detailing the breaking changes, new features, and confirming all tests pass.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch pass-envs

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (4)

src/terminal.rs (1)

15-18: Theme detection integration looks solid; consider future‑proofing the sync call

• Adding theme: Option<Theme> with #[serde(default)] keeps deserialization backward‑compatible while exposing theme data to handlers.
• Theme and detect_theme() are wired cleanly into TerminalInfo::detect(), and is_test_environment() correctly skips detection under cargo test / cargo-nextest to avoid hangs.
• The 50 ms timeout via QueryOptions bounds the worst‑case cost, but this synchronous call still briefly blocks the async runtime thread; if theme detection ever becomes heavier, you might consider offloading it via spawn_blocking and awaiting the result as an optional follow‑up refactor.

Overall, the design and safety checks (including not logging theme/env values elsewhere) look good.

Also applies to: 33-40, 49-50, 61-70, 92-118

src/client.rs (3)

37-53: EnvVarFilter integration is backward‑compatible and well‑scoped

• Adding env_var_filter: EnvVarFilter to DaemonClient and initializing it to EnvVarFilter::none() in both connect_with_name_and_timestamp and spawn_fresh_daemon preserves the previous behavior of sending no env vars by default.
• The field is kept private, so the only configuration surface is the new with_env_filter() API, which is a clean, controlled entry point.

One minor implementation detail: using EnvVarFilter::none() as the default is good; ensure Default (if implemented) matches that semantics for consistency across the type.

Also applies to: 201-210, 516-525

---

442-466: Restart correctly preserves env filter and restart settings

The restart() method now:

• Captures auto_restart_on_error and env_var_filter before replacing self with the freshly spawned client.
• Restores both settings on the new instance.

This guarantees that env var behavior and auto‑restart semantics survive daemon restarts. The mem::take(&mut self.env_var_filter) approach is fine here; alternatively, a simple clone would also work if EnvVarFilter: Clone.

---

638-667: CommandContext construction and env var passing look correct

• TerminalInfo::detect().await is still the single source for terminal metadata; the extra logging remains informative without exposing sensitive data.
• self.env_var_filter.filter_current_env() centralizes env selection. Only the count is logged, so values (and even names) are not leaked in logs.
• Building CommandContext::with_env(terminal_info, env_vars) and sending it via SocketMessage::Command { command, context } matches the server’s expectation and the transport enum.

This is a clean extension of the protocol; behavior is unchanged when env_var_filter is left at none().

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee84559 and 24c02ba.

📒 Files selected for processing (12)

• CHANGELOG.md (1 hunks)
• Cargo.toml (2 hunks)
• examples/common/mod.rs (1 hunks)
• examples/concurrent.rs (1 hunks)
• src/client.rs (7 hunks)
• src/lib.rs (8 hunks)
• src/server.rs (3 hunks)
• src/terminal.rs (4 hunks)
• src/tests.rs (6 hunks)
• src/transport.rs (2 hunks)
• tests/integration_tests.rs (9 hunks)
• tests/version_tests.rs (5 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

src/client.rs (1)

src/lib.rs (2)

• none (145-147)
• with_env (203-208)

src/tests.rs (1)

src/lib.rs (5)

• new (195-200)
• with_env (203-208)
• none (145-147)
• filter_current_env (166-174)
• with_names (150-154)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: test (windows-latest)

🔇 Additional comments (29)

Cargo.toml (1)

3-3: Version bump and new dependency look consistent

Version 0.8.0 and the terminal-colorsaurus = "0.4" dependency line up with the new Theme detection logic in src/terminal.rs and the documented breaking changes in the changelog. No issues from the manifest side.

Also applies to: 21-21

src/transport.rs (1)

1-1: SocketMessage now carries CommandContext end‑to‑end

Switching SocketMessage::Command to { command, context: CommandContext } and importing CommandContext keeps the transport layer aligned with the new handler API and the client’s CommandContext::with_env(...) usage. Serialization still goes through the enum, so as long as CommandContext: Serialize + Deserialize (which it should be), this is sound and version‑gated by the existing handshake.

Also applies to: 231-239

CHANGELOG.md (1)

8-25: 0.8.0 entry accurately documents the breaking/API changes

The new 0.8.0 section correctly calls out the CommandContext migration, env var passing + EnvVarFilter, DaemonClient::with_env_filter(), and theme detection with a 50 ms timeout and TerminalInfo.theme. This matches the actual code changes, so users have a clear upgrade path.

examples/common/mod.rs (1)

26-32: Example handler signature correctly updated to CommandContext

Updating the example CommandHandler::handle to take _ctx: CommandContext instead of TerminalInfo keeps it in sync with the trait while making it explicit that the context (terminal info + env vars) is available if needed. No functional issues here.

examples/concurrent.rs (1)

60-66: Concurrent example aligned with CommandContext‑based API

The TaskQueueHandler implementation now accepts _ctx: CommandContext, matching the updated CommandHandler trait and the server’s use of CommandContext. Logic is unchanged and remains correct.

src/server.rs (2)

45-57: Documentation example matches new CommandHandler signature

The example now shows handle(&self, command: &str, _ctx: CommandContext, ...), which correctly mirrors the updated trait and the runtime behavior. This keeps the docs trustworthy for users upgrading to 0.8.0.

---

305-335: Server correctly consumes CommandContext and preserves privacy

• Receiving SocketMessage::Command { command, context } and then passing context into handler.handle aligns with the new data model.
• The debug log pulls width/height/TTY/color info from context.terminal_info and only logs env_var_count rather than env var names or values, which is a good privacy baseline.
• The context is moved into the handler task after logging, so there’s no lifetime or ownership issue.

This end of the protocol looks coherent with the client and transport changes.

Also applies to: 314-321

src/client.rs (1)

556-579: with_env_filter() builder API is clear and composable

The with_env_filter(self, filter: EnvVarFilter) -> Self builder:

• Mirrors the style of with_auto_restart, so existing chaining patterns continue to feel natural.
• Makes it explicit that filtering is opt‑in, which matches the “backward compatible / pass nothing by default” design.

The usage example in the doc comment is helpful and matches the intended pattern.

tests/version_tests.rs (4)

19-29: LGTM! Handler signature correctly updated.

The SimpleHandler properly implements the updated CommandHandler::handle signature with CommandContext instead of TerminalInfo.

---

216-227: LGTM! TerminalInfo and CommandContext correctly constructed.

The test properly constructs TerminalInfo with the new theme field and wraps it in CommandContext::new() for the SocketMessage::Command payload.

---

266-277: LGTM! Consistent with the updated API.

Same pattern correctly applied for the "command without handshake" test case.

---

447-459: LGTM! Multiple command test correctly updated.

The test properly clones terminal_info when creating multiple CommandContext instances, which is necessary since CommandContext::new consumes the TerminalInfo.

src/lib.rs (5)

38-48: LGTM! Documentation example correctly updated.

The quick start example properly demonstrates the new CommandContext parameter in the handler signature.

---

119-175: LGTM! Well-designed EnvVarFilter API.

The implementation provides:

• Sensible default (no env vars passed)
• Builder pattern with include() for incremental construction
• with_names() for bulk initialization
• Efficient filtering that only collects matching vars

The documentation is clear with practical examples.

---

177-209: LGTM! CommandContext struct well-designed for extensibility.

Good design choices:

• #[serde(default)] on env_vars ensures backward compatibility with older clients/daemons
• Two constructors (new and with_env) provide clear API for common use cases
• Documentation notes the extensibility intent

---

267-275: LGTM! Prelude correctly exports all new public types.

EnvVarFilter, CommandContext, and Theme are properly added to the prelude for convenient access.

---

398-421: LGTM! Breaking change to CommandHandler trait properly implemented.

The trait signature change from terminal_info: TerminalInfo to ctx: CommandContext is a clean breaking change that bundles related context together. The documentation properly explains the new ctx parameter and its contents.

src/tests.rs (3)

18-30: LGTM! Test handler correctly updated.

The TestHandler implementation properly uses the new CommandContext parameter.

---

55-85: LGTM! Comprehensive serialization test for CommandContext.

The test properly verifies:

• SocketMessage::Command carries context: CommandContext
• Environment variables are correctly serialized/deserialized
• Terminal info fields are accessible via context.terminal_info

---

126-139: LGTM! Basic handler test updated correctly.

The test properly constructs CommandContext::new(terminal_info) and passes it to the handler.

tests/integration_tests.rs (9)

83-97: LGTM! EchoHandler correctly updated.

Handler signature properly changed to use CommandContext.

---

103-120: LGTM! ChunkedHandler correctly updated.

Consistent with the API change.

---

126-145: LGTM! CancellableHandler correctly updated.

Consistent with the API change.

---

151-163: LGTM! ErrorHandler correctly updated.

Consistent with the API change.

---

354-388: LGTM! ConcurrentTrackingHandler correctly updated.

Consistent with the API change.

---

783-800: LGTM! ImmediateOutputHandler correctly updated.

This handler tests the critical race condition fix for handler output/task completion, and is correctly updated to use CommandContext.

---

839-859: LGTM! LargeOutputHandler correctly updated.

Consistent with the API change.

---

895-907: LGTM! PanicHandler correctly updated.

Consistent with the API change.

---

1085-1099: LGTM! SlowHandler (nested) correctly updated.

Even the nested handler definition within test_connection_limit_immediate_rejection is correctly updated to use CommandContext.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/lib.rs (1)

303-338: Docstring for get_build_timestamp does not match implementation units

The docs say this returns the executable mtime “as seconds since Unix epoch”, but the implementation uses .as_millis() as u64, i.e., milliseconds. That mismatch could confuse callers doing arithmetic or comparisons based on the documented units.

I’d suggest either updating the doc to say “milliseconds since Unix epoch” or changing the implementation to .as_secs() to keep the API in seconds, depending on what existing callers expect.

🧹 Nitpick comments (5)

src/tests.rs (2)

55-85: Serde round-trip for SocketMessage::Command now exercises CommandContext and env vars

The updated test_socket_message_serialization correctly:

• Constructs TerminalInfo with the new theme field (Line 61),
• Builds a HashMap of env vars and wraps both in CommandContext::with_env (Lines 63–65),
• Verifies after serde that context.terminal_info fields and context.env_vars["TEST_VAR"] are preserved (Lines 73–82).

As an optional enhancement, you could also assert on context.terminal_info.theme by setting it to e.g. Some(Theme::Dark) and verifying it survives the round-trip, to cover the new enum-backed field more explicitly.

-    let terminal_info = TerminalInfo {
+    let terminal_info = TerminalInfo {
         width: Some(80),
         height: Some(24),
         is_tty: true,
         color_support: ColorSupport::Truecolor,
-        theme: None,
+        theme: Some(Theme::Dark),
     };
@@
-            assert_eq!(context.terminal_info.color_support, ColorSupport::Truecolor);
+            assert_eq!(context.terminal_info.color_support, ColorSupport::Truecolor);
+            assert_eq!(context.terminal_info.theme, Some(Theme::Dark));

---

187-219: EnvVarFilter tests cover core behaviors; consider making the missing-var test fully hermetic

The new tests:

• test_env_var_filter_none (Lines 189–192) verifies that EnvVarFilter::none() results in an empty map from filter_current_env(), which matches the “no env vars forwarded” semantics.
• test_env_var_filter_with_names (Lines 195–201) and test_env_var_filter_include (Lines 204–211) exercise filtering from mock iterators and validate both inclusion and map size.
• test_env_var_filter_missing_var (Lines 214–219) checks that filtering for a clearly non-existent name yields an empty map from filter_current_env().

Behavior-wise this looks correct and gives good coverage of none(), with_names(...), include(...), and filter_current_env().

If you want the tests to be completely independent of the process environment, you could make test_env_var_filter_missing_var use a mock iterator (like the other tests) instead of reading the real environment:

 #[test]
 fn test_env_var_filter_missing_var() {
-    // Filter for a var that doesn't exist
-    let filter = EnvVarFilter::with_names(["NONEXISTENT_VAR_12345"]);
-    let filtered = filter.filter_current_env();
+    // Filter for a var that doesn't exist in the provided mock env
+    let mock_env = [("EXISTING_VAR", "value")];
+    let filter = EnvVarFilter::with_names(["NONEXISTENT_VAR_12345"]);
+    let filtered = filter.filter_from(mock_env);
     assert!(filtered.is_empty());
 }

src/lib.rs (3)

35-47: API docs and examples are correctly updated to use CommandContext

The Quick Start examples and the CommandHandler trait docs now consistently use ctx: CommandContext, and the prose explains what ctx carries. This keeps copy‑paste examples in sync with the public API and clearly surfaces the new context abstraction. You might optionally extend one of the examples to show a simple ctx.terminal_info or ctx.env_vars access to advertise the new capabilities, but this is not required.

Also applies to: 61-71, 369-410, 420-423

---

120-141: EnvVarFilter behavior is correct; consider set-based lookup if filters grow

The filter behavior matches the docs: default passes nothing, with_names and include build up an explicit allow‑list, and filter_current_env delegates cleanly to filter_from. The exact‑match semantics on names are clear and predictable.

If you ever expect either a large names list or to run filter_from on large env maps in hot paths, you could switch names from Vec<String> to HashSet<String> (or precompute a HashSet internally) so the membership test in filter_from is O(1) instead of scanning self.names on every key. For current scale this is purely an optimization opportunity, not a correctness concern.

Also applies to: 143-190

---

192-224: CommandContext design and handler signature change look solid

CommandContext cleanly bundles TerminalInfo and filtered env vars, and #[serde(default)] on env_vars gives you backward compatibility when deserializing messages from older clients. The new / with_env constructors cover both common construction paths, and using ctx: CommandContext in CommandHandler::handle achieves the intended extensibility without further trait breaking changes.

You might later add convenience helpers like fn env(&self, key: &str) -> Option<&str> if you find repeated map access patterns in handlers, but the current API is perfectly fine as a first iteration.

Also applies to: 433-433

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 24c02ba and 3306e0d.

📒 Files selected for processing (2)

• src/lib.rs (8 hunks)
• src/tests.rs (6 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/tests.rs (1)

src/lib.rs (5)

• new (210-215)
• with_env (218-223)
• none (145-147)
• filter_current_env (187-189)
• with_names (150-154)

🔇 Additional comments (5)

src/tests.rs (4)

1-4: HashMap import correctly supports new env-var usage

use std::collections::HashMap; (Line 3) is appropriate and used in both the serialization test and env-var tests; no issues here.

---

18-30: CommandHandler test impl correctly updated to CommandContext

The CommandHandler for TestHandler impl now taking _ctx: CommandContext (Line 23) is consistent with the new public API. Using the leading underscore to mark it unused is idiomatic, and the test still validates the trait compiles.

---

121-139: test_handler_basic_output correctly migrated to CommandContext

Creating CommandContext::new(terminal_info) (Line 133) and passing it into handler.handle("test", ctx, &mut output, cancel) (Line 135) matches the updated handler signature while preserving the original behavior checks. The added theme: None field on TerminalInfo (Line 131) keeps the struct initialization in sync with the type definition.

---

141-186: Cancellation test remains sound with new context parameter

In test_handler_with_cancellation, the handler impl now takes _ctx: CommandContext (Line 152), and the test constructs a context via CommandContext::new(terminal_info) (Line 178) with theme: None added (Line 176). The cancellation flow and assertions are unchanged and still valid.

src/lib.rs (1)

103-104: Imports and public exports for context/theme types look consistent

Pulling in serde::{Deserialize, Serialize} and HashMap here is appropriate for CommandContext and EnvVarFilter, and re‑exporting Theme alongside ColorSupport/TerminalInfo plus adding EnvVarFilter, CommandContext, and Theme to the prelude keeps the public surface coherent. No issues from a correctness or API‑design standpoint.

Also applies to: 118-118, 283-286