fix: accept reformatted launchd service ownership

[TheAngryPit]

Closes #147.

What Problem This Solves

Editing an OCM LaunchAgent with PlistBuddy can change XML formatting without changing its store. OCM then incorrectly refuses service lifecycle commands as belonging to a different store.

User Impact

Same-store XML and binary plists remain manageable after reformatting. Foreign, missing, malformed, and incorrectly typed owners still fail closed. Systemd ownership behavior is preserved.

Why This Change Was Made

Read EnvironmentVariables.OCM_HOME as a plist value. The current-main adaptation puts this in the shared ownership helper used by service inspection and lifecycle operations. The parser supports Rust 1.88; the lockfile adds only plist and quick-xml. Includes the contributor's parser controls, a CLI regression, usage documentation, and a changelog credit to @TheAngryPit.

Evidence

The new CLI regression first failed on unchanged main with the different-store error after an indentation-only edit. It now passes through fresh service setup, native macOS PlistBuddy reserialization, same-store start, foreign-store refusal without rewriting the definition, and owner stop. Service-manager calls use the repository's isolated synthetic fixtures; no installed service was modified. Parser tests also cover binary plists, escaped paths, comment decoys, missing/wrongly typed owners, and malformed input.

The same regression also passes against the optimized binary produced by cargo install --locked --path . into an isolated prefix. Formatting, all-target checking, parser controls, and independent P0–P2 review pass. Python release-boundary (33) and npm (21) tests pass with native gh scoped to their loopback fixture transport. Full Rust and exact-head CI results will be recorded after completion. Native launchd bootstrap on an installed service was not exercised; the CLI and native plist serializer were exercised against synthetic definitions.

The full gate exposed three existing fixture problems, repaired without changing production behavior: wait for a resumed child marker before terminating it, keep rollback HTTP health available until teardown, and copy the npm executable fixture when the build and temp directories are on different filesystems.

Broad validation on an isolated Linux runner covers the full Rust suite: the full run passed unaffected targets, and a clean rebuild passed all 96 tests in the three npm-fixture targets after fixing the cross-filesystem fixture. The shared Mac's broad run also exposed lifecycle timing failures; the changed ownership path and installed-release regression pass natively. Final committed-patch review is clean at P0–P2. Exact-head GitHub CI remains the merge gate.

All nine cross-platform CI jobs passed at faea648f2a55a1dbf6b92f4a2d9eb19e307a6a13: https://github.com/openclaw/ocm/actions/runs/35052552513.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed September 15, 2026, 11:50 PM ET / September 16, 2026, 03:50 UTC (Revision 5).

ClawSweeper review

What this changes

Parse macOS service ownership from XML or binary plists, document the behavior, add ownership regressions, and repair three test fixtures.

Merge readiness

⛔ Blocked before merge - 2 items remain

The fix remains necessary on main and v0.2.47. No actionable patch defect was found, but the prior native service proof request remains unsatisfied.

Priority: P2
Reviewed head: faea648f2a55a1dbf6b92f4a2d9eb19e307a6a13

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The patch is focused and source-correct, but real service behavior remains supported only by fixture-backed integration results.
Proof confidence	🦪 silver shellfish (2/6)	Needs real behavior proof before merge: The body reports the real OCM CLI and native PlistBuddy exercising the changed parser, including fresh and reformatted definitions and foreign-store refusal, but launchctl remains a fixture. These integration results do not satisfy the prior request for an isolated native service-entrypoint transcript showing after-fix operation and refusal without modifying the foreign service. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: The body reports the real OCM CLI and native PlistBuddy exercising the changed parser, including fresh and reformatted definitions and foreign-store refusal, but launchctl remains a fixture. These integration results do not satisfy the prior request for an isolated native service-entrypoint transcript showing after-fix operation and refusal without modifying the foreign service. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	8 items	Current main still has the defect: The released main revision reads UTF-8 text and requires an exact newline and six-space indentation before the owner string. Formatting-only edits therefore still fail ownership matching. Release boundary: The local v0.2.47 tag points at the inspected main revision, which still contains the formatting-sensitive check. Structural ownership validation: The introduced helper parses bytes and accepts only a string at EnvironmentVariables.OCM_HOME equal to the requesting store. Definition replacement and deactivation retain ownership validation; systemd keeps both existing escaped markers.
Findings	None	None.
Security	None	None.

How this fits together

OCM manages background OpenClaw services through launchd on macOS. Its ownership check compares the service definition’s store with the requesting store before allowing service changes.

flowchart TD
  A[Service command] --> B[Requesting OCM store]
  C[LaunchAgent plist] --> D[Parse environment dictionary]
  B --> E{Store matches?}
  D --> E
  E -->|Yes| F[Allow service operation]
  E -->|No or invalid| G[Reject operation]

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: The body reports the real OCM CLI and native PlistBuddy exercising the changed parser, including fresh and reformatted definitions and foreign-store refusal, but launchctl remains a fixture. These integration results do not satisfy the prior request for an isolated native service-entrypoint transcript showing after-fix operation and refusal without modifying the foreign service. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Complete next step (P2) - Provide the isolated native macOS service transcript requested above; terminal screenshots or recordings are welcome, and copied output or logs count. Redact private paths, addresses, credentials, and endpoints. Update the PR body to trigger re-review; if it does not run, ask a maintainer to comment @clawsweeper re-review.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	Production +37/-16; tests +121/-12	Production growth is justified by structural plist parsing; test totals include inline parser coverage and three fixture repairs.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #147
Summary: This PR is the candidate fix for the formatting-sensitive ownership report; bootstrap-log placement is separate.

Members:

• canonical: Service ownership rejects a plist after formatting-only changes #147 - Reports the exact ownership failure addressed by this patch.
• adjacent_distinct: macOS supervisor cannot start with bootstrap logs on an affected external volume #148 - Concerns launchd access to bootstrap logs on an affected external volume.
• adjacent_distinct: fix: allow a separate supervisor bootstrap log directory #150 - Proposes a separate bootstrap-log location override.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Technical review

Best possible solution:

Retain the shared structural parser and exact store comparison, with native acceptance evidence covering existing reformatted services and foreign-store refusal.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: changing indentation before the generated owner string defeats main’s exact substring check despite preserving the store value. This review did not execute the regression.

Is this the best way to solve the issue?

Yes. Reading the existing ownership field structurally in the shared helper repairs the serialization dependency without adding a competing ownership mechanism.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against b7e2802d9ae0.

Labels

Label justifications:

• P2: This repairs a bounded macOS service lifecycle failure after a plist-aware editor changes formatting.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The body reports the real OCM CLI and native PlistBuddy exercising the changed parser, including fresh and reformatted definitions and foreign-store refusal, but launchctl remains a fixture. These integration results do not satisfy the prior request for an isolated native service-entrypoint transcript showing after-fix operation and refusal without modifying the foreign service. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

• Current main still has the defect: The released main revision reads UTF-8 text and requires an exact newline and six-space indentation before the owner string. Formatting-only edits therefore still fail ownership matching. (src/service/platform.rs:303, b7e2802d9ae0)
• Release boundary: The local v0.2.47 tag points at the inspected main revision, which still contains the formatting-sensitive check. (b7e2802d9ae0)
• Structural ownership validation: The introduced helper parses bytes and accepts only a string at EnvironmentVariables.OCM_HOME equal to the requesting store. Definition replacement and deactivation retain ownership validation; systemd keeps both existing escaped markers. (src/service/platform.rs:304, faea648f2a55)
• Proof coverage and continuity: The complete captured body reports before/after CLI regression results, native PlistBuddy serialization, an optimized installed binary, fresh setup, same-store reuse, and foreign-store rejection. It explicitly says service-manager calls use synthetic fixtures and native launchd bootstrap was not exercised. The source confirms launchd_env installs fake launchctl. This leaves the previous review’s isolated native service-entrypoint transcript request open. Captured sourceRevision: ddb5c9743cf7bab5ad0a3bfc1472fa5a178b9f83ebad613398fd130c24c4021f; the live body read matched the supplied body. (tests/service_command_tests.rs:26, faea648f2a55)
• Negative controls: Parser coverage includes binary serialization, escaped paths, foreign owners, misplaced keys, wrong types, comment decoys, missing ownership, and malformed input. The CLI regression verifies foreign-store refusal leaves the definition unchanged. (src/service/platform.rs:1749, faea648f2a55)
• Relevant history: Current-main file history identifies recent service work, and GitHub commit inspection verifies the shared ownership-helper extraction. Deeper local pickaxe and blame attempts failed on an unavailable historical object, so no feature-introduction attribution is claimed. (src/service/platform.rs, e74395dc7db8)

Likely related people:

• shakkernerd: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Add a redacted native macOS transcript from an isolated setup showing fresh service creation, same-store operation after reformatting, and foreign-store refusal without altering the existing service.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (4 earlier review cycles)

• reviewed 2026-09-05T22:01:07.267Z sha 268df51 :: needs real behavior proof before merge. :: none
• reviewed 2026-09-05T22:41:28.192Z sha 268df51 :: needs real behavior proof before merge. :: none
• reviewed 2026-09-05T23:04:23.899Z sha 268df51 :: needs real behavior proof before merge. :: none
• reviewed 2026-09-16T03:42:38.277Z sha faea648 :: needs real behavior proof before merge. :: none