fix(macos): stop runtime config-health sidecar access#99039
Conversation
openclaw-barnacle
Bot
added
app: macos
|
Codex review: passed. Reviewed July 2, 2026, 8:20 AM ET / 12:20 UTC. Summary PR surface: Other +22. Total +22 across 2 files. Reproducibility: yes. Current main still writes Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land this sidecar-removal shape after maintainer acceptance, keeping durable config health in SQLite/doctor migration and avoiding any restored steady-state JSON fallback. Do we have a high-confidence way to reproduce the issue? Yes. Current main still writes Is this the best way to solve the issue? Yes, pending maintainer acceptance. Removing the macOS steady-state sidecar read/write is the narrow storage-boundary fix because durable config health is already owned by AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 98e97661dae4. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Other +22. Total +22 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Updated the PR Evidence section with the requested behavior proof. Added a macOS before/after source-runtime smoke using the actual Also noted that GitHub CI passed @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
proof: sufficient
|
Maintainer judgment: I would accept this storage-boundary change. The durable config-health owner is now the shared SQLite I am comfortable with the tradeoff that the macOS app loses a cross-session JSON baseline here. That sidecar is not a user data contract, and the app still preserves in-session anomaly detection via process-local state. Durable cross-session config health should flow through the canonical SQLite/gateway-owned path rather than reviving the retired sidecar. The proof is sufficient for this scope: CI passed the macOS Swift and Node lanes, and the before/after Swift harness demonstrates that current main recreates the sidecar after deletion while this PR head does not. A full signed GUI smoke would be extra confidence, but I would not block this narrow fix on it. Given that #98928 only removes the writer while keeping a legacy runtime reader, I would treat this PR as the better canonical fix and supersede #98928. @clawsweeper approve |
|
🦞👀 Command router queued. I will update this comment with the next step. |
|
@clawsweeper automerge |
|
🦞✅ Source: What merged:
Automerge notes:
The automerge loop is complete. Automerge progress:
|
clawsweeper
Bot
added
clawsweeper:automerge
Summary: - The PR removes macOS runtime reads and writes of `logs/config-health.json`, keeps config-health observation in process memory, and adds Swift tests for ignoring and not recreating the retired sidecar. - PR surface: Other +22. Total +22 across 2 files. - Reproducibility: yes. Current main still writes `logs/config-health.json` from the macOS config observation path, and the PR body includes before/after terminal output for that same Swift path. Automerge notes: - No ClawSweeper repair was needed after automerge opt-in. Validation: - ClawSweeper review passed for head 8cbc4e1. - Required merge gates passed before the squash merge. Prepared head SHA: 8cbc4e1 Review: openclaw#99039 (comment) Co-authored-by: momothemage <[email protected]> Approved-by: momothemage
Summary: - The PR removes macOS runtime reads and writes of `logs/config-health.json`, keeps config-health observation in process memory, and adds Swift tests for ignoring and not recreating the retired sidecar. - PR surface: Other +22. Total +22 across 2 files. - Reproducibility: yes. Current main still writes `logs/config-health.json` from the macOS config observation path, and the PR body includes before/after terminal output for that same Swift path. Automerge notes: - No ClawSweeper repair was needed after automerge opt-in. Validation: - ClawSweeper review passed for head 8cbc4e1. - Required merge gates passed before the squash merge. Prepared head SHA: 8cbc4e1 Review: openclaw#99039 (comment) Co-authored-by: momothemage <[email protected]> Approved-by: momothemage
1 participant
Closes #98917
Supersedes #98928
What Problem This Solves
Fixes an issue where users running the macOS app alongside the CLI/gateway would see recurring legacy state migration warnings because the app kept recreating
~/.openclaw/logs/config-health.jsonafter Node core moved config-health state to the shared SQLiteconfig_health_entriestable.The existing #98928 PR stops the legacy writer, but keeps a runtime fallback reader for the retired sidecar. This PR supersedes it by removing both the steady-state read and write paths from the macOS runtime, matching the database-first contract that legacy JSON state is only a doctor/migration input.
Why This Change Was Made
The macOS app now keeps its config observation baseline in process-local memory instead of reading or writing
logs/config-health.json. That preserves in-session suspicious-read detection while lettingopenclaw doctor --fixown legacy JSON import/archive and preventing the app from recreating or depending on the retired sidecar.Regression coverage now verifies both sides of the boundary: existing legacy sidecars are ignored by
loadDict(), and normal plus suspicious config reads do not createlogs/config-health.json.User Impact
Mac app users should no longer get repeated
[state-migrations]config-health conflict warnings on every CLI command or gateway bootstrap. Once doctor archives an existing legacy sidecar, the macOS app will not recreate it.The only behavior intentionally lost is cross-session macOS-only config health baseline persistence; canonical config health state belongs to Node core's shared SQLite store.
Evidence
git diff --check origin/main...HEADpassed.node scripts/run-vitest.mjs src/infra/state-migrations.test.tspassed: 1 file, 38 tests.macos-swiftandmacos-nodeon PR head8cbc4e10d3b88bdd3f99f987c06d351e62a3c2d5.codex review --base origin/mainreported no actionable correctness findings.rg -n "configHealthFileName|configHealthStateURL|readConfigHealthState|writeConfigHealthState|config-health\\.json" apps/macos/Sources/OpenClaw/OpenClawConfigFile.swift apps/macos/Tests/OpenClawIPCTests/OpenClawConfigFileTests.swift src/infra/state-migrations.tsnow finds the legacy path only in migration code and regression tests.OpenClawConfigFile.swiftimplementation fromorigin/mainand from PR head, using real macOS filesystem state under a tempOPENCLAW_STATE_DIR. The harness ransaveDict(), deletedlogs/config-health.jsonif present, then ranloadDict()twice. This reproduces the old sidecar recreation loop and shows the PR stops it:This is not a full GUI smoke, but it exercises the same macOS config file reader/writer path that the app calls (
OpenClawConfigFile.loadDict/observeConfigRead) and proves the retired sidecar is no longer recreated after deletion.Attempted
swift test --filter OpenClawConfigFileTestsinapps/macos; it is blocked locally before the touched tests run because this machine only has Command Line Tools, not full Xcode. The compile failure isChatView+Previews.swiftfailing to resolvePreviewsMacros.SwiftUIViewfor#Preview, andxcodebuild -versionreports the active developer directory is/Library/Developer/CommandLineTools. GitHub'smacos-swiftCI lane does have the required Xcode environment and passed.AI-assisted (Codex)