Release v0.2.0: P0 seams + CORE-1 encoder + Phase 5B HealthBridge/XPC + master reconciliation - #16
Merged
Merged
Conversation
…device provider scaffold
A multi-agent review (docs/REVIEW_2026-06-18.md) found that several of the 33
Rules are implemented as correct modules but not enforced in the live path,
because the integration seams are missing or wrong. Adversarial verification
confirmed 21/25 critical-high findings. This lands the three P0 seam fixes plus
the Apple Foundation Models groundwork.
C1 — launchd socket activation (Rule 1, 0W idle):
daemon/main.py read systemd LISTEN_FDS/fromfd(3); launchd never sets that, so
macOS always fell to a dev rebind that races launchd's held socket. New
daemon/socket_activation.py acquires the listener via launch_activate_socket
("HarloCommand") → systemd → dev bind, and main.py only unlinks a dev-owned
socket on exit.
C2 — Swift<->daemon wire framing:
the Swift HealthBridge sends 4-byte length-prefixed frames; the daemon read
until newline, so biometric ingest never parsed. New daemon/framing.py makes
length-prefix the canonical wire (legacy newline auto-detected and answered in
kind), wired into daemon/main.py + cli/ipc.py.
C3 — biometric force_red -> Motor Cortex (Rule 28 + ADR-0001):
_handle_biometric_ingest computed force_red but nothing persisted it, and the
per-process tracker dies between activations. New modulation/state.py persists
ONLY derived scalars (Rule 9 key allow-list — never raw HR/HRV), with ADR-0001
freshness expiry, and apply_to_session_state surfaces it to the gate.
Foundation Models (#4): new provider/apple_fm.py (on-device, optional SDK) +
optional llm_provider path in inquiry/sincerity_gate.py (heuristic fallback
intact), demonstrating the local-reasoning swap for the S8 heuristic.
Tests: 24 new tests (framing incl. Swift-style frame, socket selection, derived-
state persistence + freshness + end-to-end gate inhibition, provider/sincerity
swap). Compliance greps clean (no sleep(/while True; biometric isolation intact;
Rule 11 guard untouched). Out of scope: C4 (wiring motor/DMN/escalate into the
live path) and #0 cross-encoder — tracked in the review doc.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
📝 WalkthroughWalkthroughFour verified critical issues are addressed through targeted rewiring: canonical length-prefixed IPC framing (C2), multi-platform socket activation with guarded dev fallback (C1), persisted derived biometric modulation state with freshness logic (C3), and Apple Foundation Models provider scaffold (C4). Cross-encoder trace guards prevent mismatched-encoder recalls in both Rust lexical and Python semantic paths. Respiratory-rate biometrics extend allostatic load computation. Sincerity classification gains optional LLM-judge delegation. XPC relay bridges the sandboxed HealthBridge to the daemon. Supporting documentation covers HealthKit provisioning, build environment gotchas, macOS ambient effector design and implementation plan, and a comprehensive architecture review with findings register. ChangesP0 Critical Fixes with Platform Wiring and Modulation Persistence
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/harlo/cli/ipc.py (1)
34-48:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSocket not closed on exception paths — potential resource leak.
If an exception occurs after
sock.connect()succeeds (e.g., duringsendall,read_message, or if aFramingErroris raised), the socket is never closed. Use atry/finallyor context manager to ensure cleanup.🐛 Proposed fix
try: sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) sock.settimeout(timeout) sock.connect(sock_path) - - # Canonical length-prefixed framing (matches the daemon + the Swift - # HealthBridge). The daemon replies in the same framing mode. - sock.sendall(framing.encode_frame({"command": command, "args": args})) - response, _mode = framing.read_message(sock) - sock.close() - return response + try: + # Canonical length-prefixed framing (matches the daemon + the Swift + # HealthBridge). The daemon replies in the same framing mode. + sock.sendall(framing.encode_frame({"command": command, "args": args})) + response, _mode = framing.read_message(sock) + return response + finally: + sock.close() except (ConnectionRefusedError, FileNotFoundError, socket.timeout, OSError): return None except framing.FramingError: return None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/harlo/cli/ipc.py` around lines 34 - 48, The socket resource is not being properly closed when exceptions occur after successful connection. In the current code, sock.close() is only called in the success path, but exceptions during sock.sendall(), framing.read_message(), or when a FramingError is raised will leave the socket open. Refactor this code to use a try/finally block that ensures sock.close() is always called regardless of whether an exception occurs, or alternatively use a context manager pattern with the socket to guarantee cleanup on all execution paths.
🧹 Nitpick comments (4)
docs/REVIEW_2026-06-18.md (1)
149-160: ⚡ Quick winLabel the Apple FM section as provisional.
These package, model-size, and API-capability claims are time-sensitive and read as confirmed facts. Please cite the source or mark this as research-only so the doc doesn’t age into misinformation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/REVIEW_2026-06-18.md` around lines 149 - 160, The Apple Foundation Models section presents specific technical claims about package availability, model sizes, context windows, and API capabilities as confirmed facts without proper attribution or provisional marking. Add a clear provisional label or disclaimer at the beginning of the section titled "The transformative one: Apple Foundation Models as a *local* reasoning provider" that indicates these claims are research-based and time-sensitive. Either provide citations for the package names, model specifications, and API details mentioned (such as the 3B model size, 8K context, Private Cloud Compute 32K tier, and OpenAI-compatible endpoint), or explicitly mark the section as research-only documentation to prevent the information from aging into misinformation.python/harlo/provider/apple_fm.py (1)
96-103: 💤 Low valueConsider validating context entries to prevent empty content.
_join_contextuses.get('content', '')which will silently produce empty lines like"user: "if a context message has no content. While the current callers may always provide valid content, defensive validation would make the helper more robust.♻️ Optional validation to filter empty content
def _join_context(prompt: str, context: list[dict] | None) -> str: if not context: return prompt lines = [ f"{m.get('role', 'user')}: {m.get('content', '')}" for m in context + if m.get('content', '').strip() ] lines.append(f"user: {prompt}") return "\n".join(lines)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/harlo/provider/apple_fm.py` around lines 96 - 103, The _join_context function currently allows context entries with missing or empty content to create empty lines in the output (like "user: "). Add validation to filter out context entries that have no content or empty content values before processing them in the list comprehension. This will prevent silent empty lines from being added to the joined output and make the function more robust against invalid input data.python/harlo/inquiry/sincerity_gate.py (2)
53-86: ⚡ Quick winBare
except Exceptionprovides fallback resilience but hides all errors.Line 80's bare exception catch ensures the heuristic fallback always succeeds, which aligns with the "optional LLM judge" design. However, it also silently swallows provider errors, JSON parsing failures, and any unexpected bugs. For observability, consider logging the exception before returning
None, or at minimum, distinguish between expected failures (bad JSON) and unexpected ones (provider bugs).♻️ Optional: Add minimal logging for debugging LLM judge failures
+ import logging + + logger = logging.getLogger(__name__) + try: raw = llm_provider.generate(prompt) start, end = raw.find("{"), raw.rfind("}") if start < 0 or end < 0: return None parsed = json.loads(raw[start : end + 1]) cls = SincerityClass(str(parsed["classification"]).lower().strip()) confidence = float(parsed.get("confidence", 0.6)) - except Exception: + except Exception as exc: + logger.debug("LLM judge failed, falling back to heuristic: %s", exc) return None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/harlo/inquiry/sincerity_gate.py` around lines 53 - 86, The bare exception handler in the _classify_via_llm function silently catches all errors without any logging, making it difficult to debug LLM judge failures. Add logging inside the except Exception block to capture and record the actual error details (using an appropriate logger) before returning None. This preserves the fallback behavior while improving observability for provider errors, JSON parsing failures, and unexpected bugs during LLM classification attempts.
74-77: ⚖️ Poor tradeoffJSON extraction using
find/rfindcould fail with multiple JSON objects.Lines 74-77 extract the first
{and last}from the LLM output, which works when there's a single JSON object. However, if the model returns multiple JSON objects (e.g.,{"a": 1} {"b": 2}), this will incorrectly parse{"a": 1} {"b": 2}(invalid JSON) instead of just the first object.In practice, the prompt explicitly requests "ONLY a JSON object," so multiple objects are unlikely. The current implementation is reasonable given the fallback behavior, but if LLM output becomes more unpredictable, consider using a regex to match balanced braces or iterating to find the first valid JSON.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/harlo/inquiry/sincerity_gate.py` around lines 74 - 77, The current implementation in the sincerity_gate.py file uses find() to locate the first opening brace and rfind() to locate the last closing brace, then extracts and parses that substring with json.loads(). This approach fails when the LLM output contains multiple JSON objects because it attempts to parse everything between the first `{` and the last `}` as a single JSON object, resulting in invalid JSON. To fix this, replace the simple find/rfind approach with a regex-based solution that matches balanced braces to extract only the first valid JSON object, or implement an iterative approach that attempts to parse progressively longer substrings until finding valid JSON. This ensures the code can gracefully handle cases where the LLM output contains multiple JSON objects by extracting and parsing only the first complete object.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/harlo/modulation/state.py`:
- Around line 123-126: In the freshness check for biometric_force_red around
line 123-126, strengthen the condition to treat force_red as stale not only when
the timestamp is older than the freshness threshold, but also when the timestamp
is missing or invalid. Currently the logic only sets force_red to False when
both state.updated_at exists AND exceeds the freshness_sec threshold; instead,
set force_red to False when either state.updated_at is missing/falsy OR when the
time delta exceeds freshness_sec. This defensive approach ensures that a
biometric_force_red without a valid timestamp cannot indefinitely persist and
inhibit motor commands.
In `@python/harlo/provider/apple_fm.py`:
- Around line 43-75: The reasoning parameter is accepted and stored in
self._reasoning in the __init__ method but is never actually used in the
implementation. According to Apple's Foundation Models documentation, the
reasoningLevel should be passed via ContextOptions to the respond() and stream()
methods on a per-request basis, not to the session constructor. Remove the
unused self._reasoning instance variable from __init__ or implement it properly
by passing it via ContextOptions when calling the respond() and stream() methods
(referenced at lines 84 and 92) to apply the reasoning level to each API
request.
---
Outside diff comments:
In `@python/harlo/cli/ipc.py`:
- Around line 34-48: The socket resource is not being properly closed when
exceptions occur after successful connection. In the current code, sock.close()
is only called in the success path, but exceptions during sock.sendall(),
framing.read_message(), or when a FramingError is raised will leave the socket
open. Refactor this code to use a try/finally block that ensures sock.close() is
always called regardless of whether an exception occurs, or alternatively use a
context manager pattern with the socket to guarantee cleanup on all execution
paths.
---
Nitpick comments:
In `@docs/REVIEW_2026-06-18.md`:
- Around line 149-160: The Apple Foundation Models section presents specific
technical claims about package availability, model sizes, context windows, and
API capabilities as confirmed facts without proper attribution or provisional
marking. Add a clear provisional label or disclaimer at the beginning of the
section titled "The transformative one: Apple Foundation Models as a *local*
reasoning provider" that indicates these claims are research-based and
time-sensitive. Either provide citations for the package names, model
specifications, and API details mentioned (such as the 3B model size, 8K
context, Private Cloud Compute 32K tier, and OpenAI-compatible endpoint), or
explicitly mark the section as research-only documentation to prevent the
information from aging into misinformation.
In `@python/harlo/inquiry/sincerity_gate.py`:
- Around line 53-86: The bare exception handler in the _classify_via_llm
function silently catches all errors without any logging, making it difficult to
debug LLM judge failures. Add logging inside the except Exception block to
capture and record the actual error details (using an appropriate logger) before
returning None. This preserves the fallback behavior while improving
observability for provider errors, JSON parsing failures, and unexpected bugs
during LLM classification attempts.
- Around line 74-77: The current implementation in the sincerity_gate.py file
uses find() to locate the first opening brace and rfind() to locate the last
closing brace, then extracts and parses that substring with json.loads(). This
approach fails when the LLM output contains multiple JSON objects because it
attempts to parse everything between the first `{` and the last `}` as a single
JSON object, resulting in invalid JSON. To fix this, replace the simple
find/rfind approach with a regex-based solution that matches balanced braces to
extract only the first valid JSON object, or implement an iterative approach
that attempts to parse progressively longer substrings until finding valid JSON.
This ensures the code can gracefully handle cases where the LLM output contains
multiple JSON objects by extracting and parsing only the first complete object.
In `@python/harlo/provider/apple_fm.py`:
- Around line 96-103: The _join_context function currently allows context
entries with missing or empty content to create empty lines in the output (like
"user: "). Add validation to filter out context entries that have no content or
empty content values before processing them in the list comprehension. This will
prevent silent empty lines from being added to the joined output and make the
function more robust against invalid input data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f80900cc-342e-451c-b622-a348f4eeac06
📒 Files selected for processing (13)
docs/REVIEW_2026-06-18.mdpython/harlo/cli/ipc.pypython/harlo/daemon/framing.pypython/harlo/daemon/main.pypython/harlo/daemon/router.pypython/harlo/daemon/socket_activation.pypython/harlo/inquiry/sincerity_gate.pypython/harlo/modulation/state.pypython/harlo/provider/apple_fm.pytests/test_daemon/test_framing.pytests/test_daemon/test_socket_activation.pytests/test_modulation/test_modulation_state.pytests/test_provider/test_apple_fm.py
Comment on lines
+123
to
+126
| force_red = state.biometric_force_red | ||
| if force_red and state.updated_at and (now - state.updated_at) > freshness_sec: | ||
| force_red = False # stale RED snapshot — cannot keep inhibiting motor | ||
| merged["biometric_force_red"] = force_red |
There was a problem hiding this comment.
Strengthen freshness check to handle missing timestamp.
The current freshness logic skips expiry when state.updated_at is falsy (e.g., 0 or None). If biometric_force_red=True is set but updated_at is missing (via manual JSON edit or a future write-path bug), the stale RED would persist indefinitely and continue inhibiting motor commands.
While the write path (router.py line 1034) always sets updated_at, defensive programming dictates that a force_red without a valid timestamp should be treated as stale.
🛡️ Proposed fix to expire force_red when timestamp is missing
force_red = state.biometric_force_red
- if force_red and state.updated_at and (now - state.updated_at) > freshness_sec:
+ if force_red and (not state.updated_at or (now - state.updated_at) > freshness_sec):
force_red = False # stale RED snapshot — cannot keep inhibiting motor
merged["biometric_force_red"] = force_red📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| force_red = state.biometric_force_red | |
| if force_red and state.updated_at and (now - state.updated_at) > freshness_sec: | |
| force_red = False # stale RED snapshot — cannot keep inhibiting motor | |
| merged["biometric_force_red"] = force_red | |
| force_red = state.biometric_force_red | |
| if force_red and (not state.updated_at or (now - state.updated_at) > freshness_sec): | |
| force_red = False # stale RED snapshot — cannot keep inhibiting motor | |
| merged["biometric_force_red"] = force_red |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/harlo/modulation/state.py` around lines 123 - 126, In the freshness
check for biometric_force_red around line 123-126, strengthen the condition to
treat force_red as stale not only when the timestamp is older than the freshness
threshold, but also when the timestamp is missing or invalid. Currently the
logic only sets force_red to False when both state.updated_at exists AND exceeds
the freshness_sec threshold; instead, set force_red to False when either
state.updated_at is missing/falsy OR when the time delta exceeds freshness_sec.
This defensive approach ensures that a biometric_force_red without a valid
timestamp cannot indefinitely persist and inhibit motor commands.
Comment on lines
+43
to
+75
| def __init__( | ||
| self, | ||
| model: str = "system", | ||
| system_prompt: str | None = None, | ||
| max_tokens: int = 1024, | ||
| reasoning: str | None = None, | ||
| ) -> None: | ||
| """Initialize the provider. | ||
|
|
||
| Args: | ||
| model: ``"system"`` (on-device) or ``"pcc"`` (Private Cloud Compute). | ||
| system_prompt: Optional instructions prepended to every request. | ||
| max_tokens: Response length cap (the on-device window is small). | ||
| reasoning: Optional PCC reasoning level (e.g. ``"light"``/``"deep"``). | ||
| """ | ||
| self._model = model | ||
| self._system_prompt = system_prompt | ||
| self._max_tokens = max_tokens | ||
| self._reasoning = reasoning | ||
| self._session = self._make_session() | ||
|
|
||
| def _make_session(self): | ||
| try: | ||
| import apple_fm # type: ignore | ||
| except ImportError as exc: # pragma: no cover - env-dependent | ||
| raise RuntimeError( | ||
| "apple-fm-sdk is not installed. On macOS 26+/27 run " | ||
| "`pip install apple-fm-sdk` to enable on-device reasoning." | ||
| ) from exc | ||
| return apple_fm.LanguageModelSession( | ||
| model=self._model, | ||
| instructions=self._system_prompt, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check apple_fm SDK documentation or examples for reasoning parameter usage
# Search the codebase for any apple_fm usage examples or documentation
rg -n -C3 'LanguageModelSession' --type=py
rg -n -C3 'reasoning.*pcc|pcc.*reasoning' --iglob '*.py' --iglob '*.md'Repository: JosephOIbrahim/Harlo
Length of output: 1145
🏁 Script executed:
cat -n python/harlo/provider/apple_fm.pyRepository: JosephOIbrahim/Harlo
Length of output: 4638
🏁 Script executed:
fd -type f -name '*test*apple*' -o -name '*apple*test*'Repository: JosephOIbrahim/Harlo
Length of output: 235
🌐 Web query:
apple_fm SDK LanguageModelSession reasoning parameter documentation
💡 Result:
In the Apple Foundation Models framework, reasoning capabilities are controlled via the reasoningLevel parameter, which is part of the ContextOptions object rather than a direct parameter of the LanguageModelSession itself [1][2]. When interacting with a model through a LanguageModelSession, you specify the reasoning behavior by passing a ContextOptions instance to the session's respond methods [1][2]. The reasoningLevel determines the extent to which the model engages in thinking or reasoning processes before generating a final response [1][2]. Key implementation details include: - ContextOptions: The reasoningLevel is a property within the ContextOptions structure, which manages request-specific configuration [1]. - Usage: You include it during a generation request as shown in this conceptual example [2]: let response = try await session.respond( to: "Your prompt here", contextOptions: ContextOptions(reasoningLevel:.moderate)) - Available Levels: Common reasoning levels include.light,.moderate, and.deep, though specific support may vary by the underlying model provider [2]. For developers implementing their own models via the Foundation Models framework, the LanguageModelCapabilities must explicitly include the.reasoning capability to support this feature [1]. During the execution of a response, you can access the configured level from the request object: let reasoningLevel = request.contextOptions.reasoningLevel [1]
Citations:
- 1: https://developer.apple.com/videos/play/wwdc2026/339/
- 2: https://firebase.google.cn/docs/ai-logic/apple-foundation-models-framework/configuration
🏁 Script executed:
fd -type f \( -name '*test*.py' -o -name '*_test.py' \) | xargs grep -l 'apple_fm\|AppleFoundationModelsProvider' 2>/dev/null || echo "No test files found"Repository: JosephOIbrahim/Harlo
Length of output: 273
🏁 Script executed:
rg -n 'ContextOptions|reasoningLevel' --type=pyRepository: JosephOIbrahim/Harlo
Length of output: 46
🏁 Script executed:
rg -n 'ContextOptions|reasoningLevel'Repository: JosephOIbrahim/Harlo
Length of output: 46
🏁 Script executed:
rg -n 'respond\(' python/harlo/provider/apple_fm.py -A2 -B2Repository: JosephOIbrahim/Harlo
Length of output: 303
The reasoning parameter is accepted but never used.
Lines 48 and 61 accept and store self._reasoning, but it is not passed to any SDK methods. According to Apple's Foundation Models documentation, reasoningLevel should be passed via ContextOptions to the respond() and stream() methods on a per-request basis, not to the session constructor. The current implementation (lines 84 and 92) does not include ContextOptions or any reasoning-related arguments.
This is an incomplete implementation. The docstring (line 56) documents that reasoning controls the PCC reasoning level ("light"/"deep"), but the parameter has no effect. The file acknowledges the SDK is beta (lines 19–22), but the stored parameter should either be used or removed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/harlo/provider/apple_fm.py` around lines 43 - 75, The reasoning
parameter is accepted and stored in self._reasoning in the __init__ method but
is never actually used in the implementation. According to Apple's Foundation
Models documentation, the reasoningLevel should be passed via ContextOptions to
the respond() and stream() methods on a per-request basis, not to the session
constructor. Remove the unused self._reasoning instance variable from __init__
or implement it properly by passing it via ContextOptions when calling the
respond() and stream() methods (referenced at lines 84 and 92) to apply the
reasoning level to each API request.
Tag every trace with its encoder (encoder_id column on the shared traces table; Rust store -> lexical, Python semantic_store -> semantic) and refuse cross-encoder Hamming comparison with a typed EncoderMismatch, raised from py_recall (Rust) and semantic_recall (Python, fail-fast before model load). Legacy rows COALESCE to lexical. Tests assert cross-encoder raises + same-encoder rank-1 recall. Ratified PASS-WITH-ENV-CAVEATS. Co-Authored-By: Claude Opus 4.8 <[email protected]>
DaemonWriter: hoist sun_path capacity out of the withUnsafeMutablePointer closure (Swift exclusivity violation blocked the build). Package.swift: exclude the entitlements file from the SwiftPM target. Bridge builds clean on the macOS 26.5 SDK (HealthKit available). Adds docs/HEALTHKIT_ACTIVATION.md (provisioning + Stage 2 build/sign/install runbook). Co-Authored-By: Claude Opus 4.8 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_encoder/test_semantic.py`:
- Line 413: The temporary database path is being created using tempfile.mktemp
which is insecure due to race conditions (CWE-377). Replace the mktemp call with
mkstemp in the db variable assignment. Since mkstemp returns a tuple containing
a file descriptor and the path, unpack the tuple appropriately (extracting the
path which is the second element) and ensure the file descriptor is properly
closed after obtaining the path to avoid resource leaks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 55b6f71c-7c5e-4c7d-9912-cde373e3a081
📒 Files selected for processing (11)
CLAUDE.mdcrates/hippocampus/src/encoder.rscrates/hippocampus/src/lib.rscrates/hippocampus/src/query.rscrates/hippocampus/src/store.rsdocs/HEALTHKIT_ACTIVATION.mdmacos/HarloHealthBridge/Package.swiftmacos/HarloHealthBridge/Sources/HarloHealthBridge/DaemonWriter.swiftpython/harlo/encoder/__init__.pytests/test_encoder/test_semantic.pytests/test_hippocampus/test_recall.py
✅ Files skipped from review due to trivial changes (2)
- macos/HarloHealthBridge/Package.swift
- CLAUDE.md
|
|
||
| monkeypatch.setattr(encoder_mod, "get_semantic_encoder", _tripwire) | ||
|
|
||
| db = tempfile.mktemp(suffix=".db") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify no insecure tempfile.mktemp usages remain in Python files.
rg -nP '\btempfile\.mktemp\s*\(' --type=pyRepository: JosephOIbrahim/Harlo
Length of output: 4148
Replace insecure temporary DB path creation.
Line 413 uses tempfile.mktemp, which is race-prone (CWE-377). Use mkstemp instead.
Suggested patch
- db = tempfile.mktemp(suffix=".db")
+ fd, db = tempfile.mkstemp(suffix=".db")
+ os.close(fd)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| db = tempfile.mktemp(suffix=".db") | |
| fd, db = tempfile.mkstemp(suffix=".db") | |
| os.close(fd) |
🧰 Tools
🪛 Ruff (0.15.17)
[error] 413-413: Use of insecure and deprecated function (mktemp)
(S306)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_encoder/test_semantic.py` at line 413, The temporary database path
is being created using tempfile.mktemp which is insecure due to race conditions
(CWE-377). Replace the mktemp call with mkstemp in the db variable assignment.
Since mkstemp returns a tuple containing a file descriptor and the path, unpack
the tuple appropriately (extracting the path which is the second element) and
ensure the file descriptor is properly closed after obtaining the path to avoid
resource leaks.
Source: Linters/SAST tools
Daemon data pipeline proven end-to-end on-device: a framed biometric_ingest (180 bpm) over twind.sock -> biometric_barrier -> AllostasisTracker -> {force_red, depleted, biometric_load:1.0}. config.py gains HARLO_SOCKET_PATH to relocate the command socket without moving twin.db. App icon added (Resources/AppIcon.icns + Info.plist CFBundleIconFile). Fix: xcodegen was clobbering Info.plist (stripping NSHealthShareUsageDescription + LSUIElement) -- dropped the info: block, on-disk Info.plist is now source of truth via INFOPLIST_FILE. DaemonWriter documents the dev socket path; HarloHealthBridge.dev.entitlements scaffolds a no-sandbox dev variant. Remaining Phase-5B task: sandbox-safe bridge<->daemon rendezvous (XPC/helper) -- the App Group container can't host the socket (daemon EPERM, verified). See docs/HEALTHKIT_ACTIVATION.md.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/harlo/daemon/config.py (1)
109-117:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDirectory creation doesn't handle overridden socket path.
The
ensure_data_dirs()function createsDATA_DIRand its subdirectories, but whenSOCKET_PATHis overridden viaHARLO_SOCKET_PATH(lines 61-69), the socket's parent directory may not exist, causing bind failures.See the proposed fix in the comment on lines 61-69.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/harlo/daemon/config.py` around lines 109 - 117, The ensure_data_dirs() function creates multiple data directories but does not create the parent directory for the SOCKET_PATH, which can be overridden via the HARLO_SOCKET_PATH environment variable. Add code in the ensure_data_dirs() function to also create the parent directory of SOCKET_PATH using the same pattern as the other directory creations (mkdir with parents=True and exist_ok=True), ensuring the socket's parent directory exists regardless of whether SOCKET_PATH uses the default value or is overridden.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/harlo/daemon/config.py`:
- Around line 61-69: When HARLO_SOCKET_PATH environment variable is set to
override the default socket path, the parent directory of that custom path may
not exist, causing the daemon to fail at bind time. Add logic to the
ensure_data_dirs() function to create the parent directory for SOCKET_PATH if it
does not already exist. This should handle both the case where HARLO_SOCKET_PATH
is provided (pointing outside DATA_DIR) and the default case where the socket
lives in DATA_DIR, ensuring the parent directory is created before the daemon
attempts to bind the socket.
---
Outside diff comments:
In `@python/harlo/daemon/config.py`:
- Around line 109-117: The ensure_data_dirs() function creates multiple data
directories but does not create the parent directory for the SOCKET_PATH, which
can be overridden via the HARLO_SOCKET_PATH environment variable. Add code in
the ensure_data_dirs() function to also create the parent directory of
SOCKET_PATH using the same pattern as the other directory creations (mkdir with
parents=True and exist_ok=True), ensuring the socket's parent directory exists
regardless of whether SOCKET_PATH uses the default value or is overridden.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea8f7960-92a6-4d57-86bc-ad6ea59babc1
📒 Files selected for processing (7)
docs/HEALTHKIT_ACTIVATION.mdmacos/HarloHealthBridge/Info.plistmacos/HarloHealthBridge/Resources/AppIcon.icnsmacos/HarloHealthBridge/Sources/HarloHealthBridge/DaemonWriter.swiftmacos/HarloHealthBridge/Sources/HarloHealthBridge/HarloHealthBridge.dev.entitlementsmacos/HarloHealthBridge/project.ymlpython/harlo/daemon/config.py
✅ Files skipped from review due to trivial changes (1)
- macos/HarloHealthBridge/Sources/HarloHealthBridge/HarloHealthBridge.dev.entitlements
🚧 Files skipped from review as they are similar to previous changes (1)
- macos/HarloHealthBridge/Sources/HarloHealthBridge/DaemonWriter.swift
Comment on lines
+61
to
+69
| # HARLO_SOCKET_PATH lets the command socket live OUTSIDE DATA_DIR — specifically | ||
| # in the App Group container shared with the sandboxed HarloHealthBridge, the one | ||
| # path both the sandboxed bridge and the (non-sandboxed) daemon can reach | ||
| # (ADR-0001 Phase 5B). twin.db and all other state stay in DATA_DIR. | ||
| SOCKET_PATH = ( | ||
| Path(os.environ["HARLO_SOCKET_PATH"]).expanduser() | ||
| if os.environ.get("HARLO_SOCKET_PATH") | ||
| else DATA_DIR / "twind.sock" | ||
| ) |
There was a problem hiding this comment.
Ensure parent directory exists for overridden socket path.
When HARLO_SOCKET_PATH points outside DATA_DIR, the daemon will fail to bind if the parent directory doesn't exist. This breaks the documented App Group container use case where the operator sets HARLO_SOCKET_PATH to a path that may not have been created yet.
🔧 Proposed fix to create socket parent directory
Add socket path parent creation to ensure_data_dirs():
def ensure_data_dirs():
"""Create data directories if they don't exist."""
DATA_DIR.mkdir(parents=True, exist_ok=True)
+ # Create parent directory for socket path if overridden outside DATA_DIR
+ SOCKET_PATH.parent.mkdir(parents=True, exist_ok=True)
STAGES_DIR.mkdir(parents=True, exist_ok=True)
DEFERRED_DIR.mkdir(parents=True, exist_ok=True)
REFLEX_DIR.mkdir(parents=True, exist_ok=True)
AGENTS_DIR.mkdir(parents=True, exist_ok=True)
AGENTS_QUEUE_DIR.mkdir(parents=True, exist_ok=True)
AGENTS_OUTPUTS_DIR.mkdir(parents=True, exist_ok=True)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/harlo/daemon/config.py` around lines 61 - 69, When HARLO_SOCKET_PATH
environment variable is set to override the default socket path, the parent
directory of that custom path may not exist, causing the daemon to fail at bind
time. Add logic to the ensure_data_dirs() function to create the parent
directory for SOCKET_PATH if it does not already exist. This should handle both
the case where HARLO_SOCKET_PATH is provided (pointing outside DATA_DIR) and the
default case where the socket lives in DATA_DIR, ensuring the parent directory
is created before the daemon attempts to bind the socket.
…endezvous)
Phase 5B sandbox rendezvous, built + proven. Bridge --NSXPCConnection--> com.harlo.xpc Mach service --> HarloXPCRelay (launchd, non-sandboxed) --framed JSON--> twind.sock --> daemon. HarloXPCRelay.swift vends the Mach service + forwards biometric_ingest; com.harlo.xpc.plist is its on-demand LaunchAgent (idle-exits, keeps Rule 1); DaemonWriter now calls the relay over XPC; entitlements add the mach-lookup temporary-exception. PROVEN end-to-end: client -> Mach service -> relay -> daemon -> AllostasisTracker -> {accepted:1, force_red:true, biometric_load:1.0}. Remaining: hardware-present live test (install bridge+relay, trigger a Watch sample). Production hardening -> SMAppService + caller code-sig validation. See docs/HEALTHKIT_ACTIVATION.md.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
…> daemon) Add the Apple Watch -> HealthBridge -> XPC relay -> daemon biometric-loop section + mermaid, a Phase 5B status line + table row, and Swift/HealthKit + XPC entries in the Tech Stack. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…ish prep) Make the XPC relay a real signed build artifact instead of a hand-compiled .swift: HarloXPCRelay is now an xcodegen tool target embedded into HarloHealthBridge.app/Contents/Helpers/ (where com.harlo.xpc.plist expects it) and signed by the build; the app signature seals the nested relay (codesign --verify passes). Renamed the relay entry to main.swift (xcodebuild requires top-level code there). README Phase 5B status -> built; gitignore session scratch. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Add a 'make build-healthbridge' target (xcodegen + xcodebuild -> HarloHealthBridge.app with the embedded HarloXPCRelay; HB_CODESIGN_ID overridable for Developer-ID release). Add a conservative [tool.ruff] config (F + high-signal E rules) so the lint barrier has a tool; Rust stays on cargo clippy. Co-Authored-By: Claude Opus 4.8 <[email protected]>
JosephOIbrahim
changed the title
…ealthBridge/XPC Reconciles origin/master (biometric_prior session-seed, HarloPulse iPhone sidecar/ADR-0002, src→engine reorg, ADR-0003 days-decay, D60/D69/D72 daemon hardening) with the Phase 5B branch (CORE-1 encoder tagging + EncoderMismatch, C1/C2/C3 IPC seams, sandboxed HealthBridge -> XPC relay -> daemon loop). Conflict resolutions (6 files): - daemon/socket_activation.py: UNION both acquisition APIs. master's acquire_listening_socket (D69 live-listener guard, owns_node) is the production path; acquire_listen_socket (source label) retained. Both test files pin their respective API. - daemon/main.py: master's serve()/run_socket_activated() (D72 rude-client survival, Rule-19 teardown preempt, idle_shutdown, D69 owns_node) + framing handle_client (C2). The XPC relay reads a length-prefixed reply (readN(fd,4)->readN(fd,n)), so framing's reply-in-same-mode is load-bearing; a newline-only reply would break the shipped relay. - daemon/router.py: biometric_ingest writes the derived verdict to BOTH modulation stores -- state.py (C3, motor path: biometric_force_red -> basal_ganglia) AND state_store.py (D60, status/coach surface). Complementary consumers; Rule 9 honored (derived scalars only, never raw HR/HRV). - test_daemon/test_wire_framing.py: adapted to read replies via the canonical framing reader (master's _recv_request superseded). Edge cases now assert a bounded error frame -- same safety property. - README.md / .gitignore: union. HealthBridge (macOS/XPC) documented as the Mac-local sibling to the HarloPulse iPhone (TCP) loop. Verification: cargo 43 ok; per-group fresh-interpreter run 42/45 green incl. all merge-critical paths (test_daemon 75, test_modulation 65, test_biometric_prior 55, test_integration 79). The 3 non-green are pre-existing and merge-independent: test_schedule clock-substitution path is byte-identical to master, test_mcp native model-load segfault, test_path_d empty. Rebuilt hippocampus for ADR-0003 days-decay. CLAUDE.md compliance greps pass (Rule 1/2/9/11). Co-Authored-By: Claude Opus 4.8 <[email protected]>
…pp · bridge First unified version after the master reconciliation merge. Single product version 0.2.0 across every surface, replacing the drift (0.1.6 core/Rust/app, 0.1.5 HealthBridge, 0.1.0 project.yml MARKETING_VERSION, 1.0 HarloPulse plist): - pyproject.toml, crates/hippocampus/Cargo.toml (+ Cargo.lock), harlo.__version__ - macos/Harlo.app, macos/HarloHealthBridge, ios/HarloPulse Info.plist (CFBundleShortVersionString; CFBundleVersion build counters left at 1) - HealthBridge + HarloPulse project.yml MARKETING_VERSION - README banner v0.2.0 milestone line Co-Authored-By: Claude Opus 4.8 <[email protected]>
…ED path) Roadmap #1 from the HealthKit audit. respiratory_rate already crossed the biometric_barrier but contributed nothing to the state machine. It now feeds get_biometric_load (the DEPLETED trend) but NOT should_force_red — RR is a slow signal (Apple samples it mainly during sleep/rest), too coarse for the fresh-only RED motor-inhibition path. Scoring is excess-over-floor, not mean/threshold: only RR above the normal resting floor (16 br/min default) adds load, ramping to 1.0 at the red mark (25 br/min). Normal breathing contributes zero — RR has a high physiological baseline, so the HR-style mean/threshold shape would wrongly inflate load. Thresholds are constructor params (rr_floor_bpm / rr_red_bpm) so intake calibration can personalize them, exactly like hr_red_bpm / hrv_red_ms. TDD, 5 tests: elevated->saturated, normal->zero, midpoint->~0.5, high->DEPLETED, high->NOT force_red. Rule 9 intact (derived load only; raw RR never persisted). Co-Authored-By: Claude Opus 4.8 <[email protected]>
…torm approved) First Motor Cortex effector on macOS: live DEPLETED state drives ambient OS adjustments through the existing premotor -> basal_ganglia -> executor path. Three-band ownership model (Harlo decides / Tier 2 / Locked) + five-part test + four safety rails, mapped onto ConsentLevel. DEPLETED triggers, RED blocks (via execute_one's RED-halt). Phased 2->3->1 with graduated autonomy as the Phase-2 North Star. Filed under docs/design/ (docs/superpowers/specs is gitignored). No code yet — pending spec review, then writing-plans. Co-Authored-By: Claude Opus 4.8 <[email protected]>
- Tier-2 UX: nudge-to-confirm by default (the nudge IS the PER_ACTION consent); pre-grant via profile or earned via Phase-2 graduated autonomy. - Lease: state-recovery first + 90-min backstop; revert also fires on session teardown and next daemon startup. No setting left silently changed across sessions (§7 updated). - Transparency: notification with [keep]/[undo]/[stop] at apply-time for v1; menu-bar indicator deferred to Phase 2 (YAGNI). Co-Authored-By: Claude Opus 4.8 <[email protected]>
9 bite-sized TDD tasks turning the approved design into code: consent registration, Actuator interface + FakeActuator, lease store, handlers, config (default OFF), pure edge-detected trigger logic, router wiring (RED-halts via execute_one), MacOSActuator (shortcuts run + osascript), and a manual Mac integration runbook. Actuation is hidden behind an injectable interface so all trigger/gate/lease logic is pure-Python and TDD-able. Two §5 rails (interactive undo buttons, back-off-on-override) explicitly deferred to Phase 2 with rationale. Co-Authored-By: Claude Opus 4.8 <[email protected]>
scripts/rotate_anthropic_key.sh — one-shot, safe ANTHROPIC_API_KEY rotation: prompts for the new key (hidden, never in shell history), validates it against the API before changing anything, sets the launchctl session env + a chmod-600 login agent for persistence, bounces the daemon, smoke-tests. Never prints the key. Local key surface confirmed = launchctl env only (no profiles/keychain). README hero rewritten to lead with the layman pitch (matches the GitHub description) instead of opening on USD composition jargon. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Reflect the shipped respiratory_rate scoring (fe94e8e): RR added to the Apple Watch signal nodes in both biometric diagrams, and the AllostasisTracker node now shows HR · HRV · RR -> load. Co-Authored-By: Claude Opus 4.8 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
44-61:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
build-healthbridgetohelpoutput.The new target is callable but currently not discoverable via
make help, which makes Phase 5B build flow easy to miss.Suggested patch
help: `@echo` "Harlo build targets" @@ `@echo` " verify test + compliance-greps + doctor + signing-readiness" + `@echo` " build-healthbridge Build HarloHealthBridge.app + embedded HarloXPCRelay (macOS only)" `@echo` " regen-trajectories Regenerate data/trajectories_10k.jsonl (~229 MB, ~minutes)"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 44 - 61, Add documentation for the build-healthbridge target to the help output in the Makefile. In the help target, add a new `@echo` line that describes what the build-healthbridge target does. Place this new line in a logical position among the other build targets (such as after the build-rust or build-macos lines) to ensure the Phase 5B build flow is discoverable when users run make help.
🧹 Nitpick comments (6)
docs/design/2026-06-19-macos-ambient-effectors-plan.md (2)
891-909: ⚡ Quick winOverly broad exception handling risks silent failures in ambient effectors.
The
except Exception: passblock at lines 908–909 is intentionally broad for "advisory" behavior, but swallowing all exceptions without even logging them may hide setup errors (config loading, lease-path issues) that should be visible during testing and debugging.Consider adding at least a debug log or a narrow exception list for Phase 1. For production, this can remain permissive, but during development, visibility is helpful.
🔍 Example: add debug logging
try: ... except Exception as e: # noqa: BLE001 import logging logging.debug(f"ambient effectors advisory skip: {e}")Alternatively, narrow to expected exceptions:
except (FileNotFoundError, yaml.YAMLError, KeyError): pass🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/2026-06-19-macos-ambient-effectors-plan.md` around lines 891 - 909, The bare except Exception: pass block that follows the run_ambient_effectors call silently swallows all exceptions without any logging, which hides setup errors like config loading failures or lease-path issues that should be visible during debugging. Add at least a debug log statement that captures and logs the exception details to improve visibility during development, or alternatively narrow the exception handling to only catch specific expected exceptions like FileNotFoundError, yaml.YAMLError, or KeyError rather than catching all exceptions broadly.
129-129: 💤 Low valueCapitalize "macOS" consistently (LanguageTool hint).
The operating system from Apple is capitalized as "macOS," not "Mac OS" or "MAC OS". Found at lines 129, 1000, 1001.
Also applies to: 1000-1000, 1001-1001
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/2026-06-19-macos-ambient-effectors-plan.md` at line 129, The document contains inconsistent capitalization of Apple's operating system name. Replace all instances of "macos" with "macOS" throughout the document, specifically at line 129 in the macos.install() reference, and at lines 1000 and 1001 where the operating system name appears. Ensure the proper capitalization "macOS" is used consistently wherever the operating system is mentioned in the documentation.Source: Linters/SAST tools
python/harlo/daemon/main.py (1)
136-140: ⚡ Quick winLog DMN teardown join failures instead of swallowing them.
The current
except Exception: passhides shutdown-path failures and makes teardown regressions hard to diagnose.Suggested patch
try: from .dmn_teardown import get_teardown get_teardown().join_with_budget(timeout=float(DMN_BUDGET_S)) - except Exception: - pass + except Exception: + _LOGGER.warning("dmn teardown join failed", exc_info=True)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/harlo/daemon/main.py` around lines 136 - 140, The except block in the DMN teardown section silently swallows all exceptions using 'except Exception: pass', preventing visibility into shutdown failures. Replace the pass statement with appropriate logging of the caught exception. Log the error when the get_teardown().join_with_budget(timeout=float(DMN_BUDGET_S)) call fails so that shutdown-path issues and teardown regressions can be properly diagnosed instead of being hidden.Source: Linters/SAST tools
scripts/rotate_anthropic_key.sh (1)
51-65: 💤 Low valueConsider escaping XML special characters for robustness.
Direct heredoc interpolation of
$NEWKEYinto XML would malform the plist if the key ever contained&,<, or>. Anthropic keys use a safe charset today (sk-ant-*with base64-ish suffix), so practical risk is near-zero, but defensive escaping is trivial:♻️ Optional defensive fix
+# Escape XML special chars (defensive; Anthropic keys don't contain these today) +ESCAPED_KEY="${NEWKEY//&/&}" +ESCAPED_KEY="${ESCAPED_KEY//</<}" +ESCAPED_KEY="${ESCAPED_KEY//>/>}" + cat > "$AGENT" <<PLIST ... - <string>$NEWKEY</string> + <string>$ESCAPED_KEY</string> ... PLIST🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/rotate_anthropic_key.sh` around lines 51 - 65, The `$NEWKEY` variable is directly interpolated into the XML plist content without escaping XML special characters. While current Anthropic keys are safe, if the key ever contained characters like `&`, `<`, or `>`, it would malform the plist. Create a variable that escapes these XML special characters by replacing `&` with `&`, `<` with `<`, `>` with `>`, and quote marks as needed, then use this escaped variable instead of `$NEWKEY` in the heredoc that builds the plist file.macos/HarloXPCRelay/main.swift (1)
93-100: ⚖️ Poor tradeoffNo client code-signature validation in XPC listener.
shouldAcceptNewConnectionaccepts all incoming connections without verifying the client's code signature. Any process with access to the Mach namespace can invokeingestand push arbitrary payloads toward the daemon.The docs note this as future hardening. For Phase 5B this is likely acceptable, but before production, validate
conn.auditTokenorconn.effectiveUserIdentifierto ensure only the legitimate HarloHealthBridge can connect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@macos/HarloXPCRelay/main.swift` around lines 93 - 100, The shouldAcceptNewConnection method in the Delegate class currently accepts all incoming XPC connections without validating the client. Add validation logic to check the connection's auditToken or effectiveUserIdentifier to ensure only the legitimate HarloHealthBridge process can connect. Verify the client credentials before exporting the interface and resuming the connection, and return false to reject any unauthorized connection attempts.docs/HEALTHKIT_ACTIVATION.md (1)
129-132: 💤 Low valueAdd language specifiers to fenced code blocks.
The architecture diagram (line 129) and build instructions (line 149) are missing language specifiers.
📝 Proposed fix
Line 129: Add
textor leave as-is (ASCII diagram)-``` +```text HarloHealthBridge (sandboxed) --NSXPCConnection--> com.harlo.xpc (Mach service)Line 149: Add
bash-``` +```bash swiftc -O macos/HarloXPCRelay/main.swift -o <relay path> # e.g. app Helpers/Also applies to: 149-155
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/HEALTHKIT_ACTIVATION.md` around lines 129 - 132, The markdown document contains fenced code blocks that are missing language specifiers, which reduces readability and syntax highlighting in rendered documentation. Add the `text` language specifier to the opening backticks of the fenced code block containing the architecture diagram that shows the HarloHealthBridge and NSXPCConnection flow, and add the `bash` language specifier to the opening backticks of the fenced code block containing the build instructions with the swiftc compilation command. This will enable proper formatting and syntax highlighting when the documentation is rendered.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/2026-06-19-macos-ambient-effectors-plan.md`:
- Around line 900-910: The function call to run_ambient_effectors is passing a
keyword argument with the wrong name. The function signature defines the
parameter as cognitive_state, but the call is passing cognitive=cognitive.
Change the keyword argument name from cognitive to cognitive_state in the
run_ambient_effectors function call to match the expected parameter name in the
function signature.
- Around line 883-928: The issue is that `_handle_biometric_ingest` computes and
writes the new depleted state without first capturing the prior depleted state
from the persisted modulation state, making it unavailable for the ambient
effectors logic. Add a read of the existing persisted modulation state at the
very top of `_handle_biometric_ingest` (before any existing writes occur and
before line 1036) using something like `read_modulation_state(...)`, extract the
`is_depleted` value from that prior state into a `prior_depleted` variable, and
ensure this variable is available to the `run_ambient_effectors()` call in the
try-block so that the ambient effectors receive the depletion state before it
was overwritten by the current ingest.
In `@docs/design/2026-06-19-macos-ambient-effectors.md`:
- Around line 55-64: The fenced code block in the design document is missing a
language identifier, which violates the MD040 markdown linting rule. Locate the
opening triple backticks (```) that precede the ASCII tree diagram showing the
biometric_ingest flow with AllostasisTracker and executor.execute_one(), and add
the language identifier text immediately after the backticks so it reads ```text
instead of just ```. This applies only to the opening fence, not the closing
one.
In `@docs/HEALTHKIT_ACTIVATION.md`:
- Around line 140-142: The documentation for HarloHealthBridge.entitlements
shows the mach service name as abbreviated `com.harlo.xpc` but the actual
entitlement value includes a team ID prefix. Update the documentation line
showing the `com.apple.security.temporary-exception.mach-lookup.global-name`
entitlement value to display the complete service name
`233JSS4X69.com.harlo.xpc` instead of the abbreviated `com.harlo.xpc` to
accurately reflect the actual code implementation.
- Around line 135-136: The documentation incorrectly references the relay source
file as HarloXPCRelay.swift when the actual filename is main.swift. Update all
references to the relay file from HarloXPCRelay.swift to main.swift, including
both the description of the relay around line 135-136 where it mentions the Mach
service and NSXPCListener, as well as the build command reference on line 150
that uses the incorrect filename.
In `@macos/HarloHealthBridge/Sources/HarloHealthBridge/DaemonWriter.swift`:
- Around line 35-43: The cast of remoteObjectProxyWithErrorHandler to
HarloXPCProtocol on line 38 can fail silently, resulting in proxy being nil,
which causes the ingest call to do nothing and the connection to never be
invalidated. Add an explicit check after the cast to verify that proxy is not
nil. If proxy is nil, log an error message indicating the cast failed and
immediately invalidate the connection. Only proceed with the ingest call if
proxy is not nil. This ensures that all code paths properly clean up the
connection and provide visibility into cast failures.
In `@macos/launchd/com.harlo.xpc.plist`:
- Around line 29-33: The ProcessType Adaptive setting in the plist does not
automatically terminate idle services—it only provides hints to the scheduler.
The HarloXPCRelay implementation currently runs RunLoop.main.run() without any
XPC transaction tracking or idle-exit logic, causing the service to remain alive
indefinitely. To achieve true idle-exit compliance, modify the HarloXPCRelay to
wrap its work operations in xpc_transaction_begin and xpc_transaction_end calls
to signal activity periods to launchd, or alternatively implement an explicit
idle timeout mechanism that exits the process after the specified idle period
with no active transactions.
In `@python/harlo/daemon/router.py`:
- Around line 1066-1081: The modulation state persistence is gated behind the
accepted condition and swallows database errors silently, which can cause SQLite
state to diverge from JSON state and leave surfaces out of sync with
motor-gating inputs. Remove the if accepted: gate wrapping the
write_modulation_state_db call so persistence happens consistently regardless of
acceptance status, and replace the silent exception handling (except Exception:
pass) with proper error surfacing or logging to ensure database failures are
visible rather than hidden, maintaining consistency between state stores per the
coding guideline to surface problems rather than skip them.
In `@python/harlo/modulation/allostatic.py`:
- Around line 84-93: Add validation logic in the __init__ method to ensure that
rr_red_bpm is strictly greater than rr_floor_bpm at construction time. After the
parameter assignments for rr_floor_bpm and rr_red_bpm, insert a check that
raises a ValueError with a descriptive message if rr_red_bpm is less than or
equal to rr_floor_bpm. This will fail fast and prevent silent misconfiguration
where the RR contribution gets zeroed out later in the code.
In `@README.md`:
- Around line 31-42: The fenced code block containing the PRODUCTION LIVE status
information is missing a language specifier on the opening fence. Add a language
hint such as `text` immediately after the opening triple backticks (change ` ```
` to ` ```text `) to satisfy the markdownlint MD040 rule and keep the
documentation lint clean.
---
Outside diff comments:
In `@Makefile`:
- Around line 44-61: Add documentation for the build-healthbridge target to the
help output in the Makefile. In the help target, add a new `@echo` line that
describes what the build-healthbridge target does. Place this new line in a
logical position among the other build targets (such as after the build-rust or
build-macos lines) to ensure the Phase 5B build flow is discoverable when users
run make help.
---
Nitpick comments:
In `@docs/design/2026-06-19-macos-ambient-effectors-plan.md`:
- Around line 891-909: The bare except Exception: pass block that follows the
run_ambient_effectors call silently swallows all exceptions without any logging,
which hides setup errors like config loading failures or lease-path issues that
should be visible during debugging. Add at least a debug log statement that
captures and logs the exception details to improve visibility during
development, or alternatively narrow the exception handling to only catch
specific expected exceptions like FileNotFoundError, yaml.YAMLError, or KeyError
rather than catching all exceptions broadly.
- Line 129: The document contains inconsistent capitalization of Apple's
operating system name. Replace all instances of "macos" with "macOS" throughout
the document, specifically at line 129 in the macos.install() reference, and at
lines 1000 and 1001 where the operating system name appears. Ensure the proper
capitalization "macOS" is used consistently wherever the operating system is
mentioned in the documentation.
In `@docs/HEALTHKIT_ACTIVATION.md`:
- Around line 129-132: The markdown document contains fenced code blocks that
are missing language specifiers, which reduces readability and syntax
highlighting in rendered documentation. Add the `text` language specifier to the
opening backticks of the fenced code block containing the architecture diagram
that shows the HarloHealthBridge and NSXPCConnection flow, and add the `bash`
language specifier to the opening backticks of the fenced code block containing
the build instructions with the swiftc compilation command. This will enable
proper formatting and syntax highlighting when the documentation is rendered.
In `@macos/HarloXPCRelay/main.swift`:
- Around line 93-100: The shouldAcceptNewConnection method in the Delegate class
currently accepts all incoming XPC connections without validating the client.
Add validation logic to check the connection's auditToken or
effectiveUserIdentifier to ensure only the legitimate HarloHealthBridge process
can connect. Verify the client credentials before exporting the interface and
resuming the connection, and return false to reject any unauthorized connection
attempts.
In `@python/harlo/daemon/main.py`:
- Around line 136-140: The except block in the DMN teardown section silently
swallows all exceptions using 'except Exception: pass', preventing visibility
into shutdown failures. Replace the pass statement with appropriate logging of
the caught exception. Log the error when the
get_teardown().join_with_budget(timeout=float(DMN_BUDGET_S)) call fails so that
shutdown-path issues and teardown regressions can be properly diagnosed instead
of being hidden.
In `@scripts/rotate_anthropic_key.sh`:
- Around line 51-65: The `$NEWKEY` variable is directly interpolated into the
XML plist content without escaping XML special characters. While current
Anthropic keys are safe, if the key ever contained characters like `&`, `<`, or
`>`, it would malform the plist. Create a variable that escapes these XML
special characters by replacing `&` with `&`, `<` with `<`, `>` with
`>`, and quote marks as needed, then use this escaped variable instead of
`$NEWKEY` in the heredoc that builds the plist file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62fb3d9b-dd7c-42f4-854a-7d69a2b51032
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
.gitignoreCLAUDE.mdMakefileREADME.mdcrates/hippocampus/Cargo.tomlcrates/hippocampus/src/store.rsdocs/HEALTHKIT_ACTIVATION.mddocs/design/2026-06-19-macos-ambient-effectors-plan.mddocs/design/2026-06-19-macos-ambient-effectors.mdios/HarloPulse/Info.plistios/HarloPulse/project.ymlmacos/Harlo.app/Contents/Info.plistmacos/HarloHealthBridge/Info.plistmacos/HarloHealthBridge/Sources/HarloHealthBridge/DaemonWriter.swiftmacos/HarloHealthBridge/Sources/HarloHealthBridge/HarloHealthBridge.entitlementsmacos/HarloHealthBridge/project.ymlmacos/HarloXPCRelay/main.swiftmacos/launchd/com.harlo.xpc.plistpyproject.tomlpython/harlo/__init__.pypython/harlo/daemon/config.pypython/harlo/daemon/main.pypython/harlo/daemon/router.pypython/harlo/daemon/socket_activation.pypython/harlo/encoder/__init__.pypython/harlo/modulation/allostatic.pyscripts/rotate_anthropic_key.shtests/test_daemon/test_wire_framing.pytests/test_modulation/test_biometric_barrier.py
✅ Files skipped from review due to trivial changes (4)
- ios/HarloPulse/project.yml
- .gitignore
- crates/hippocampus/Cargo.toml
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (3)
- python/harlo/encoder/init.py
- macos/HarloHealthBridge/project.yml
- crates/hippocampus/src/store.rs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
44-61:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
build-healthbridgetohelpoutput.The new target is callable but currently not discoverable via
make help, which makes Phase 5B build flow easy to miss.Suggested patch
help: `@echo` "Harlo build targets" @@ `@echo` " verify test + compliance-greps + doctor + signing-readiness" + `@echo` " build-healthbridge Build HarloHealthBridge.app + embedded HarloXPCRelay (macOS only)" `@echo` " regen-trajectories Regenerate data/trajectories_10k.jsonl (~229 MB, ~minutes)"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 44 - 61, Add documentation for the build-healthbridge target to the help output in the Makefile. In the help target, add a new `@echo` line that describes what the build-healthbridge target does. Place this new line in a logical position among the other build targets (such as after the build-rust or build-macos lines) to ensure the Phase 5B build flow is discoverable when users run make help.
🧹 Nitpick comments (6)
docs/design/2026-06-19-macos-ambient-effectors-plan.md (2)
891-909: ⚡ Quick winOverly broad exception handling risks silent failures in ambient effectors.
The
except Exception: passblock at lines 908–909 is intentionally broad for "advisory" behavior, but swallowing all exceptions without even logging them may hide setup errors (config loading, lease-path issues) that should be visible during testing and debugging.Consider adding at least a debug log or a narrow exception list for Phase 1. For production, this can remain permissive, but during development, visibility is helpful.
🔍 Example: add debug logging
try: ... except Exception as e: # noqa: BLE001 import logging logging.debug(f"ambient effectors advisory skip: {e}")Alternatively, narrow to expected exceptions:
except (FileNotFoundError, yaml.YAMLError, KeyError): pass🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/2026-06-19-macos-ambient-effectors-plan.md` around lines 891 - 909, The bare except Exception: pass block that follows the run_ambient_effectors call silently swallows all exceptions without any logging, which hides setup errors like config loading failures or lease-path issues that should be visible during debugging. Add at least a debug log statement that captures and logs the exception details to improve visibility during development, or alternatively narrow the exception handling to only catch specific expected exceptions like FileNotFoundError, yaml.YAMLError, or KeyError rather than catching all exceptions broadly.
129-129: 💤 Low valueCapitalize "macOS" consistently (LanguageTool hint).
The operating system from Apple is capitalized as "macOS," not "Mac OS" or "MAC OS". Found at lines 129, 1000, 1001.
Also applies to: 1000-1000, 1001-1001
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/2026-06-19-macos-ambient-effectors-plan.md` at line 129, The document contains inconsistent capitalization of Apple's operating system name. Replace all instances of "macos" with "macOS" throughout the document, specifically at line 129 in the macos.install() reference, and at lines 1000 and 1001 where the operating system name appears. Ensure the proper capitalization "macOS" is used consistently wherever the operating system is mentioned in the documentation.Source: Linters/SAST tools
python/harlo/daemon/main.py (1)
136-140: ⚡ Quick winLog DMN teardown join failures instead of swallowing them.
The current
except Exception: passhides shutdown-path failures and makes teardown regressions hard to diagnose.Suggested patch
try: from .dmn_teardown import get_teardown get_teardown().join_with_budget(timeout=float(DMN_BUDGET_S)) - except Exception: - pass + except Exception: + _LOGGER.warning("dmn teardown join failed", exc_info=True)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/harlo/daemon/main.py` around lines 136 - 140, The except block in the DMN teardown section silently swallows all exceptions using 'except Exception: pass', preventing visibility into shutdown failures. Replace the pass statement with appropriate logging of the caught exception. Log the error when the get_teardown().join_with_budget(timeout=float(DMN_BUDGET_S)) call fails so that shutdown-path issues and teardown regressions can be properly diagnosed instead of being hidden.Source: Linters/SAST tools
scripts/rotate_anthropic_key.sh (1)
51-65: 💤 Low valueConsider escaping XML special characters for robustness.
Direct heredoc interpolation of
$NEWKEYinto XML would malform the plist if the key ever contained&,<, or>. Anthropic keys use a safe charset today (sk-ant-*with base64-ish suffix), so practical risk is near-zero, but defensive escaping is trivial:♻️ Optional defensive fix
+# Escape XML special chars (defensive; Anthropic keys don't contain these today) +ESCAPED_KEY="${NEWKEY//&/&}" +ESCAPED_KEY="${ESCAPED_KEY//</<}" +ESCAPED_KEY="${ESCAPED_KEY//>/>}" + cat > "$AGENT" <<PLIST ... - <string>$NEWKEY</string> + <string>$ESCAPED_KEY</string> ... PLIST🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/rotate_anthropic_key.sh` around lines 51 - 65, The `$NEWKEY` variable is directly interpolated into the XML plist content without escaping XML special characters. While current Anthropic keys are safe, if the key ever contained characters like `&`, `<`, or `>`, it would malform the plist. Create a variable that escapes these XML special characters by replacing `&` with `&`, `<` with `<`, `>` with `>`, and quote marks as needed, then use this escaped variable instead of `$NEWKEY` in the heredoc that builds the plist file.macos/HarloXPCRelay/main.swift (1)
93-100: ⚖️ Poor tradeoffNo client code-signature validation in XPC listener.
shouldAcceptNewConnectionaccepts all incoming connections without verifying the client's code signature. Any process with access to the Mach namespace can invokeingestand push arbitrary payloads toward the daemon.The docs note this as future hardening. For Phase 5B this is likely acceptable, but before production, validate
conn.auditTokenorconn.effectiveUserIdentifierto ensure only the legitimate HarloHealthBridge can connect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@macos/HarloXPCRelay/main.swift` around lines 93 - 100, The shouldAcceptNewConnection method in the Delegate class currently accepts all incoming XPC connections without validating the client. Add validation logic to check the connection's auditToken or effectiveUserIdentifier to ensure only the legitimate HarloHealthBridge process can connect. Verify the client credentials before exporting the interface and resuming the connection, and return false to reject any unauthorized connection attempts.docs/HEALTHKIT_ACTIVATION.md (1)
129-132: 💤 Low valueAdd language specifiers to fenced code blocks.
The architecture diagram (line 129) and build instructions (line 149) are missing language specifiers.
📝 Proposed fix
Line 129: Add
textor leave as-is (ASCII diagram)-``` +```text HarloHealthBridge (sandboxed) --NSXPCConnection--> com.harlo.xpc (Mach service)Line 149: Add
bash-``` +```bash swiftc -O macos/HarloXPCRelay/main.swift -o <relay path> # e.g. app Helpers/Also applies to: 149-155
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/HEALTHKIT_ACTIVATION.md` around lines 129 - 132, The markdown document contains fenced code blocks that are missing language specifiers, which reduces readability and syntax highlighting in rendered documentation. Add the `text` language specifier to the opening backticks of the fenced code block containing the architecture diagram that shows the HarloHealthBridge and NSXPCConnection flow, and add the `bash` language specifier to the opening backticks of the fenced code block containing the build instructions with the swiftc compilation command. This will enable proper formatting and syntax highlighting when the documentation is rendered.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/2026-06-19-macos-ambient-effectors-plan.md`:
- Around line 900-910: The function call to run_ambient_effectors is passing a
keyword argument with the wrong name. The function signature defines the
parameter as cognitive_state, but the call is passing cognitive=cognitive.
Change the keyword argument name from cognitive to cognitive_state in the
run_ambient_effectors function call to match the expected parameter name in the
function signature.
- Around line 883-928: The issue is that `_handle_biometric_ingest` computes and
writes the new depleted state without first capturing the prior depleted state
from the persisted modulation state, making it unavailable for the ambient
effectors logic. Add a read of the existing persisted modulation state at the
very top of `_handle_biometric_ingest` (before any existing writes occur and
before line 1036) using something like `read_modulation_state(...)`, extract the
`is_depleted` value from that prior state into a `prior_depleted` variable, and
ensure this variable is available to the `run_ambient_effectors()` call in the
try-block so that the ambient effectors receive the depletion state before it
was overwritten by the current ingest.
In `@docs/design/2026-06-19-macos-ambient-effectors.md`:
- Around line 55-64: The fenced code block in the design document is missing a
language identifier, which violates the MD040 markdown linting rule. Locate the
opening triple backticks (```) that precede the ASCII tree diagram showing the
biometric_ingest flow with AllostasisTracker and executor.execute_one(), and add
the language identifier text immediately after the backticks so it reads ```text
instead of just ```. This applies only to the opening fence, not the closing
one.
In `@docs/HEALTHKIT_ACTIVATION.md`:
- Around line 140-142: The documentation for HarloHealthBridge.entitlements
shows the mach service name as abbreviated `com.harlo.xpc` but the actual
entitlement value includes a team ID prefix. Update the documentation line
showing the `com.apple.security.temporary-exception.mach-lookup.global-name`
entitlement value to display the complete service name
`233JSS4X69.com.harlo.xpc` instead of the abbreviated `com.harlo.xpc` to
accurately reflect the actual code implementation.
- Around line 135-136: The documentation incorrectly references the relay source
file as HarloXPCRelay.swift when the actual filename is main.swift. Update all
references to the relay file from HarloXPCRelay.swift to main.swift, including
both the description of the relay around line 135-136 where it mentions the Mach
service and NSXPCListener, as well as the build command reference on line 150
that uses the incorrect filename.
In `@macos/HarloHealthBridge/Sources/HarloHealthBridge/DaemonWriter.swift`:
- Around line 35-43: The cast of remoteObjectProxyWithErrorHandler to
HarloXPCProtocol on line 38 can fail silently, resulting in proxy being nil,
which causes the ingest call to do nothing and the connection to never be
invalidated. Add an explicit check after the cast to verify that proxy is not
nil. If proxy is nil, log an error message indicating the cast failed and
immediately invalidate the connection. Only proceed with the ingest call if
proxy is not nil. This ensures that all code paths properly clean up the
connection and provide visibility into cast failures.
In `@macos/launchd/com.harlo.xpc.plist`:
- Around line 29-33: The ProcessType Adaptive setting in the plist does not
automatically terminate idle services—it only provides hints to the scheduler.
The HarloXPCRelay implementation currently runs RunLoop.main.run() without any
XPC transaction tracking or idle-exit logic, causing the service to remain alive
indefinitely. To achieve true idle-exit compliance, modify the HarloXPCRelay to
wrap its work operations in xpc_transaction_begin and xpc_transaction_end calls
to signal activity periods to launchd, or alternatively implement an explicit
idle timeout mechanism that exits the process after the specified idle period
with no active transactions.
In `@python/harlo/daemon/router.py`:
- Around line 1066-1081: The modulation state persistence is gated behind the
accepted condition and swallows database errors silently, which can cause SQLite
state to diverge from JSON state and leave surfaces out of sync with
motor-gating inputs. Remove the if accepted: gate wrapping the
write_modulation_state_db call so persistence happens consistently regardless of
acceptance status, and replace the silent exception handling (except Exception:
pass) with proper error surfacing or logging to ensure database failures are
visible rather than hidden, maintaining consistency between state stores per the
coding guideline to surface problems rather than skip them.
In `@python/harlo/modulation/allostatic.py`:
- Around line 84-93: Add validation logic in the __init__ method to ensure that
rr_red_bpm is strictly greater than rr_floor_bpm at construction time. After the
parameter assignments for rr_floor_bpm and rr_red_bpm, insert a check that
raises a ValueError with a descriptive message if rr_red_bpm is less than or
equal to rr_floor_bpm. This will fail fast and prevent silent misconfiguration
where the RR contribution gets zeroed out later in the code.
In `@README.md`:
- Around line 31-42: The fenced code block containing the PRODUCTION LIVE status
information is missing a language specifier on the opening fence. Add a language
hint such as `text` immediately after the opening triple backticks (change ` ```
` to ` ```text `) to satisfy the markdownlint MD040 rule and keep the
documentation lint clean.
---
Outside diff comments:
In `@Makefile`:
- Around line 44-61: Add documentation for the build-healthbridge target to the
help output in the Makefile. In the help target, add a new `@echo` line that
describes what the build-healthbridge target does. Place this new line in a
logical position among the other build targets (such as after the build-rust or
build-macos lines) to ensure the Phase 5B build flow is discoverable when users
run make help.
---
Nitpick comments:
In `@docs/design/2026-06-19-macos-ambient-effectors-plan.md`:
- Around line 891-909: The bare except Exception: pass block that follows the
run_ambient_effectors call silently swallows all exceptions without any logging,
which hides setup errors like config loading failures or lease-path issues that
should be visible during debugging. Add at least a debug log statement that
captures and logs the exception details to improve visibility during
development, or alternatively narrow the exception handling to only catch
specific expected exceptions like FileNotFoundError, yaml.YAMLError, or KeyError
rather than catching all exceptions broadly.
- Line 129: The document contains inconsistent capitalization of Apple's
operating system name. Replace all instances of "macos" with "macOS" throughout
the document, specifically at line 129 in the macos.install() reference, and at
lines 1000 and 1001 where the operating system name appears. Ensure the proper
capitalization "macOS" is used consistently wherever the operating system is
mentioned in the documentation.
In `@docs/HEALTHKIT_ACTIVATION.md`:
- Around line 129-132: The markdown document contains fenced code blocks that
are missing language specifiers, which reduces readability and syntax
highlighting in rendered documentation. Add the `text` language specifier to the
opening backticks of the fenced code block containing the architecture diagram
that shows the HarloHealthBridge and NSXPCConnection flow, and add the `bash`
language specifier to the opening backticks of the fenced code block containing
the build instructions with the swiftc compilation command. This will enable
proper formatting and syntax highlighting when the documentation is rendered.
In `@macos/HarloXPCRelay/main.swift`:
- Around line 93-100: The shouldAcceptNewConnection method in the Delegate class
currently accepts all incoming XPC connections without validating the client.
Add validation logic to check the connection's auditToken or
effectiveUserIdentifier to ensure only the legitimate HarloHealthBridge process
can connect. Verify the client credentials before exporting the interface and
resuming the connection, and return false to reject any unauthorized connection
attempts.
In `@python/harlo/daemon/main.py`:
- Around line 136-140: The except block in the DMN teardown section silently
swallows all exceptions using 'except Exception: pass', preventing visibility
into shutdown failures. Replace the pass statement with appropriate logging of
the caught exception. Log the error when the
get_teardown().join_with_budget(timeout=float(DMN_BUDGET_S)) call fails so that
shutdown-path issues and teardown regressions can be properly diagnosed instead
of being hidden.
In `@scripts/rotate_anthropic_key.sh`:
- Around line 51-65: The `$NEWKEY` variable is directly interpolated into the
XML plist content without escaping XML special characters. While current
Anthropic keys are safe, if the key ever contained characters like `&`, `<`, or
`>`, it would malform the plist. Create a variable that escapes these XML
special characters by replacing `&` with `&`, `<` with `<`, `>` with
`>`, and quote marks as needed, then use this escaped variable instead of
`$NEWKEY` in the heredoc that builds the plist file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62fb3d9b-dd7c-42f4-854a-7d69a2b51032
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
.gitignoreCLAUDE.mdMakefileREADME.mdcrates/hippocampus/Cargo.tomlcrates/hippocampus/src/store.rsdocs/HEALTHKIT_ACTIVATION.mddocs/design/2026-06-19-macos-ambient-effectors-plan.mddocs/design/2026-06-19-macos-ambient-effectors.mdios/HarloPulse/Info.plistios/HarloPulse/project.ymlmacos/Harlo.app/Contents/Info.plistmacos/HarloHealthBridge/Info.plistmacos/HarloHealthBridge/Sources/HarloHealthBridge/DaemonWriter.swiftmacos/HarloHealthBridge/Sources/HarloHealthBridge/HarloHealthBridge.entitlementsmacos/HarloHealthBridge/project.ymlmacos/HarloXPCRelay/main.swiftmacos/launchd/com.harlo.xpc.plistpyproject.tomlpython/harlo/__init__.pypython/harlo/daemon/config.pypython/harlo/daemon/main.pypython/harlo/daemon/router.pypython/harlo/daemon/socket_activation.pypython/harlo/encoder/__init__.pypython/harlo/modulation/allostatic.pyscripts/rotate_anthropic_key.shtests/test_daemon/test_wire_framing.pytests/test_modulation/test_biometric_barrier.py
✅ Files skipped from review due to trivial changes (4)
- ios/HarloPulse/project.yml
- .gitignore
- crates/hippocampus/Cargo.toml
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (3)
- python/harlo/encoder/init.py
- macos/HarloHealthBridge/project.yml
- crates/hippocampus/src/store.rs
🛑 Comments failed to post (10)
docs/design/2026-06-19-macos-ambient-effectors-plan.md (2)
883-928:
⚠️ Potential issue | 🔴 Critical🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Check if prior_depleted or similar is already captured in router.py rg -n -A 20 'def _handle_biometric_ingest' python/harlo/daemon/router.pyRepository: JosephOIbrahim/Harlo
Length of output: 823
🏁 Script executed:
#!/bin/bash # Get the full _handle_biometric_ingest method (increase context) rg -n -A 100 'def _handle_biometric_ingest' python/harlo/daemon/router.py | head -120Repository: JosephOIbrahim/Harlo
Length of output: 3305
Incomplete guidance for capturing
prior_depletedin router.The current implementation of
_handle_biometric_ingest(lines 1036–1061) computes the newdepletedstate and immediately writes it viawrite_modulation_state()without capturing the prior depleted state first. The design doc's constraint—"prior_depleted must be read from the persisted modulation state before the existing code overwrites it"—is not yet implemented.Add a read of the prior modulation state to the top of
_handle_biometric_ingest(before line 1036), captureprior_depleted, and make that variable available to the try-block that callsrun_ambient_effectors(...).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/2026-06-19-macos-ambient-effectors-plan.md` around lines 883 - 928, The issue is that `_handle_biometric_ingest` computes and writes the new depleted state without first capturing the prior depleted state from the persisted modulation state, making it unavailable for the ambient effectors logic. Add a read of the existing persisted modulation state at the very top of `_handle_biometric_ingest` (before any existing writes occur and before line 1036) using something like `read_modulation_state(...)`, extract the `is_depleted` value from that prior state into a `prior_depleted` variable, and ensure this variable is available to the `run_ambient_effectors()` call in the try-block so that the ambient effectors receive the depletion state before it was overwritten by the current ingest.
900-910:
⚠️ Potential issue | 🔴 Critical | ⚡ Quick winParameter name mismatch in
run_ambient_effectorscall (line 903).The function signature defines
cognitive_state: str(line 835), but the call at line 903 passescognitive=cognitive. This will raise aTypeError: unexpected keyword argument 'cognitive'. The argument must match the parameter name.run_ambient_effectors( prior_depleted=bool(prior_depleted), now_depleted=bool(depleted), - cognitive=cognitive, + cognitive_state=cognitive_state, cfg=cfg,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/2026-06-19-macos-ambient-effectors-plan.md` around lines 900 - 910, The function call to run_ambient_effectors is passing a keyword argument with the wrong name. The function signature defines the parameter as cognitive_state, but the call is passing cognitive=cognitive. Change the keyword argument name from cognitive to cognitive_state in the run_ambient_effectors function call to match the expected parameter name in the function signature.docs/design/2026-06-19-macos-ambient-effectors.md (1)
55-64:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSpecify a language for this fenced block.
This code fence is missing a language identifier (MD040).
textis sufficient here.Suggested patch
-``` +```text biometric_ingest (router, event-driven — Rule 1, no polling) └─ AllostasisTracker → is_depleted() transitions False→True └─ premotor.create_plan(intent="ambient_wind_down", reversible=True) └─ executor.execute_one() ├─ cognitive_state == "RED"? → HALTED (Rule 28) [nudge-only] ├─ basal_ganglia.gate() → the 5 checks, inhibition-default └─ DISINHIBIT → handler "display_warmth" (etc.) └─ TIERED BACKEND: nudge | shortcuts run | direct-helper -``` +```🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 55-55: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/2026-06-19-macos-ambient-effectors.md` around lines 55 - 64, The fenced code block in the design document is missing a language identifier, which violates the MD040 markdown linting rule. Locate the opening triple backticks (```) that precede the ASCII tree diagram showing the biometric_ingest flow with AllostasisTracker and executor.execute_one(), and add the language identifier text immediately after the backticks so it reads ```text instead of just ```. This applies only to the opening fence, not the closing one.Source: Linters/SAST tools
docs/HEALTHKIT_ACTIVATION.md (2)
135-136:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDocumentation references wrong filename.
The relay source file is
macos/HarloXPCRelay/main.swift, notHarloXPCRelay.swift. The build command on line 150 also references the incorrect filename.📝 Proposed fix
Components (committed): -- `macos/HarloXPCRelay/HarloXPCRelay.swift` — the relay: `NSXPCListener` on the +- `macos/HarloXPCRelay/main.swift` — the relay: `NSXPCListener` on the Mach service, forwards framed `biometric_ingest` to the daemon's twind.sock.And for line 150:
-swiftc -O macos/HarloXPCRelay/HarloXPCRelay.swift -o <relay path> # e.g. app Helpers/ +swiftc -O macos/HarloXPCRelay/main.swift -o <relay path> # e.g. app Helpers/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.- `macos/HarloXPCRelay/main.swift` — the relay: `NSXPCListener` on the Mach service, forwards framed `biometric_ingest` to the daemon's twind.sock.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/HEALTHKIT_ACTIVATION.md` around lines 135 - 136, The documentation incorrectly references the relay source file as HarloXPCRelay.swift when the actual filename is main.swift. Update all references to the relay file from HarloXPCRelay.swift to main.swift, including both the description of the relay around line 135-136 where it mentions the Mach service and NSXPCListener, as well as the build command reference on line 150 that uses the incorrect filename.
140-142:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMach service name in docs is abbreviated.
Line 141 shows
[com.harlo.xpc]but the actual entitlement value is233JSS4X69.com.harlo.xpc(with the team ID prefix). This could confuse readers cross-referencing with the code.📝 Proposed fix
- `HarloHealthBridge.entitlements` — adds - `com.apple.security.temporary-exception.mach-lookup.global-name = [com.harlo.xpc]` + `com.apple.security.temporary-exception.mach-lookup.global-name = [233JSS4X69.com.harlo.xpc]` so the sandboxed bridge can look up the relay.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.- `HarloHealthBridge.entitlements` — adds `com.apple.security.temporary-exception.mach-lookup.global-name = [233JSS4X69.com.harlo.xpc]` so the sandboxed bridge can look up the relay.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/HEALTHKIT_ACTIVATION.md` around lines 140 - 142, The documentation for HarloHealthBridge.entitlements shows the mach service name as abbreviated `com.harlo.xpc` but the actual entitlement value includes a team ID prefix. Update the documentation line showing the `com.apple.security.temporary-exception.mach-lookup.global-name` entitlement value to display the complete service name `233JSS4X69.com.harlo.xpc` instead of the abbreviated `com.harlo.xpc` to accurately reflect the actual code implementation.macos/HarloHealthBridge/Sources/HarloHealthBridge/DaemonWriter.swift (1)
35-43:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSilent failure when proxy cast fails.
If
remoteObjectProxyWithErrorHandlerreturns an object that doesn't conform toHarloXPCProtocol(line 38 cast fails),proxybecomesniland theingestcall silently does nothing. The connection is never invalidated in this path.Consider logging and invalidating when proxy is nil:
🛡️ Proposed fix
let proxy = conn.remoteObjectProxyWithErrorHandler { [weak self] error in self?.log.error("xpc relay error: \(error.localizedDescription)") conn.invalidate() } as? HarloXPCProtocol - proxy?.ingest(data) { [weak self] ok, resp in + guard let proxy = proxy else { + log.error("xpc proxy cast failed") + conn.invalidate() + return + } + proxy.ingest(data) { [weak self] ok, resp in self?.log.info("daemon ingest ok=\(ok) resp=\(resp, privacy: .public)") conn.invalidate() }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.let proxy = conn.remoteObjectProxyWithErrorHandler { [weak self] error in self?.log.error("xpc relay error: \(error.localizedDescription)") conn.invalidate() } as? HarloXPCProtocol guard let proxy = proxy else { log.error("xpc proxy cast failed") conn.invalidate() return } proxy.ingest(data) { [weak self] ok, resp in self?.log.info("daemon ingest ok=\(ok) resp=\(resp, privacy: .public)") conn.invalidate() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@macos/HarloHealthBridge/Sources/HarloHealthBridge/DaemonWriter.swift` around lines 35 - 43, The cast of remoteObjectProxyWithErrorHandler to HarloXPCProtocol on line 38 can fail silently, resulting in proxy being nil, which causes the ingest call to do nothing and the connection to never be invalidated. Add an explicit check after the cast to verify that proxy is not nil. If proxy is nil, log an error message indicating the cast failed and immediately invalidate the connection. Only proceed with the ingest call if proxy is not nil. This ensures that all code paths properly clean up the connection and provide visibility into cast failures.macos/launchd/com.harlo.xpc.plist (1)
29-33:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🌐 Web query:
Does launchd ProcessType Adaptive automatically kill idle Mach services?💡 Result:
No, the ProcessType Adaptive key does not automatically kill idle Mach services [1][2]. The ProcessType key is used to define the intended purpose and resource constraints for a job [1]. Specifically, Adaptive jobs are designed to dynamically transition between Background and Interactive classifications based on activity observed over XPC connections [1][2]. This mechanism relies on xpc_transaction_begin(3) and xpc_transaction_end(3) to track active work, allowing the system to adjust resource limits (such as CPU and I/O priority) accordingly [3][1]. While launchd is responsible for managing the lifecycle of daemons and can launch them on-demand when a service request is received on a Mach port [4][5], the automatic termination of an idle process is a separate behavior. A daemon may choose to exit voluntarily after a period of inactivity to conserve memory [5], but the Adaptive designation itself does not trigger or govern the automatic killing of idle services [1][2].
Citations:
- 1: https://leancrew.com/all-this/man/man5/launchd.plist.html
- 2: https://www.manpagez.com/man/5/launchd.plist/
- 3: https://developer.apple.com/forums/thread/682769
- 4: https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/BPSystemStartup/Chapters/Lifecycle.html
- 5: https://developer.apple.com/library/mac/documentation/MacOSX/Conceptual/BPSystemStartup/Chapters/CreatingLaunchdJobs.html
🏁 Script executed:
# Find the HarloXPCRelay implementation find . -name "*HarloXPC*" -o -name "*relay*" -o -name "*xpc*" | grep -i relay | head -20Repository: JosephOIbrahim/Harlo
Length of output: 86
🏁 Script executed:
# Search for xpc_transaction usage in the codebase rg "xpc_transaction" --type swift --type objc --type cRepository: JosephOIbrahim/Harlo
Length of output: 46
🏁 Script executed:
# Search for idle-exit or keep-alive patterns in the codebase rg "(idle|timeout|KeepAlive|EnableTransactions)" --type swift --type objc --type plistRepository: JosephOIbrahim/Harlo
Length of output: 94
🏁 Script executed:
# List the contents of HarloXPCRelay directory ls -la ./macos/HarloXPCRelay/Repository: JosephOIbrahim/Harlo
Length of output: 235
🏁 Script executed:
# Check Swift files in HarloXPCRelay for any transaction or lifecycle code rg "(xpc_transaction|exit|timeout|deinit|RunLoop)" ./macos/HarloXPCRelay/ --type swiftRepository: JosephOIbrahim/Harlo
Length of output: 116
🏁 Script executed:
# Search for idle-exit or timeout patterns without plist type filter rg "(idle|timeout|KeepAlive|EnableTransactions)" --type swift --type objcRepository: JosephOIbrahim/Harlo
Length of output: 592
🏁 Script executed:
# Examine the plist file itself to understand the complete configuration cat -n ./macos/launchd/com.harlo.xpc.plistRepository: JosephOIbrahim/Harlo
Length of output: 1519
Idle-exit behavior is not implemented; ProcessType Adaptive alone does not provide automatic idle termination.
The plist claims "idle-exits → keeps Rule 1 (0W idle) intact", but
ProcessType Adaptiveonly hints to the scheduler based onxpc_transactiontracking—it does not automatically kill idle services. The HarloXPCRelay implementation contains onlyRunLoop.main.run()with no xpc transaction wrapping or idle-exit logic, so the relay stays alive indefinitely.To achieve true 0W idle compliance, wrap work in
xpc_transaction_begin/endto signal activity to launchd, or implement an explicit idle timeout and exit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@macos/launchd/com.harlo.xpc.plist` around lines 29 - 33, The ProcessType Adaptive setting in the plist does not automatically terminate idle services—it only provides hints to the scheduler. The HarloXPCRelay implementation currently runs RunLoop.main.run() without any XPC transaction tracking or idle-exit logic, causing the service to remain alive indefinitely. To achieve true idle-exit compliance, modify the HarloXPCRelay to wrap its work operations in xpc_transaction_begin and xpc_transaction_end calls to signal activity periods to launchd, or alternatively implement an explicit idle timeout mechanism that exits the process after the specified idle period with no active transactions.python/harlo/daemon/router.py (1)
1066-1081:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep modulation-state persistence consistent across stores and surface failures.
Gating SQLite persistence on
acceptedand then swallowing DB errors can leave SQLite state stale while JSON state is updated, causing status/coach surfaces to diverge from motor-gating inputs.Suggested patch
- if accepted: - try: - from ..daemon.config import DB_PATH - from ..modulation.state_store import ( - write_modulation_state as write_modulation_state_db, - ) - - write_modulation_state_db( - str(DB_PATH), - biometric_load=biometric_load, - depleted=depleted, - force_red=force_red, - samples_accepted=accepted, - ) - except Exception: # noqa: BLE001 — persistence is advisory - pass + try: + from ..daemon.config import DB_PATH + from ..modulation.state_store import ( + write_modulation_state as write_modulation_state_db, + ) + + write_modulation_state_db( + str(DB_PATH), + biometric_load=biometric_load, + depleted=depleted, + force_red=force_red, + samples_accepted=accepted, + ) + except Exception: + import logging + logging.getLogger(__name__).warning( + "failed to persist modulation state to SQLite", + exc_info=True, + )As per coding guidelines: “Queue unverified outputs during burst. Run GVR on burst exit. Surface problems. Defer, never skip.”
🧰 Tools
🪛 Ruff (0.15.17)
[error] 1080-1081:
try-except-passdetected, consider logging the exception(S110)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/harlo/daemon/router.py` around lines 1066 - 1081, The modulation state persistence is gated behind the accepted condition and swallows database errors silently, which can cause SQLite state to diverge from JSON state and leave surfaces out of sync with motor-gating inputs. Remove the if accepted: gate wrapping the write_modulation_state_db call so persistence happens consistently regardless of acceptance status, and replace the silent exception handling (except Exception: pass) with proper error surfacing or logging to ensure database failures are visible rather than hidden, maintaining consistency between state stores per the coding guideline to surface problems rather than skip them.Source: Coding guidelines
python/harlo/modulation/allostatic.py (1)
84-93:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate RR threshold ordering at construction time.
Line 84 and Line 85 allow
rr_red_bpm <= rr_floor_bpm; then Line 221-223 silently zeroes RR contribution forever. Fail fast in__init__so misconfiguration can’t suppress depletion signaling.Suggested fix
def __init__( self, window_sec: float = _WINDOW_SEC, biometric_window_sec: float = _BIOMETRIC_WINDOW_SEC, biometric_red_freshness_sec: float = _BIOMETRIC_RED_FRESHNESS_SEC, hr_red_bpm: float = _DEFAULT_HR_RED_BPM, hrv_red_ms: float = _DEFAULT_HRV_RED_MS, rr_floor_bpm: float = _DEFAULT_RR_FLOOR_BPM, rr_red_bpm: float = _DEFAULT_RR_RED_BPM, ) -> None: + if rr_red_bpm <= rr_floor_bpm: + raise ValueError("rr_red_bpm must be greater than rr_floor_bpm") self._window_sec = window_sec self._biometric_window_sec = biometric_window_sec self._biometric_red_freshness_sec = biometric_red_freshness_sec self._hr_red_bpm = hr_red_bpm self._hrv_red_ms = hrv_red_ms🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/harlo/modulation/allostatic.py` around lines 84 - 93, Add validation logic in the __init__ method to ensure that rr_red_bpm is strictly greater than rr_floor_bpm at construction time. After the parameter assignments for rr_floor_bpm and rr_red_bpm, insert a check that raises a ValueError with a descriptive message if rr_red_bpm is less than or equal to rr_floor_bpm. This will fail fast and prevent silent misconfiguration where the RR contribution gets zeroed out later in the code.README.md (1)
31-42:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a fenced-code language for this status block.
markdownlintflags this block (MD040). Use a language hint (e.g.,text) to keep docs lint clean.Suggested patch
-``` +```text PRODUCTION LIVE — Harlo v6.1-MOTOR ... -v0.2.0: unified release — biometric_prior seed + HarloPulse (iPhone) + HealthBridge/XPC (macOS) reconciled; one version across every surface (core · Rust · macOS app · iOS app · bridge) -``` +v0.2.0: unified release — biometric_prior seed + HarloPulse (iPhone) + HealthBridge/XPC (macOS) reconciled; one version across every surface (core · Rust · macOS app · iOS app · bridge) +```🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 31 - 42, The fenced code block containing the PRODUCTION LIVE status information is missing a language specifier on the opening fence. Add a language hint such as `text` immediately after the opening triple backticks (change ` ``` ` to ` ```text `) to satisfy the markdownlint MD040 rule and keep the documentation lint clean.Source: Linters/SAST tools
JosephOIbrahim
changed the title
Owner
Author
CI status note — the red check is inherited from
|
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
1 participant
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.
Summary
Closes the P0 IPC/biometric seams, lands the Phase 5B macOS Apple-Watch biometric
loop, reconciles the divergence with
master, unifies the product version tov0.2.0, adds respiratory-rate scoring, and lands the design + TDD plan
for the first macOS ambient effectors.
48 files · +3,587 / −353· 16 commits.What's in it
Daemon IPC + biometric seams (C1/C2/C3) + CORE-1 encoder
launch_activate_socket); Rule 1 (0W idle) actually realized on macOS.daemon/framing.py); reply-in-same-mode (the XPC relay reads a length-prefixed reply).biometric_force_red.EncoderMismatch+ persistedencoder_idtag; lexical vs semantic SDRs can't silently cross.Phase 5B — Apple Watch → HealthBridge → XPC relay → daemon (macOS)
HarloHealthBridge.app(HealthKit observers, signed) +HarloXPCRelaylaunchd Mach service bridging the sandbox → daemon UNIX socket (the App Group container can't host it).force_red: true.master reconciliation merge (
30dbd6a)acquire_listening_socket+ our framinghandle_client; both socket APIs kept (both test suites pin them).biometric_ingestwrites the derived verdict to both modulation stores (C3 motor path + D60 status surface).v0.2.0 version unification (
399a42f)Cargo.lock) ·harlo.__version__· all 3 app bundles · bothproject.yml.respiratory_rate scoring (
fe94e8e)get_biometric_load(DEPLETED trend), excess-over-floor so normal breathing adds zero load; not wired toforce_red(RR is a slow signal). 5 TDD tests.Design docs — NOT implemented, design only
docs/design/2026-06-19-macos-ambient-effectors-design.md— biometric state → gated macOS ambient adjustments through the existing Motor Cortex (three-band ownership model, four safety rails, DEPLETED-triggers / RED-blocks).docs/design/2026-06-19-macos-ambient-effectors-plan.md— Phase-1 TDD implementation plan (9 tasks).Ops / docs
scripts/rotate_anthropic_key.sh— safe one-shot API-key rotation (prompts hidden, validates, persists, restarts, never prints the key).Testing
cargo test -p hippocampus: 43/43.test_daemon75,test_modulation65,test_biometric_prior55,test_integration79).test_scheduleclock-substitution flake (code path byte-identical tomaster),test_mcpnative model-load segfault (env),test_path_dempty dir.Out of scope / follow-ups
🤖 Generated with Claude Code