fix: ensure gateway restarts on install failure via ERR trap

[Zhe-SH-CN]

Description

When set -euo pipefail causes the install script to exit early (e.g. npm install fails, tarball extraction fails), the OpenClaw gateway was already stopped but never restarted, leaving the service down.

This PR adds an ERR trap after gateway stop in both install scripts (memos-local-plugin/install.sh and memos-local-openclaw/install.sh) that restarts the gateway on failure. The trap is cleared once the normal gateway start step succeeds.

Related Issue: Fixes #1391

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested

• bash -n syntax check passes for both scripts
• Manual code review confirming trap lifecycle (set after stop, cleared after start)

Checklist

• I have performed a self-review of my own code
• I have commented my code in hard-to-understand areas
• I have linked the issue to this PR

[Memtensor-AI]

🤖 Open Code Review

Target: PR #2152
Task: 8d2f1b1cb54362c9
Base: main
Head: fix/install-trap-gateway-restart

🔍 OpenCodeReview found 4 issue(s) in this PR.

---

1. apps/memos-local-openclaw/install.sh (L232-L234)

The cleanup_on_exit function references ${OPENCLAW_BIN} directly, but OPENCLAW_BIN is not passed or validated inside the function. If the trap fires in a rare scenario where OPENCLAW_BIN is empty or unset (e.g., early exit before it is assigned), the command degenerates to gateway start 2>&1, which will fail silently in an unexpected way.

The sibling file apps/memos-local-plugin/install.sh uses a dedicated GATEWAY_RECOVERY_BIN variable that is explicitly set to the binary path only after the gateway is confirmed stopped, and is checked with -n before executing. The same guard should be applied here:

if [[ "${GATEWAY_RECOVERY_STATE:-inactive}" == "needs_recovery" && -n "${OPENCLAW_BIN:-}" ]]; then

💡 Suggested Change

Before:

  if [[ "${GATEWAY_RECOVERY_STATE:-inactive}" == "needs_recovery" ]]; then
    local recovery_out=""
    if ! recovery_out="$("${OPENCLAW_BIN}" gateway start 2>&1)"; then

After:

  if [[ "${GATEWAY_RECOVERY_STATE:-inactive}" == "needs_recovery" && -n "${OPENCLAW_BIN:-}" ]]; then
    local recovery_out=""
    if ! recovery_out="$("${OPENCLAW_BIN}" gateway start 2>&1)"; then

---

2. apps/memos-local-openclaw/install.sh (L339)

GATEWAY_RECOVERY_STATE is set to "needs_recovery" immediately after gateway stop, but there is no sleep or wait to confirm the gateway has actually stopped before continuing. The sibling script apps/memos-local-plugin/install.sh adds a sleep 1 after stopping. Without any delay, the subsequent lsof port-check and reinstallation may race against a still-running gateway process, leading to port conflicts or file-lock issues.

💡 Suggested Change

Before:

GATEWAY_RECOVERY_STATE="needs_recovery"

After:

GATEWAY_RECOVERY_STATE="needs_recovery"
sleep 1

---

3. apps/memos-local-openclaw/install.sh (L356)

TMP_PACK_DIR is initialized to "" at the top of the script, but is only populated inside the if [[ -f "${PLUGIN_VERSION}" ]] / else block later. If mktemp -d fails (e.g., disk full), TMP_PACK_DIR will be empty and the cleanup guard [[ -n "${TMP_PACK_DIR:-}" ]] correctly skips removal — that part is fine. However, if the early exit 1 branches between lines 415-418 are taken (dependency failure), TMP_PACK_DIR may already be populated and the EXIT trap will correctly clean it up. This is actually correct behavior, but it is worth confirming that every code path that populates TMP_PACK_DIR is covered by the trap, since the old per-trap placement on the same line was removed.

💡 Suggested Change

Before:

TMP_PACK_DIR="$(mktemp -d)"

After:

TMP_PACK_DIR="$(mktemp -d)" || { error "Failed to create temp directory"; exit 1; }

---

4. apps/memos-local-plugin/install.sh (L660-L668)

The foreground gateway fallback path (nohup + wait_for_viewer) does not reset GATEWAY_RECOVERY_STATE to "inactive" before returning. If the viewer becomes reachable via the nohup fallback and the function returns at line 667, the EXIT trap will still see GATEWAY_RECOVERY_STATE="needs_recovery" and attempt an additional gateway start against an already-running process.

The same GATEWAY_RECOVERY_STATE="inactive" / GATEWAY_RECOVERY_BIN="" pair that guards the success path at lines 647-648 should be applied here before each early return in the fallback block.

💡 Suggested Change

Before:

  nohup "${oc_bin}" gateway >/tmp/openclaw-memos-gateway.log 2>&1 &
  sleep 2
  if wait_for_viewer "${OPENCLAW_PORT}"; then
    echo
    success "OpenClaw install complete"
    printf "       ${DIM}Plugin:${NC}    %s\n" "${HOME}/.openclaw/extensions/${PLUGIN_ID}"
    printf "       ${DIM}Viewer:${NC}    ${CYAN}http://127.0.0.1:${OPENCLAW_PORT}/${NC}\n"
    return 0
  fi

After:

  nohup "${oc_bin}" gateway >/tmp/openclaw-memos-gateway.log 2>&1 &
  sleep 2
  if wait_for_viewer "${OPENCLAW_PORT}"; then
    echo
    success "OpenClaw install complete"
    printf "       ${DIM}Plugin:${NC}    %s\n" "${HOME}/.openclaw/extensions/${PLUGIN_ID}"
    printf "       ${DIM}Viewer:${NC}    ${CYAN}http://127.0.0.1:${OPENCLAW_PORT}/${NC}\n"
    GATEWAY_RECOVERY_STATE="inactive"
    GATEWAY_RECOVERY_BIN=""
    return 0
  fi

Generated by cloud-assistant via Open Code Review.

[Memtensor-AI]

⚠️ Automated Test Results: INCONCLUSIVE

Automated tests inconclusive (auto-generated test defect); treated as non-blocking. Manual review recommended. Details: Test failures are unrelated to the diff, which only modifies install.sh scripts. The failures are pre-existing test defects: one has an incorrect status assertion, and another references a non-existent 'role' column in the traces table.

Branch: fix/install-trap-gateway-restart

[WeiminLee]

I need to change my previous approval. This PR is not ready to merge in its current state.

Blocking findings:

1. The branch is currently conflicting against main. gh reports mergeable=CONFLICTING, and merge-tree shows conflicts in unrelated files such as apps/memos-local-plugin/adapters/hermes/memos_provider/__init__.py, pyproject.toml, src/memos/__init__.py, src/memos/api/client.py, src/memos/log.py, tests/api/test_client.py, and tests/test_log.py.

2. The PR is not scoped to the stated install failure fix. It includes API client/model changes, local-plugin LLM changes, lifecycle/logging changes, scheduler docs, version bumps, and large API tests. Please rebase onto current main and reduce this PR to the gateway restart change only, ideally just the affected install scripts and focused tests/manual validation notes.

3. The current install-script fix uses an ERR trap after stopping the gateway, but the normal gateway start path is not explicitly checked before clearing the trap. If the final start command fails, the recovery behavior is ambiguous and may attempt another start via the ERR trap. Please handle the final start explicitly and clear recovery state only after a successful restart.

Please update the branch and rerun CI after narrowing the diff.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

[syzsunshine219]

Updated the original branch with merge commit 2e07638f:

• Merged current main and resolved all 8 unrelated conflicts in favor of main.
• Narrowed the final PR diff from 22 files to 3 files: the two installers plus a focused regression test.
• Replaced the ERR traps with one composable EXIT cleanup path, so temporary-directory cleanup is preserved.
• Recovery is armed only after gateway stop; failures before the normal restart recover the gateway once.
• The final gateway start is checked explicitly. A failed final start exits non-zero and is not retried by the exit trap; recovery state is cleared only after success.
• Added fault-injection coverage for broken tar extraction, failed npm pack, and failed final gateway start.

Validation:

• bash -n passed for both installers.
• Focused tests: 3/3 passed.
• Full local-plugin suite: 1305 passed, 2 skipped across 160 files.
• npm run lint passed.
• npm run build passed.
• Final diff and whitespace checks passed.

[syzsunshine219]

Follow-up pushed in 2d302f60 to address the latest OCR feedback:

• warnings/errors now consistently use stderr in the legacy installer
• the legacy installer reports success only after gateway start succeeds
• recovery state now distinguishes an install-time failure from a confirmed final-start failure
• a confirmed final-start failure is intentionally not retried from the EXIT cleanup, matching the human review request to avoid a second ambiguous start attempt
• regression coverage now checks one final start attempt, nonzero exit, failure output, and no false success output

Validation: 160 test files passed (1305 tests passed, 2 skipped), targeted recovery tests 3/3, lint, build, both bash -n checks, and git diff --check.

[syzsunshine219]

OCR follow-up in 2b37f109:

• The reported foreground-fallback double start was a false positive: GATEWAY_RECOVERY_BIN is cleared unconditionally before the viewer wait/fallback path. I added an explicit ordering comment and a regression test that exercises this path; the observed calls are exactly gateway stop, gateway start, then the foreground gateway command, with no EXIT-trap restart.
• The recovery variables intentionally remain global because the EXIT trap must read them after install_openclaw returns. The top-level dispatch invokes install_openclaw at most once; this constraint and trap lifetime are now documented together.

Validation remains green: targeted recovery tests 4/4; full suite 160 files, 1306 passed and 2 skipped; lint, build, shell syntax, and diff checks passed.

[syzsunshine219]

OCR follow-up pushed in b3fc2167:

Addressed:

• both installers now retain and print the original gateway start output when EXIT recovery fails
• recovery control is a single explicit state (inactive, needs_recovery, final_failed) instead of two booleans
• tests assert that recovery diagnostics include the gateway error

Not applicable:

• the legacy installer already disables recovery on the first command after a successful final start; separate INT/TERM traps do not remove the shell-level gap between two commands
• a failed final start returns from install_openclaw; it cannot fall through into the later foreground fallback block. The fallback test also confirms no second service start occurs

Validation: targeted tests 4/4; full suite 160 files, 1306 passed and 2 skipped; lint, build, both shell syntax checks, and diff checks passed.

[Memtensor-AI]

⚠️ Automated Test Results: INCONCLUSIVE

Automated tests inconclusive (flaky/non-deterministic test); treated as non-blocking. Manual review recommended. Details: The failing test asserts an old task's status is 'completed' or 'skipped' after a session change, but the status was 'active'. This test is unrelated to the diff, which only touches install.sh shell scripts and a new plugin test file. [advisory, non-gating] AI-generated tests on branch test/auto-gen-8d2f1b1cb54362c9-20260806222544: 47/51 passed, 4 failed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix/install-trap-gateway-restart