Skip to content

fix(installer): restart upgraded Hermes/OpenClaw agents#835

Open
agent-p1p wants to merge 1 commit into
masterfrom
pip/mdk-834
Open

fix(installer): restart upgraded Hermes/OpenClaw agents#835
agent-p1p wants to merge 1 commit into
masterfrom
pip/mdk-834

Conversation

@agent-p1p

@agent-p1p agent-p1p commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • restart active Hermes/OpenClaw wn-agent systemd user services after replacing their binaries and units
  • retain enable --now for fresh/inactive installs
  • preserve --no-start-wn-agent and --no-service behavior
  • add a shared deterministic installer harness covering fresh, active-upgrade, no-start, and no-service paths for both connectors

Verification

  • integrations/test_installer_systemd_service.sh hermes
  • integrations/test_installer_systemd_service.sh openclaw
  • integrations/hermes/marmot/tests/test_dev_scripts.sh
  • integrations/openclaw/marmot/test/dev-scripts.sh (196 passed, 1 skipped per setup run)
  • just fast-ci

Fixes #834


Open in Stage

Summary by CodeRabbit

  • New Features

    • Linux installations now enable and start the agent service when inactive.
    • Existing active services are enabled as needed and restarted to apply updates.
    • Dry-run output clearly describes the service action that will be performed.
  • Bug Fixes

    • Improved fallback messaging when system service setup cannot be completed.
  • Tests

    • Added coverage for fresh installs, upgrades, disabled service startup, and environments without the service manager.

Restart active Hermes and OpenClaw wn-agent user services after replacing their binaries and units. Keep enable --now for fresh installs and preserve no-start/no-service paths.

Fixes #834
@stage-review

stage-review Bot commented Jul 12, 2026

Copy link
Copy Markdown

Ready to review this PR? Stage has broken it down into 4 individual chapters for you:

Title
1 Implement service restart logic in Hermes installer
2 Implement service restart logic in OpenClaw installer
3 Add shared systemd service installer test harness
4 Integrate systemd tests into developer scripts
Open in Stage

Chapters generated by Stage for commit 732f891 on Jul 12, 2026 6:27pm UTC.

@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The installers now conditionally start or restart their systemd user services. A shared integration test mocks installer dependencies and validates fresh, upgrade, and disabled-service scenarios for Hermes and OpenClaw, while development scripts assert the updated Linux dry-run output.

Changes

Systemd service restart behavior

Layer / File(s) Summary
Conditional service activation
scripts/install-hermes-marmot.sh, scripts/install-openclaw-marmot.sh
Both installers restart active services and enable/start inactive services, with updated dry-run and fallback messages.
Mocked installer scenarios
integrations/test_installer_systemd_service.sh
Adds deterministic coverage for fresh, upgrade, no-start, and no-service cases using mocked commands and systemctl logs.
Integration test wiring and dry-run assertions
integrations/hermes/..., integrations/openclaw/...
Development scripts invoke the shared systemd test and verify Linux dry-run output for service restart behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

  • #832: Covers the same active-service restart behavior and fresh-versus-upgrade installer testing for Hermes and OpenClaw.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes restart active wn-agent services, keep enable --now for inactive installs, preserve no-start/no-service behavior, and add deterministic coverage.
Out of Scope Changes check ✅ Passed The added test harness and dev script assertions are support work for the installer behavior and stay within the linked issue scope.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: upgraded Hermes/OpenClaw agents now restart instead of only starting fresh installs.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pip/mdk-834

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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
scripts/install-openclaw-marmot.sh (1)

395-403: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider extracting shared installer logic.

enable_or_restart_systemd_user_service and install_linux_user_service are duplicated verbatim between install-hermes-marmot.sh and install-openclaw-marmot.sh. While these are intentionally self-contained scripts, a shared lib sourced at runtime could reduce maintenance risk when the systemd activation logic evolves.

🤖 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/install-openclaw-marmot.sh` around lines 395 - 403, Extract the
duplicated systemd installer helpers, including
enable_or_restart_systemd_user_service and install_linux_user_service, into a
shared runtime-sourced library used by both installer scripts. Remove their
duplicate definitions from each script while preserving self-contained execution
and the existing activation behavior.
🤖 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.

Nitpick comments:
In `@scripts/install-openclaw-marmot.sh`:
- Around line 395-403: Extract the duplicated systemd installer helpers,
including enable_or_restart_systemd_user_service and install_linux_user_service,
into a shared runtime-sourced library used by both installer scripts. Remove
their duplicate definitions from each script while preserving self-contained
execution and the existing activation behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e321cdd7-0c57-4096-a88d-8c1f729453ff

📥 Commits

Reviewing files that changed from the base of the PR and between e391adc and 732f891.

📒 Files selected for processing (5)
  • integrations/hermes/marmot/tests/test_dev_scripts.sh
  • integrations/openclaw/marmot/test/dev-scripts.sh
  • integrations/test_installer_systemd_service.sh
  • scripts/install-hermes-marmot.sh
  • scripts/install-openclaw-marmot.sh

@agent-p1p agent-p1p left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pip adversarial review

Head: 732f891a06250cde8054bdc79d6a46c463ce3354

BLOCKING

None.

SUGGESTION

None.

NITPICK

None.

Assessment

The active-service branch enables and restarts the existing unit after daemon-reload; the inactive/fresh branch retains enable --now. The upstream --no-start-wn-agent and --no-service gates still bypass service installation. The shared harness exercises the real installer control flow with exact systemctl call assertions for all four cases on both connectors.

The CodeRabbit shared-library nitpick is not requested: these are standalone release installers, and adding a runtime-sourced helper would increase installation failure surface for a small duplicated helper. The shared behavioral harness already guards drift.

Verification

  • bash integrations/test_installer_systemd_service.sh hermes — pass
  • bash integrations/test_installer_systemd_service.sh openclaw — pass
  • integrations/hermes/marmot/tests/test_dev_scripts.sh — pass
  • integrations/openclaw/marmot/test/dev-scripts.sh — pass; 196 tests passed, 1 skipped per setup run
  • shell syntax and git diff --check — pass
  • GitHub checks — all completed checks green, including CodeRabbit

VERDICT: APPROVE

Sensitive paths: none under the task-configured sensitive-path rubric.

@agent-p1p
agent-p1p marked this pull request as ready for review July 12, 2026 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hermes/OpenClaw installers: restart active wn-agent systemd service after upgrade

1 participant