Rust parity port: macOS auto theme-follow, 2 themes, agent-state bounds, singleton guard, fish spawn fix

[panosAthDBX]

Ports a set of features and fixes onto the Rust/ratatui runtime (v0.2.0-alpha.6). All changes are cross-platform (the macOS-only feature is cfg(target_os = "macos")-gated with no-op fallbacks). cargo test --workspace is green (16 new tests); no unrelated reformatting (the repo isn't rustfmt-enforced, so I matched the existing style).

Bug fix (affects all fish/csh users)

• fix(tmux): wrap the sidebar spawn command in sh -c. spawn_sidebar emitted a POSIX command (FOO=bar exec …; "${VAR:-default}") that tmux runs via the user's default-command/default-shell. Under a non-POSIX shell (e.g. fish) it fails to parse and the sidebar pane dies with status 127 before start.sh runs. Wrapping in sh -c '…' makes it shell-agnostic.

Feature: follow macOS system Appearance (auto dark/light theme)

• feat(config): add autoThemeFollowsSystem, darkTheme, lightTheme.
• feat(runtime): a system_theme module — appearance read (defaults read -g AppleInterfaceStyle), a pure mode→theme mapping, and a push-based watcher (notify on ~/Library/Preferences/.GlobalPreferences.plist + a 60s safety poll). macOS-gated; no-op elsewhere.
• feat(server): auto-switch the theme on appearance change (re-reads config each time so manual overrides win), and persist a manual set-theme to the per-appearance slot so dark and light remember independently. Opt-in (off by default; tests stay hermetic).

Feature: two new themes

• feat(themes): tokyo-night-storm and tango-adapted (light).

Robustness: bound long-running state

• feat(tracker): hard-prune unseen terminal agents after 15 min (previously unseen terminals were never pruned, so they accumulated indefinitely).
• feat(server): evict agent-watcher dedup-cache entries whose key hasn't appeared in a scan for 15 min (the last_seen map grew unbounded on a long-running server).
• feat(server): clean singleton guard — on a port conflict, probe the pid file and exit cleanly with "another server is already running (pid N)" instead of a bare AddrInUse; a stale (dead) pid file does not block startup.

Testing

• New unit/integration tests for themes, the two-tier prune, watcher eviction, config round-trip, the system_theme primitives, the singleton helpers, and the sh -c spawn wrapper.
• cargo test --workspace: all green.

This pull request and its description were written by Isaac.

[inspect-review]

inspect review

Triage: 66 entities analyzed | 0 critical, 0 high, 34 medium, 32 low
Verdict: standard_review

Findings (7)

1. [low] In prune_terminal, the cleared_unseen vector is populated but the corresponding unseen_instances entries are removed AFTER the session is potentially removed from instances. If the session becomes empty and is removed, the cleared_unseen keys are still removed from unseen_instances, but this happens after we've lost the reference to which session they belonged to. This creates a logic error where we're building session-qualified keys (format!("{session}\0{key}")) but the session variable is still in scope from the outer loop, so it works correctly. However, the code structure is confusing and error-prone.
2. [low] In spawn_sidebar for tmux_provider, the shell command escaping for single quotes in session names is incomplete. The code does inner.replace('\'', r"'\''") which escapes single quotes for the sh -c wrapper, but the OPENSESSIONS_SESSION_NAME value itself is not quoted in the inner command string. If a session name contains spaces or special shell characters, the command will break.
3. [low] The evict_stale_watcher_keys function has a potential logic error: it first retains entries in last_seen_at based on the time window, then retains entries in last_seen that exist in last_seen_at. However, if a key exists in last_seen but not in last_seen_at (which shouldn't happen in normal operation but could due to a bug), it would be silently removed without being within the eviction window. The maps should be kept in sync, but there's no assertion or guarantee of this invariant.
4. [low] In run_agent_watcher_loop, the code populates last_seen_at for all snapshots at the start of each tick, but then only updates last_seen fingerprints for non-Idle snapshots. This means Idle snapshots will have their last_seen_at timestamp updated (preventing eviction) but won't have a fingerprint in last_seen. This creates an inconsistency where last_seen_at can contain keys that last_seen doesn't have, violating the assumed invariant.
5. [low] In spawn_sidebar in tmux_provider.rs, the command construction uses single quotes to wrap the shell command, and escapes single quotes in the session name with '\''. However, the inner format string contains double quotes around ${OPENSESSIONS_DIR:-.} and the path, which will be inside single quotes. While this works, if target.session_name contains a single quote, the escape sequence '\'' will produce '\'' literally in the shell command string, which is correct. But the real issue is that window_id is interpolated directly without any escaping - if window_id contains single quotes or other shell metacharacters, the command could break or be exploited.
6. [low] In evict_stale_watcher_keys, the function first retains entries in last_seen_at based on the time window, then retains entries in last_seen that exist in last_seen_at. However, if a key exists in last_seen but not in last_seen_at (which should not happen in normal operation but could happen if the maps get out of sync), it will be removed from last_seen. This is likely intentional for cleanup, but the function signature and documentation don't make it clear that it enforces consistency between the two maps. This is not a bug per se, but the logic assumes the maps are always in sync.
7. [low] In run_agent_watcher_loop, the code updates last_seen_at for every snapshot in the scan, but this happens before checking if the snapshot status is Idle. This means even Idle snapshots update the last_seen_at timestamp, preventing their keys from being evicted. However, Idle snapshots are skipped in the fingerprint comparison logic (if snapshot.status == AgentStatus::Idle { continue; }), so their keys in last_seen will never be populated. This creates a memory leak: last_seen_at will accumulate keys for Idle snapshots that never get evicted because their timestamps keep getting refreshed, but they never appear in last_seen.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[inspect-review]

inspect review

Triage: 66 entities analyzed | 0 critical, 0 high, 34 medium, 32 low
Verdict: standard_review

Findings (7)

1. [low] In prune_terminal, the cleared_unseen vector is populated but the corresponding unseen_instances entries are only removed after the loop completes. However, if the session itself is removed (when empty is true), the cleared_unseen keys will still reference that session and attempt to remove entries that may no longer be relevant. More critically, the logic collects keys to remove from unseen_instances based on keys that were removed from session_instances, but this happens inside the if let Some(session_instances) block. If session_instances is None (session doesn't exist), the cleared_unseen cleanup still runs but with an empty vector, which is fine. However, the real bug is that cleared_unseen is scoped to each session iteration but the cleanup happens after potentially removing the session from self.instances, creating a logical inconsistency.
2. [low] In spawn_sidebar for tmux_provider, the command wraps the entire spawn command in sh -c '...' with single quotes, and attempts to escape single quotes in the session name with replace('\''', r"'\''"'). However, the replacement pattern is incorrect: it's replacing the literal string '\'' (backslash-single-quote-single-quote) instead of just ' (single quote). This means single quotes in session names won't be properly escaped, leading to shell injection vulnerabilities or command failures.
3. [low] In evict_stale_watcher_keys, the function retains entries in last_seen_at where now.saturating_sub(*seen_at) < evict_ms, then retains entries in last_seen only if the key exists in last_seen_at. However, if now is less than *seen_at (clock skew or time travel), saturating_sub returns 0, which is always < evict_ms, so the entry is kept. This is correct behavior for clock skew, but the function doesn't handle the case where now could be u64::MAX and overflow scenarios properly. More importantly, there's no bug here on second inspection - saturating_sub handles underflow correctly.
4. [low] In on_system_appearance_change, the function reads config from disk on every appearance change, which could fail (I/O error, malformed JSON). If load_config_from_home returns a default config due to an error, the function will silently use default theme names (catppuccin-mocha/latte) instead of the user's configured themes. While this might be intentional fallback behavior, it means a temporary I/O error will override the user's theme choice without any indication.
5. [low] In the spawn_sidebar command construction, the session name is wrapped in double quotes within the single-quoted sh command: OPENSESSIONS_SESSION_NAME="{}". If the session name contains a double quote character, it will break out of the double quotes and potentially cause command injection or syntax errors. The single-quote escaping only protects the outer sh -c layer, not the inner double-quoted session name.
6. [low] In evict_stale_watcher_keys, the function retains entries in last_seen_at based on age, then retains entries in last_seen only if they exist in last_seen_at. However, if a key exists in last_seen but not in last_seen_at (which can happen if the maps get out of sync), it will be removed from last_seen even though it was never checked for staleness. This asymmetry could cause premature eviction.
7. [low] In run_agent_watcher_loop, the code tracks last_seen_at only for non-Idle snapshots, but then calls evict_stale_watcher_keys which expects all keys in last_seen to have corresponding entries in last_seen_at. If a key transitions from non-Idle to Idle and back, or if there's any inconsistency, keys could exist in last_seen without entries in last_seen_at, causing incorrect eviction.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[panosAthDBX]

Addressed the inspect findings:

Findings 2 & 5 — unquoted values in the spawn command (fixed, 8665f06): OPENSESSIONS_SESSION_NAME / OPENSESSIONS_WINDOW_ID / REFOCUS_WINDOW are now double-quoted inside the sh -c command, so a session name containing spaces (or other word-splitting characters) no longer breaks the launcher. Single quotes remain escaped for the sh -c wrapper, so quotes in either the session name or window id are handled.

Findings 4 & 7 (and 3 & 6) — last_seen / last_seen_at consistency (fixed, f7ca09d): run_agent_watcher_loop now skips Idle snapshots when refreshing last_seen_at, so it tracks the same non-Idle key set the last_seen dedup cache holds. That removes the lingering-Idle-key inconsistency (4, 7) and establishes the shared-key invariant the eviction helper relies on (3, 6).

Finding 1 — prune_terminal structure (no change): the collect-into-cleared_unseen-then-remove pattern is required by the borrow checker — unseen_instances cannot be mutated while the self.instances.get_mut(&session) borrow is held, so the session-qualified keys are gathered and removed after that borrow ends. The logic is correct (the session binding stays in scope), as the finding itself notes.

cargo test --workspace is green (228 tests).

[inspect-review]

inspect review

Triage: 69 entities analyzed | 0 critical, 0 high, 35 medium, 34 low
Verdict: standard_review

Findings (6)

1. [low] In prune_terminal, the cleared_unseen vector is populated inside the filter closure but the closure only returns a boolean - the side effect of pushing to cleared_unseen happens during iteration but the keys being pushed don't match the keys being removed. The code pushes format!("{session}\0{key}") for ALL keys that pass the filter, but should only push for keys that are actually being removed (i.e., keys that will be in the keys vector after collection).
2. [low] In spawn_sidebar in tmux_provider.rs, the command wrapping logic has a critical shell escaping bug. The code escapes single quotes in target.session_name with replace('\'', r"'\'''"), but this produces incorrect shell syntax. The correct POSIX escape for a single quote inside single-quoted strings is '\'' (close quote, escaped quote, open quote), but the code produces '\''' which will fail to parse.
3. [low] In evict_stale_watcher_keys, there's a logic error in the retention order. The function first retains entries in last_seen_at based on age, then retains entries in last_seen based on whether their key exists in last_seen_at. However, if a key exists in last_seen but not in last_seen_at (which can happen if the maps get out of sync), it will be removed from last_seen even though it was never checked for staleness. The function should ensure both maps are consistent before pruning.
4. [low] In run_agent_watcher_loop, the code updates last_seen_at for all non-Idle snapshots before the deduplication check in last_seen. This means that if a snapshot is filtered out by the dedup logic (fingerprint unchanged), its timestamp in last_seen_at is still updated, preventing it from ever being evicted even if it stops appearing in scans. The timestamp update should only happen when the snapshot actually passes dedup and gets reported.
5. [low] In spawn_sidebar for tmux_provider, the command wrapping logic has a critical shell escaping bug. The code escapes single quotes in inner with replace('\'', r"'\'''"), but inner contains the session name which is interpolated into a double-quoted string. If the session name contains double quotes or other shell metacharacters, they will not be escaped, leading to command injection. The session name should be shell-escaped before being placed in the double-quoted context.
6. [low] In evict_stale_watcher_keys, the function retains entries in last_seen_at where now.saturating_sub(*seen_at) < evict_ms, but then retains entries in last_seen only if the key exists in last_seen_at. However, the logic for updating last_seen_at in run_agent_watcher_loop only inserts keys for non-Idle snapshots. This means if a snapshot transitions from non-Idle to Idle, its key will never be added to last_seen_at, but it may still exist in last_seen (if it was added before the Idle transition), causing an asymmetry where last_seen can contain keys not in last_seen_at.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[inspect-review]

inspect review

Triage: 72 entities analyzed | 0 critical, 0 high, 37 medium, 35 low
Verdict: standard_review

Findings (7)

1. [low] In prune_terminal, the variable cleared_unseen is populated but the corresponding entries are removed from self.unseen_instances AFTER the mutable borrow of self.instances ends. However, cleared_unseen is built from keys that match the filter condition, but those keys are removed from session_instances inside the mutable borrow. This means if a key is removed from session_instances, it gets added to cleared_unseen, but the logic doesn't guarantee that all keys in cleared_unseen should be removed from unseen_instances - only those that were actually pruned should be cleared from unseen.
2. [low] In evict_stale_watcher_keys, the function retains entries in last_seen_at where now.saturating_sub(*seen_at) < evict_ms, then retains entries in last_seen that exist in last_seen_at. However, in run_agent_watcher_loop, last_seen_at is updated for ALL snapshots (including those with status Idle), but last_seen only tracks non-Idle snapshots. This asymmetry means last_seen_at can contain keys that last_seen never had, causing the eviction logic to be ineffective for those keys.
3. [low] In spawn_sidebar for tmux_provider.rs, the command wraps the entire spawn command in sh -c '...' with single quotes, and escapes single quotes in the session name with '\''. However, the session name is only used in one place (OPENSESSIONS_SESSION_NAME="{}"), but the window_id appears in multiple places and is NOT escaped. If a window_id contains a single quote, it will break the shell command syntax.
4. [low] In kill_orphaned_sidebar_panes, the code builds window_session and windows_by_session maps by iterating through panes, but it only inserts into these maps when processing panes. If a window has only a sidebar pane, the logic checks session_has_other_windows by looking up the session in windows_by_session. However, the condition windows.len() > 1 checks if the session has more than one window, but this count includes the current window. The correct check should be whether there are OTHER windows besides the current one.
5. [low] In watch_mac_system_appearance (macOS version), the check closure is cloned and moved into two separate threads (file watcher and safety poll). Both threads call check() which locks last mutex. If both threads fire simultaneously, there's a potential race condition where both read the same last value, both see it's different from current mode, both update it, and both call on_change - resulting in duplicate callbacks for the same appearance change.
6. [low] In prune_terminal, the variable cleared_unseen is populated inside the filter closure but the closure only returns a boolean - the side effect of pushing to cleared_unseen happens during filtering, not during the subsequent removal loop. This means cleared_unseen will contain keys that were candidates for removal (matched the filter), not necessarily the keys that were actually removed. If a key exists in the filter but session_instances.remove(&key) fails for any reason, it will still be added to cleared_unseen and incorrectly removed from unseen_instances.
7. [low] In spawn_sidebar, the command wrapping logic escapes single quotes in inner with replace('\'', r"'\''")), but inner contains the session name which may have double quotes. The session name is wrapped in double quotes in the inner command but not escaped, allowing command injection if a session name contains a double quote followed by shell metacharacters.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[inspect-review]

inspect review

Triage: 75 entities analyzed | 0 critical, 0 high, 37 medium, 38 low
Verdict: standard_review

Findings (5)

1. [low] In prune_terminal, the variable cleared_unseen is populated but the corresponding entries are removed from self.unseen_instances AFTER the session is potentially removed from self.instances. If empty is true and the session is removed, the loop tries to remove unseen entries for a session that no longer exists in the tracker, but more critically, the unseen_instances cleanup happens outside the if let Some(session_instances) block, so it will attempt to remove entries even when session_instances was None.
2. [low] In evict_stale_watcher_keys, the function retains entries in last_seen_at where the age is less than evict_ms, then retains entries in last_seen that exist in last_seen_at. However, the last_seen_at map is updated in the agent watcher loop only for non-Idle snapshots, but last_seen can contain entries for any status that was previously non-Idle. If a key transitions from Running to Idle, it will remain in last_seen but stop getting updates in last_seen_at, causing it to eventually be evicted even though it might still be relevant.
3. [low] In spawn_sidebar, the sh_double_quote_escape function is called on session_name and window_id, but the result is placed inside double quotes in the inner command string. However, the entire inner string is then wrapped in single quotes for sh -c '...', and single quotes within that are escaped using '\''. This means if session_name or window_id contain single quotes, they will be escaped in the outer layer but NOT in the inner double-quoted layer, potentially breaking the command.
4. [low] In kill_orphaned_sidebar_panes, the code checks if a session has other windows by looking up window_session.get(&window_id) and then checking if windows_by_session for that session has length > 1. However, if the window_id is not in window_session (which should not happen but is not impossible), the and_then will return None and is_some_and will return false, causing the sidebar to be kept even when it should be killed.
5. [low] In prune_terminal, the logic for determining whether to prune an instance checks if it's unseen and applies a longer timeout (TERMINAL_HARD_PRUNE_MS), otherwise uses TERMINAL_PRUNE_MS. However, the cleared_unseen vector is populated for ALL pruned keys, not just the ones that were actually unseen. This means seen instances that are pruned will also have their (non-existent) unseen entries attempted to be removed, which is harmless but indicates a logic error.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes