Skip to content

fix(update): keep host-agent refresh non-fatal - #3159

Open
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/update-host-agent-restart-nonfatal
Open

fix(update): keep host-agent refresh non-fatal#3159
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/update-host-agent-restart-nonfatal

Conversation

@tang-vu

@tang-vu tang-vu commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Why this matters

A successful ods update currently exits before Update complete when the optional host-agent refresh cannot start. cmd_update labels that refresh non-fatal and wraps it with || warn, but cmd_agent reports hard lifecycle failures through error(), which exits the current shell instead of returning. The caller's fallback therefore never runs. This is reachable on older/partial installs without bin/ods-host-agent.py, on hosts missing Python, and when the refreshed agent cannot become healthy; the Compose update has already verified and persisted its new version by then.

Run on current upstream/main, the public CLI fixture reproduced the abort with ODS host agent script not found after service verification and version persistence.

This change runs only the optional agent restart in a subshell. Agent stop/start side effects still occur normally, but an internal exit is contained and converted into the existing warning so the verified update completes.

Behavioral invariant: failure of the optional post-update host-agent refresh must be visible but must not roll an otherwise verified update into a false CLI failure.

Overlap check

Searched open and closed PRs for CLI update host agent restart verification, host agent restart non-fatal update, and the exact Restarting host agent text, plus open tang-vu PRs changing update paths and ods-cli. No PR covers this failure. #2948 changes Windows OpenCode scheduled-task restart ownership; #2709 and #3066 change source/backup transactions in ods-update.sh; #3156 changes custom install-root resolution. This scope is independent of #3158 and has no merge-order dependency.

Changed production file searched: ods/ods-cli around cmd_update and cmd_agent.

Regression test

tests/test-cli-update-verification.sh invokes the real ods update command against executable Docker/curl fixtures. The fixture intentionally omits the host-agent script, now asserts the non-fatal warning was exercised, and proves the command still reaches Update complete, persists the target version, verifies service failures honestly, and filters local build tags from pulls.

Validation

  • bash tests/test-cli-update-verification.sh ? 7 passed, 0 failed
  • bash tests/test-ods-cli-pipefail-tolerance.sh ? 16 passed, 0 failed
  • bash -n ods-cli ? passed
  • bash -n tests/test-cli-update-verification.sh ? passed
  • git diff --check ? passed

The failure was also reproduced from a clean LF Linux worktree on the integration head after #3158. A live daemon upgrade was not performed; the boundary fixture proves the real CLI orchestration and executable failure path without mutating a host service.

Tradeoffs and rollback

The subshell is scoped to this best-effort refresh rather than weakening cmd_agent itself: direct ods agent start/restart commands must continue to fail when lifecycle requirements are missing. Rollback is isolated to the call site, but restores the mismatch between the documented non-fatal policy and actual shell exit behavior.

Batch compatibility

This PR was validated on synthetic integration head 9af795fc, which applies #3158 through #3167 in numeric order on upstream/main (6ff9b4fc). Combined make lint, make test, make smoke, make simulate, and all 418 BATS cases passed (one root-specific permission assertion skipped by design).

Recommended merge order: #3158 ? #3159 ? #3160 ? #3161 ? #3162 ? #3163 ? #3164 ? #3165 ? #3166 ? #3167. The only manual reconciliation observed was the adjacent Makefile test insertion shared by #3164 and #3166; retain both test-unix-restart-recreate-env.sh and test-chat-error-exit-parity.sh lines. Production code merged automatically across the full batch.

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.

1 participant