Release corrections round 4: verified pre-deploy backup step and honest rotation-check documentation #124

Merged

approver_agent merged 30 commits from task/release-corrections-4_008c0a8a-2242-429b-a1fd-69346bc1d414 into dev 2026-10-07 20:02:54 -04:00

Conversation 8 Commits 30 Files Changed 2

+439 -55

orchestrator-agent commented 2026-10-07 17:21:16 -04:00

Member

Copy Link

Copy Source

Task

Maestro task 008c0a8a-2242-429b-a1fd-69346bc1d414 — "Release corrections round 4", from the independent release review of release/2026-10-07c (PR #123, APPROVED at fd225b9).

Documentation only: the diff is exactly deploy/RUNBOOK.md and docs/ARCHITECTURE.md.

Findings fixed

• R01 (MEDIUM), deploy/RUNBOOK.md "Routine deploy and rollback": the deploy steps did not require checking a current, restorable Litestream point before the restart applied migrations 0013 and 0014. Because an older SHA refuses the upgraded schema, the only rollback is a restore to a point before the restart — so an operator could restart production on an unverified backup. The Deploy sequence now has a discrete step 2, before sudo systemctl restart mealquest, that checks Litestream is replicating, restores the replica to a scratch file beside the live database (the documented restore command writing elsewhere, so the live files are untouched), compares integrity and counts with the existing Count check script, records the pre-deploy restore point as a UTC timestamp next to the previous SHA, and says plainly "If any check fails, do not restart". The Rollback section now uses that recorded -timestamp instead of describing a separate exercise. No script was added and deploy/compose.yaml is unchanged.
• R02 (LOW), docs/ARCHITECTURE.md: the prose said the rotation_check schedule calls MarkTried, but this release has no production caller for MarkTried — the scheduler is Feature 20, so nothing opens a Rotation check automatically. The prose now says the schedule "is to call" MarkTried and states that this release provides only the MarkTried method and the rotation review read/decide APIs.

Notes for review

• The developer deliberately gave only one remedy when the backup check fails (restore replication and repeat the step) rather than also offering a manual-backup path, per the house rule of one way to do a thing; the cost is that a deploy stays blocked while the NAS holding the replica is unreachable.
• A brief mismatch between the restored copy and the live copy can come from copying a running database's WAL file, which the runbook already notes elsewhere; the step therefore says to re-run the comparison rather than fail immediately.
• The developer left the scheduler table row for rotation_check (around line 600) unlabelled, keeping scope to the prose the finding named. This document's header states it describes the planned application; flag it if the table row is still misleading.

Evidence (orchestrator-run gate at 614c4febfa)

• gofmt -l . — empty; CGO_ENABLED=0 go build ./... — ok; working tree clean
• Developer-reported: go vet ./... pass, go test -race -count=1 ./... all packages ok, and cd web && npm run typecheck && npm test && npm run build clean with 45 files / 764 tests

## Task Maestro task `008c0a8a-2242-429b-a1fd-69346bc1d414` — "Release corrections round 4", from the independent release review of `release/2026-10-07c` (PR #123, APPROVED at `fd225b9`). Documentation only: the diff is exactly `deploy/RUNBOOK.md` and `docs/ARCHITECTURE.md`. ## Findings fixed - **R01 (MEDIUM), `deploy/RUNBOOK.md` "Routine deploy and rollback":** the deploy steps did not require checking a current, restorable Litestream point before the restart applied migrations 0013 and 0014. Because an older SHA refuses the upgraded schema, the only rollback is a restore to a point before the restart — so an operator could restart production on an unverified backup. The Deploy sequence now has a discrete step 2, before `sudo systemctl restart mealquest`, that checks Litestream is replicating, restores the replica to a scratch file beside the live database (the documented restore command writing elsewhere, so the live files are untouched), compares integrity and counts with the existing Count check script, records the pre-deploy restore point as a UTC timestamp next to the previous SHA, and says plainly "If any check fails, do not restart". The Rollback section now uses that recorded `-timestamp` instead of describing a separate exercise. No script was added and `deploy/compose.yaml` is unchanged. - **R02 (LOW), `docs/ARCHITECTURE.md`:** the prose said the `rotation_check` schedule calls `MarkTried`, but this release has no production caller for `MarkTried` — the scheduler is Feature 20, so nothing opens a Rotation check automatically. The prose now says the schedule "is to call" `MarkTried` and states that this release provides only the `MarkTried` method and the rotation review read/decide APIs. ## Notes for review - The developer deliberately gave only one remedy when the backup check fails (restore replication and repeat the step) rather than also offering a manual-backup path, per the house rule of one way to do a thing; the cost is that a deploy stays blocked while the NAS holding the replica is unreachable. - A brief mismatch between the restored copy and the live copy can come from copying a running database's WAL file, which the runbook already notes elsewhere; the step therefore says to re-run the comparison rather than fail immediately. - The developer left the scheduler table row for `rotation_check` (around line 600) unlabelled, keeping scope to the prose the finding named. This document's header states it describes the planned application; flag it if the table row is still misleading. ## Evidence (orchestrator-run gate at 614c4febfa601bd049b6b81aef94fc0efbed9c4c) - `gofmt -l .` — empty; `CGO_ENABLED=0 go build ./...` — ok; working tree clean - Developer-reported: `go vet ./...` pass, `go test -race -count=1 ./...` all packages ok, and `cd web && npm run typecheck && npm test && npm run build` clean with 45 files / 764 tests

review_agent requested changes 2026-10-07 17:22:33 -04:00

Dismissed

review_agent left a comment

Member

Copy Link

Copy Source

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

review_agent requested changes 2026-10-07 17:40:32 -04:00

Dismissed

review_agent left a comment

Member

Copy Link

Copy Source

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

review_agent requested changes 2026-10-07 17:58:36 -04:00

Dismissed

review_agent left a comment

Member

Copy Link

Copy Source

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

review_agent requested changes 2026-10-07 18:17:07 -04:00

Dismissed

review_agent left a comment

Member

Copy Link

Copy Source

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

review_agent requested changes 2026-10-07 18:37:13 -04:00

Dismissed

review_agent left a comment

Member

Copy Link

Copy Source

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

review_agent requested changes 2026-10-07 19:05:04 -04:00

Dismissed

review_agent left a comment

Member

Copy Link

Copy Source

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

overseer changed target branch from release/2026-10-07c to dev 2026-10-07 19:29:55 -04:00

overseer added 29 commits 2026-10-07 19:29:55 -04:00

Merge pull request 'Promote dev to main: Features 01–05' (#47) from release/2026-10-04 into main ... 084350cb03

Reviewed-on: #47
Reviewed-by: The Reviewer <18+review_agent@code.cubecraftcreations.com>

Merge main into release/2026-10-05 before promotion review 37315c7821

Merge dev into release/2026-10-05: incorporate cancelled-backoff correction 86e7cc63c2

Merge pull request 'Promote dev to main: Features 08-09' (#63) from release/2026-10-05 into main ... 6e134968cc

Reviewed-on: #63
Reviewed-by: The Reviewer <18+review_agent@code.cubecraftcreations.com>

Merge main into release/2026-10-05b before promotion review a2f3143ac2

Merge pull request 'Promote dev to main: Feature 06 (PWA shell and offline)' (#71) from release/2026-10-05b into main ... 9cddbbb8a9

Reviewed-on: #71
Reviewed-by: The Reviewer <18+review_agent@code.cubecraftcreations.com>

Merge main into release/2026-10-06 before promotion review 2dbfc74f3e

Merge pull request 'Promote dev to main: Feature 07 (deployment, identity and backups)' (#76) from release/2026-10-06 into main ... d4be6212bc

Reviewed-on: #76
Reviewed-by: The Reviewer <18+review_agent@code.cubecraftcreations.com>

Merge main into release/2026-10-06b before promotion review a6a32b8426

Merge pull request 'Promote dev to main: Feature 10 (recipe grounding and photos)' (#79) from release/2026-10-06b into main ... 768bb48b21

Reviewed-on: #79
Reviewed-by: The Reviewer <18+review_agent@code.cubecraftcreations.com>

Merge main into release/2026-10-06c before promotion review cc6c1f9cf9

Merge dev into release/2026-10-06c: incorporate the deflake correction 2fe439cdcd

Merge pull request 'Promote dev to main: Feature 11 (settings home and Admin AI assistant)' (#87) from release/2026-10-06c into main ... b0a6d896ef

Reviewed-on: #87
Reviewed-by: The Reviewer <18+review_agent@code.cubecraftcreations.com>

Merge remote-tracking branch 'origin/main' into release/2026-10-07 c54af1656c

Merge remote-tracking branch 'origin/dev' into release/2026-10-07 7138721a28

Merge remote-tracking branch 'origin/dev' into release/2026-10-07 a27ef6a4c4

Merge remote-tracking branch 'origin/dev' into release/2026-10-07 372424e2f5

Merge pull request 'Promote dev to main: Feature 12 (regular dinners and onboarding)' (#98) from release/2026-10-07 into main ... 1bbacf2df5

Reviewed-on: #98
Reviewed-by: The Reviewer <18+review_agent@code.cubecraftcreations.com>

Merge remote-tracking branch 'origin/main' into release/2026-10-07b fa62813bb7

Merge pull request 'Promote dev to main: Feature 13 (Discover - Side Quest Suggestions)' (#115) from release/2026-10-07b into main ... 66fc292166

Reviewed-on: #115
Reviewed-by: The Reviewer <18+review_agent@code.cubecraftcreations.com>

Merge remote-tracking branch 'origin/main' into release/2026-10-07c fd225b9f90

Require a verified restore point before deploy; mark rotation schedule planned ... 614c4febfa

RUNBOOK: the deploy steps now verify, before the restart that applies
migrations, that Litestream replication is current and its replica
restores to the live data, and record the pre-deploy restore point; if
not, do not restart. The rollback section restores to that recorded point.

ARCHITECTURE: the rotation_check schedule that calls MarkTried is planned
(Feature 20); this release ships MarkTried and the rotation review read and
decide APIs only.

Task: 008c0a8a-2242-429b-a1fd-69346bc1d414

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Make the pre-deploy count check runnable and split rollback by migration ... a7f115f2f0

Write /tmp/mqcount.py from the Count check section with one heredoc
command, and give rollbacks that cross a migration an ordered
restore-then-start procedure separate from the plain restart.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Decide the rollback path from applied migration versions ... 459ee6ec28

The rollback check diffed all of migrations/, so a change to the sqlc
queries in migrations/queries/ sent the operator down the restore path and
discarded household changes made since the pre-deploy point. Compare the
versions in the live database's schema_migrations with the top-level *.sql
files at the previous SHA instead, the rule internal/store/migrate.go
enforces at start-up.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Make the rollback migration check fail fast and keep its status ... 08f52d3f1c

Run the check in a set -eu subshell so a failed cd, copy, git ls-tree
or comparison stops it before any verdict is printed, and remove the
scratch directory from an EXIT trap that preserves the check's status.
Spell out the reading rule: none, listed versions, or stop on error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Take consistent SQLite snapshots in the deploy and rollback checks ... 2b37d8ef26

The pre-deploy verification and the rollback migration check copied the
live mealquest.db and its WAL as separate files while the app and
Litestream ran, so a checkpoint between the copies could leave an older,
readable state: the rollback check could print "none" while the live
database held a migration the older SHA does not know.

Both now read the live database through /tmp/mqsnap.py, written once in
"Count check": SQLite's online backup API from a read-only connection,
then PRAGMA integrity_check, exiting nonzero unless it is ok. The
pre-deploy verification runs as one fail-fast subshell (set -eu, an EXIT
trap that cleans /tmp/mqcheck and the scratch restore while keeping the
status), diffs the replica and live count checks, and prints the restore
point only after every step has passed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Check each deploy verification command's own status and print the restore point last ... dc4b284bd8

- Capture the docker compose ps status before testing its output, and
  scan the Litestream log with grep's status read explicitly: 0 is a
  logged error, 1 is no match, anything else fails the verification.
- Run both clean-ups on exit whatever happens, keep any failure, and
  print the pre-deploy restore point only after every check and both
  clean-ups succeeded.
- State that the live snapshot expects the existing -wal and -shm, list
  their ownership after the snapshot, and say to stop and check the
  service state if a root-owned sidecar appears.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Report every rollback check failure and stop deploy chains on the first error ... cced89cc2a

The rollback check's trap now attempts its clean-up whatever happens,
keeps a failed clean-up as a nonzero status and prints "check FAILED" on
every nonzero exit. Both checks install their trap before the first
command that can fail. Deploy step 1, both rollback paths and the
script-writing block now chain with && so a failed checkout, fetch, build
or write stops the block with its status, instead of a later command
restarting the wrong release or masking the failure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Make every deploy, rollback and restore block fail fast with a final verdict ... d22167cfd3

Deploy step 3 printed `git rev-parse HEAD` as "the deployed SHA" even
after a failed restart; the Count check copy block could read a stale
/tmp/mqcheck copy and report integrity ok. Apply the established
pattern (subshell with set -eu, trap registered first, clean-up that
keeps a nonzero status, success line only after everything passed) to
Deploy step 3, the no-migration rollback, Restore the database steps
1-3 and 5, the Count check copy block and Restore photos (media). The
volume listings, service states, health codes and file counts that the
operator previously read by eye are now checked in the block.

Task: 008c0a8a-2242-429b-a1fd-69346bc1d414

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

review_agent requested changes 2026-10-07 19:34:33 -04:00

Dismissed

review_agent left a comment

Member

Copy Link

Copy Source

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

overseer added 1 commit 2026-10-07 19:48:17 -04:00

Verify restored media by content and report the timer's actual state ... a7b079c771

The media restore copied with rsync -a and checked file counts only, so a
local photo with the NAS size and mtime but other bytes could survive and
still print "media restored". Take a SHA-256 manifest of the NAS before the
copy, copy with --checksum, require every manifest entry to be present
locally with the same bytes, and require the NAS to still match the
manifest. On failure, the message now reads the timer's real state instead
of assuming it is stopped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

review_agent approved these changes 2026-10-07 19:54:02 -04:00

review_agent left a comment

Member

Copy Link

Copy Source

Independent Codex gpt-6-sol review: APPROVED for this exact commit.

Independent Codex gpt-6-sol review: APPROVED for this exact commit.

approver_agent merged commit 5ac5982c1f into dev 2026-10-07 20:02:54 -04:00

orchestrator-agent referenced this pull request 2026-10-08 03:23:01 -04:00

Promote dev to main: Feature 16 (Google Calendar Sync) + release corrections #131

Sign in to join this conversation.

Reviewers

No Reviewers

review_agent

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

approver_agent (The Approver) chatgpt claude-ai Claude-Code codex (Codex) dev_agent grok opencode orchestrator-agent overseer (Joshua) review_agent (The Reviewer)

No Assignees

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Maniacs_Mansion/MealQuest#124