Skip to content

fix(update): recover source transactions after process death - #2709

Closed
tang-vu wants to merge 3 commits into
Osmantic:mainfrom
tang-vu:fix/update-source-transaction-rollback
Closed

tang-vu wants to merge 3 commits into
Osmantic:mainfrom
tang-vu:fix/update-source-transaction-rollback

Conversation

@tang-vu

@tang-vu tang-vu commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor

Ready for independent review (2026-09-15). Candidate 357c157e050e6deb92617b23f1646f695dae6541 has a successful GitHub check rollup and no base-branch conflict at this review snapshot. This supersedes earlier draft-status wording only. All validation gaps, migration requirements, live gates and independent human review requirements below remain in force; this is not a merge-ready approval.

Why this matters

ods-update.sh update mutates source, configuration, migrations, and the running Compose stack as one operator action. Before this PR, normal migration/health failures restored configuration but left the checkout on the failed release. Even after adding source rollback, an uncatchable exit such as host reboot, power loss, or SIGKILL could strand the installation between phases because all rollback context lived only in shell variables.

The reachable path is the shipped source-checkout command itself and the dashboard host-agent update endpoint that invokes it. A later attempt previously had no durable proof of the pre-update revision, rollback snapshot, compose overlays, or last completed phase.

Root cause and invariant

Root cause: update rollback was process-local. The updater had no cross-process owner lock and no durable transaction record linking source, snapshot, compose plan, phase, and recovery result.

Invariant: before the first source mutation, ODS durably records enough information to restore the exact prior source/config/runtime state. A later update must recover or report an explicit incomplete state before starting new mutation, and automatic recovery must never overwrite tracked operator edits made after the interruption.

What changed

  • Refuse source updates when tracked operator changes make an automatic reset destructive.
  • Capture the exact pre-pull commit and restore it before runtime configuration on normal failures.
  • Serialize direct CLI and host-agent invocations with an owner-recorded, cross-process update lock.
  • Reclaim a lock only when its recorded PID is no longer alive; malformed owner metadata is fail-closed for manual inspection.
  • Atomically publish data/update-transaction.json with phase receipts:
    prepared -> source_updated -> migrations_complete -> runtime_restarted -> verified -> committed.
  • On the next update, detect any non-terminal journal and restore the recorded source revision, snapshot, and compose stack before allowing another release attempt.
  • Preserve post-crash tracked edits and publish recovery_required rather than resetting them.
  • Publish terminal rolled_back, recovered, abandoned, or recovery_failed receipts and expose the latest state through ods-update.sh status.
  • Treat rollback restart as failed unless either Compose v2 or the v1 fallback actually starts the previous stack.

Overlap check

Searched open and closed PRs for source rollback, update journal, interrupted update, crash recovery, update lock, and PRs changing ods/ods-update.sh.

None persists an in-flight source update across process death, performs next-run recovery, or excludes concurrent updater processes. This remains an extension of the original #2709 source-rollback scope rather than a competing PR.

Regression tests

bash tests/test-update-source-rollback.sh uses a real bare Git remote and the public updater. It proves:

  • a failing migration restores the exact pre-pull commit and removes files introduced by the failed release;
  • dirty tracked source is rejected before snapshot/fetch mutation;
  • a live lock rejects a concurrent public update command;
  • a migration that sends real SIGKILL to its parent leaves a source_updated journal and stale owner lock;
  • first recovery preserves a tracked edit made after the crash and publishes recovery_required;
  • after the edit is resolved, next-run recovery restores source/config/runtime, publishes recovered, and exposes that receipt through public status.

Focused validation:

  • bash tests/test-update-source-rollback.sh — 5 passed
  • bash tests/test-ods-update-runtime-compose.sh — 5 passed
  • bash tests/test-rollback-compose-stack.sh — 7 passed
  • bash tests/test-update-script-backup.sh — 6 passed
  • bash -n ods-update.sh tests/test-update-source-rollback.sh — passed
  • make lint — passed
  • git diff --check — passed

make test progressed through the model/tier suites (129 passing assertions) but stopped at the pre-existing test-installer-context-parity.sh assertion Hermes template bounds each model turn. This branch does not change that test or any Hermes template; the same files are identical to upstream/main. Local ShellCheck is unavailable in this Windows environment, so CI owns the exact ShellCheck validation.

Design, platform, and rollback

The journal contains no secrets: only schema/state, Git revision, snapshot path, compose flags, timestamps, and an outcome reason. It and the lock owner file are written mode 0600; phase changes use temp-file plus rename. The implementation uses Bash, mkdir, mv, kill -0, Git, and jq on Linux, macOS, WSL2, and Git Bash paths already supported by this script.

Recovery deliberately stops after restoring the previous release; it does not immediately retry the same candidate that just killed the updater. Invalid journal/snapshot paths and malformed lock ownership fail closed. The snapshot is retained for operator inspection and explicit rollback.

Reverting this PR removes automatic source restoration, exclusion, and crash recovery; existing rollback snapshots remain readable. This is lifecycle-critical code and should receive independent human review before merge.

@tang-vu tang-vu changed the title fix(update): roll source back with failed releases fix(update): recover source transactions after process death Aug 27, 2026
@tang-vu
tang-vu marked this pull request as draft August 27, 2026 06:15
@tang-vu

tang-vu commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Batch compatibility validation: synthetic integration head 5b74b8b58058a969838ad9738f419be7eb907151 combines PR #2190, PR #3259, PR #3261, and this head. Source process-death recovery, deployment receipt/status (12/12), backup coverage, application-consistent backup, integrity, round-trip restore, rollback, and combined Bash syntax suites all passed. No production-file conflict was required for this PR.

@Lightheartdevs

Copy link
Copy Markdown
Collaborator

Thanks for this contribution. We're closing outside-contributor pull requests that have had no activity for 14 days or more, so the queue reflects work that can still land. This isn't a judgment on the change itself. If it's still needed, please rebase onto main and reopen it or open a focused PR. See #7253 for details and the contribution policy.

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.

2 participants