Menu

#333 fix(health): resolve PR #330 review findings

closed
nobody
None
2026-06-24
2026-06-24
Anonymous
No

Originally created by: Akarsh-Hegde

Summary

Addresses all review findings raised by @adityaharishch on PR [#330]. Targets pre-main so it is included in the pre-main → main promotion.

Fixed

[HIGH CORRECTNESS] meridian doctor exits non-zero on every healthy post-fold install

  • src/health/ui.rs: Replaced the entire Node-era UI probe (HTTP GET http://localhost:3939, /_next/static asset check, serve_mode_check) with a single Info check: "dashboard embedded in the Tauri binary (no separate service)". The com.meridiona.ui launchd agent no longer exists post-fold; probing for it always produced a false CRITICAL.
  • src/health/platform.rs: Removed LABEL_UI constant and replaced ui_service() body with the same Info check. The Node-era plist/PID check is gone.

[HIGH CORRECTNESS] MLX restart-budget exhaustion permanently wedges

  • tray/src-tauri/src/poll/mod.rs: Added MLX_COOLING_TICKS = 10. After MLX_MAX_RESTARTS consecutive failures, attempts now increments through the cooling window. At MAX_RESTARTS + COOLING_TICKS (~10 min), it resets to 0supervise() re-runs, kills any zombie holding port 7823 (via the existing KilledWedged path), and allows a fresh restart cycle. Previously the tray would stay wedged forever.

[HIGH SECURITY] MERIDIAN_RUNTIME_MANIFEST_URL redirectable in release builds

  • tray/src-tauri/src/mlx_server.rs: Gated the runtime env-var override behind #[cfg(debug_assertions)]. A release binary can no longer be redirected to an attacker-controlled manifest via a compromised environment variable.
  • Deferred: The self-referential SHA-256 issue (manifest and tarball both served from the same origin — a compromised server can swap both atomically) requires an out-of-band trust root (minisign or embedded public key, similar to the updater's pub_key). This is a follow-up design task; the env-var gate closes the more immediate attack surface.

[MEDIUM CORRECTNESS] capture_coverage datetime comparison bug

  • src/health/capture.rs: datetime('now', ?1) produces '2026-06-23 17:00:00' (space separator, no Z), but insert_capture_frame writes RFC 3339 '2026-06-24T16:00:00.000000Z' (T+Z). At index 10, 'T' (0x54) > ' ' (0x20) — every row passed the filter, so the 1-day window never excluded anything and coverage ghosting went undetected. Fixed to julianday(timestamp) > julianday('now', ?1) following the frame_freshness pattern already in the same file.

[MEDIUM CORRECTNESS] subscribe() snapshot prime silently swallows errors

  • ui/lib/bridge.ts: Replaced .catch(() => {}) with retry-once-after-2s (targeted at the cold-DB startup race — tray can beat the daemon by ~1 s), then console.warn(...) on second failure. The view can still stay blank if the DB never opens, but the error is now visible in DevTools rather than silently dropped.

[MEDIUM SECURITY] sudo npm install runs lifecycle scripts as root

  • scripts/install-from-bundle.sh: Added --ignore-scripts to both npm install -g lines (non-sudo and sudo paths). A compromised screenpipe@0.4.6 tarball can no longer execute lifecycle scripts as root.

Deferred (noted, not blocked)

  • install.sh OO sha256: OpenObserve v0.90.3 tarball is downloaded without integrity verification. Fix requires sha256 of the official arm64 + amd64 tarballs. These need to be fetched from the OO release page and pinned — straightforward but requires the real values.
  • build-mlx-runtime.sh PBS version pin: releases/latest is resolved at build time without a sha256 check. Fix requires pinning PBS_RELEASE_TAG to a specific astral-sh/python-build-standalone release and verifying the downloaded archive.
  • MLX manifest self-referential SHA-256: Manifest and tarball served from the same origin — requires a minisign/ed25519 out-of-band trust root. Design tracked separately.

Test plan

  • [ ] cargo clippy -- -D warnings — clean ✅ (pre-push verified)
  • [ ] cargo test — all passing ✅ (pre-push verified)
  • [ ] UI build — clean ✅ (pre-push verified)
  • [ ] meridian doctor on a post-fold install should show Info for the ui group, not Critical/Warn
  • [ ] MLX wedge: manually exhaust MLX_MAX_RESTARTS; verify attempts increments through cooling and resets at tick 15

🤖 Generated with Claude Code

https://claude.ai/code/session_01SKdh3tKWLZhtQ9FCfA5RYH

Related

Tickets: #330
Tickets: #347

Discussion

  • Anonymous

    Anonymous - 2026-06-24

    Originally posted by: coderabbitai[bot]

    [!IMPORTANT]

    Review skipped

    Auto reviews are disabled on base/target branches other than the default branch.

    Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.


    ⚙️ Run configuration

    Configuration used: Organization UI

    Review profile: ASSERTIVE

    Plan: Pro Plus

    Run ID: c07ab096-60d8-453c-a5b1-b43a4ddc402d

    You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

    Use the checkbox below for a quick retry:
    - [ ] 🔍 Trigger review

    ✨ Finishing Touches
    🧪 Generate unit tests (beta) - [ ] Create PR with unit tests - [ ] Commit unit tests in branch `fix/pr-330-review`

    Comment @coderabbitai help to get the list of available commands.

     
  • Anonymous

    Anonymous - 2026-06-24

    Ticket changed by: Akarsh-Hegde

    • status: open --> closed
     

Log in to post a comment.