fix(scripts): record deployed version at provisioning and pre-update snapshot (NEH-216) - #289
Merged
Merged
Conversation
…pdate snapshot (NEH-216) setup.sh now writes last-deployed-SHA when provisioning finishes, so a unit's first update — the largest jump it will ever make — has a real code-rollback target and a precise offline-safety answer instead of the conservative warning. git runs as the repo owner (root-run git hits dubious-ownership on a user-cloned repo and would poison the record). update.sh records an informational pre-update-state snapshot (superproject SHA, submodule SHAs, pixi version) before any build or dependency work, for manual recovery when an update aborts early. rollback-update.sh never reads it: the dump and pre-update-SHA are still written together in step 5 so they always pair — writing the SHA early instead could pair a fresh SHA with a stale dump and restore data-destroying state.
There was a problem hiding this comment.
Pull request overview
Adds deployment-state recording to improve first-update recovery.
Changes:
- Records the provisioned repository SHA during setup.
- Captures pre-update repository, submodule, and Pixi state.
- Retains the five newest state snapshots.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
scripts/setup.sh |
Records the initially provisioned SHA. |
scripts/update.sh |
Creates and prunes pre-update state snapshots. |
| if [ -f "$LAST_DEPLOYED_SHA_FILE" ]; then | ||
| echo "previously-deployed: $(cat "$LAST_DEPLOYED_SHA_FILE")" | ||
| else | ||
| echo "previously-deployed: (no record)" |
Comment on lines
+245
to
+247
| # Informational only: rollback-update.sh never reads it. Its inputs — the | ||
| # dump and pre-update-SHA — are still written together in step 5 so they | ||
| # always pair; recording pre-update-SHA this early instead would let an |
Comment on lines
+250
to
+262
| { | ||
| echo "recorded-at: $(date +%Y-%m-%dT%H:%M:%S%z)" | ||
| echo "installing-superproject: $CURRENT_SHA" | ||
| if [ -f "$LAST_DEPLOYED_SHA_FILE" ]; then | ||
| echo "previously-deployed: $(cat "$LAST_DEPLOYED_SHA_FILE")" | ||
| else | ||
| echo "previously-deployed: (no record)" | ||
| fi | ||
| echo "submodules:" | ||
| run_as_user git -C "$PROJECT_ROOT" submodule status 2>/dev/null || echo " (unavailable)" | ||
| echo "pixi: $(run_as_user "$PIXI_BIN" --version 2>/dev/null || echo '(unavailable)')" | ||
| } > "$STATE_FILE" || true | ||
| echo " Pre-update state recorded: $STATE_FILE" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
scripts/update.sh:262
- A failed redirect or write is discarded by
|| true, after which the script unconditionally reports that the snapshot was recorded. On a full or read-only backup filesystem, operators would be told manual recovery data exists when the file is absent or partial. Preserve the informational/non-fatal behavior, but report the failure and remove any partial file.
} > "$STATE_FILE" || true
echo " Pre-update state recorded: $STATE_FILE"
| # user-cloned repo and would silently poison the record. | ||
| PROVISIONED_SHA="$(run_as_user git -C "$PROJECT_ROOT" rev-parse HEAD 2>/dev/null || echo "")" | ||
| if [ -n "$PROVISIONED_SHA" ]; then | ||
| echo "$PROVISIONED_SHA" > /var/lib/dtk/backups/last-deployed-SHA |
| @@ -239,6 +239,32 @@ trap on_exit EXIT | |||
| mkdir -p "$BACKUP_DIR" | |||
| CURRENT_SHA="$(git -C "$PROJECT_ROOT" rev-parse HEAD 2>/dev/null || echo "unknown")" | |||
…rrespondence The pre-update snapshot now reports a write failure instead of always claiming success (informational file, so the update continues either way). The "always pair" comment overstated the dump↔SHA invariant: pre-update-SHA is a single file while five dumps are retained. The comment now states the correspondence holds only for the newest dump, and rollback-update.sh warns before the confirmation prompt when an operator selects an older dump.
Member
Author
|
On the legacy-appliance point: the only deployed unit (Rionegro) gained |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/update.sh:254
- The snapshot can still record
unknownon the supportedsudo ./scripts/update.shpath:CURRENT_SHAis resolved by root at line 240, while this PR itself notes that root-run Git rejects the user-owned clone as dubious ownership. Resolve and verifyCURRENT_SHAthroughrun_as_userbefore it is used here; this also prevents the later deployment record from being poisoned withunknown.
echo "installing-superproject: $CURRENT_SHA"
scripts/setup.sh:272
- Checking only for non-empty output does not reliably detect a Git failure. Without
--verify,git rev-parse HEADcan echo the unresolved tokenHEADand still exit nonzero (for example with an unborn or invalid HEAD); the|| echo ""preserves that output, so this branch writes garbage despite the intended warning. Branch on the command status and require a verified commit object.
PROVISIONED_SHA="$(run_as_user git -C "$PROJECT_ROOT" rev-parse HEAD 2>/dev/null || echo "")"
if [ -n "$PROVISIONED_SHA" ]; then
…rd "unknown" CURRENT_SHA now resolves via run_as_user — root-run git can fail dubious-ownership on the user-cloned repo, and "unknown" would poison the snapshot and, on success, last-deployed-SHA. A successful update also refuses to overwrite a valid record with "unknown". rollback-update.sh validates the rollback target as the repo owner too, matching the checkout below it: a root-side dubious-ownership failure would otherwise falsely report the commit unavailable and skip the code rollback entirely.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/update.sh:243
- The ownership fix is incomplete: the offline-safety check later runs
git cat-file,diff, andrev-parsedirectly as root (lines 297–317). On the user-owned checkout described here, Git rejects those probes as dubious ownership, so the newly provisioned SHA is treated as unreachable and every first update takes the “cannot confirm offline-safe” warning path. Route all of those Git probes throughrun_as_useras well.
# git runs as the repo owner: root-run git can fail dubious-ownership on the
# user-cloned repo, and "unknown" here would poison both the snapshot and
# (on success) last-deployed-SHA.
CURRENT_SHA="$(run_as_user git -C "$PROJECT_ROOT" rev-parse HEAD 2>/dev/null || echo "unknown")"
scripts/rollback-update.sh:129
- This string comparison can label the newest dump as “OLDER” when the operator names the same file through an equivalent path (for example, a symlink or a path containing
..). Compare file identity rather than pathname text; also describe a non-matching external dump as not matching the newest managed dump, since its age is unknown.
NEWEST_DUMP="$(ls -1t "$BACKUP_DIR"/pre-update-*.sql.gz 2>/dev/null | head -n 1 || true)"
if [ -n "$TARGET_SHA" ] && [ -n "$DUMP_ARG" ] && [ "$DUMP_FILE" != "$NEWEST_DUMP" ]; then
echo "⚠ The recorded code-rollback commit ($TARGET_SHA) was written by the"
echo " most recent update, but you selected an OLDER dump. The restored"
echo " database may not correspond to that code version."
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the first-update rollback gap from NEH-216.
The gap
update.shderives its rollback target fromlast-deployed-SHA, written only on a previous successful update. A freshly provisioned unit has no such record, so its first update — Rionegro's 2026-08-03 two-month jump was exactly this — runs with no code-rollback target, androllback-update.shcan restore only the database. The SHAs that made Rionegro recoverable were captured by hand before starting; nothing on the appliance recorded them.Changes
setup.shwriteslast-deployed-SHA(repo HEAD) at the end of provisioning, creating/var/lib/dtk/backupsfirst. git runs as the repo owner viarun_as_user— root-run git hits dubious-ownership on the user-cloned repo and would silently poison the record. A read failure warns instead of writing garbage.update.shwrites an informationalpre-update-state-<timestamp>file as part of step 1, before any build or dependency work: superproject SHA, previously-deployed SHA,git submodule status, andpixi --version. Retention matches the dump policy (newest 5).What deliberately did NOT change
pre-update-SHAstays where it is, written next to the database dump in step 5. Moving it earlier was considered and rejected: an abort during the build would leave a fresh SHA beside a stale dump, and a laterrollback-update.shwould drop the database into a dump up to five updates old while reporting success. The dump and the rollback SHA must always be written as a pair; the new state file is manual-recovery information only and nothing automated reads it.Validation
bash -nclean on both scripts. The full first-update path (provision → record → update → rollback with a realpre-update-SHA) goes on the bench checklist for Friday's bench test — the 2026-07-23 bench only ever exercised the no-record guard, never the actual code-rollback path.Linear: NEH-216 (GitHub mirror #285).