diff --git a/machine-docs/REVIEW-2.md b/machine-docs/REVIEW-2.md index 5fe17f3..1a4430c 100644 --- a/machine-docs/REVIEW-2.md +++ b/machine-docs/REVIEW-2.md @@ -1587,3 +1587,66 @@ the fix is honest, fail-loud, and does not mask a persistent failure), clean tea **Isolation note:** verdict from the plan + code (`_admin_register` retry logic + the full §4.3 flow + ops/test_backup/test_restore) + STATUS claim verification info + my own cold full-lifecycle re-run (which reproduced the transient first-hand). JOURNAL-2 not consulted before this verdict. + +## Q4.5 mattermost-lts — PASS @2026-05-30T01:35Z (COLD, first-hand, my clone /root/adv-verify @origin/main 1ca7b23) + +Cold full-lifecycle re-run on cc-ci from my OWN clone — the exact claimed command — PLUS a negative +control against the published recipe. Both runs first-hand; logs `/root/adv-mattermost-pr1.log` +(PR=1, the fix) and `/root/adv-mattermost-pr0-neg.log` (PR=0, published). + +**Primary — PR=1 (recipe-PR `recipe-maintainers/mattermost-lts#1`, REF=4ca7f418):** all tiers GREEN. +- RUN SUMMARY: `deploy-count = 1 (expect 1)`; `install/upgrade/backup/restore/custom` **all pass**. +- Upgrade: `head_ref=4ca7f418 chaos-version=4ca7f418 version=2.1.9+10.11.15→2.1.10+10.11.18` (HC1, + head_ref==chaos-version, real prev→PR-head crossover). +- Custom — **4 PASS**: `test_create_message_roundtrip`, `test_second_user_reads_first_users_message` + (29s), `test_root_serves`, `test_system_ping_ok`. +- Clean teardown: post-run no `matt-*` stack; 0 matt secrets / 0 volumes / 0 networks. + +**P4 — the headline crux — restore PROVEN non-vacuous via a NEGATIVE CONTROL (decisive).** I re-ran +the SAME overlay against the **published** recipe (`PR=0`, no fix), STAGES=install,backup,restore: +- `tests/_generic/test_restore.py::test_restore_healthy PASSED` (app healthy after restore) **but** + `tests/mattermost-lts/test_restore.py::test_restore_returns_state FAILED` — + `RuntimeError: docker exec … failed (rc=1) … ERROR: relation "ci_marker" does not exist`. RUN + SUMMARY: `restore : fail`. +- This independently confirms (a) the published recipe's restore is a **silent no-op** (looks healthy, + data lost — exactly the bug class cc-ci exists to catch); (b) the P4 overlay is **non-vacuous** — a + health-only test passes here, the data-integrity assertion catches it; (c) it fails **LOUD** — + `exec_in_app` RAISES on a failed exec, never a silent `''` false-pass; (d) `ops.pre_restore` DROPs + `ci_marker` + asserts the drop took, so a no-op restore is observable. With PR #1's coop-cloud + `/pg_backup.sh` restore (terminate/FORCE-drop/recreate/reimport), `test_restore_returns_state` + PASSES (PR=1 run) and `ci_marker` also survives the upgrade. The recipe-PR is a **genuine fix**, + not a test weakening — verified end-to-end by running both halves myself (stronger than static). + +**P3 — ≥2 SEPARATE non-vacuous functional tests (both PASSED), read the bodies, genuinely distinct:** +- `test_create_message_roundtrip` — single-user self round-trip: admin → team → channel → POST a + unique-per-run marker → GET back by post id → assert text round-trips. +- `test_second_user_reads_first_users_message` — cross-user delivery: user_a posts a marker; a SECOND + user (user_b) is created via the admin API, added to team+channel, **logs in with its OWN session + token**, GETs the channel posts, and asserts it sees user_a's marker. Membership + ACL + multi- + session fetch — NOT a self read-back. Unique marker per run ⇒ no stale/echo false-pass. +- `_mm.bootstrap_admin` correctly handles mattermost's single-unauthenticated-first-user constraint + (create-or-login the deterministic shared admin; RAISES on a broken auth path). It does NOT make the + multi-user test vacuous — user_b is a genuinely separate principal with its own token. +- (`test_system_ping_ok` JSON `{"status":"OK"}` + `test_root_serves` are supporting liveness, not + counted to the P3 floor.) + +**P2 vacuous** (no `recipe-info/mattermost-lts/tests/` corpus; documented in PARITY.md) — acceptable. +**P5/P6 N/A** — postgres is in-recipe (no external dep); the defining team-chat behaviour is exercised +fully via the REST API (message create/read-back + cross-user delivery), no browser-only UX owed. +**P7** — no weakened/skipped/xfail/mocked tests; the restore gap was fixed at the SOURCE (recipe-PR), +not papered over; the overlay is fail-loud. + +**Break-it checks:** (1) the negative control above (published recipe → restore RED, proving teeth); +(2) clean teardown after a **FAILED** run — post-PR=0 node fully clean (no matt stack/secrets/vols/ +nets); (3) per-run unique markers defeat stale-response false-pass; (4) deploy-count=1 (no hidden +redeploy). + +**Verdict: Q4.5 mattermost-lts PASS.** Full lifecycle GREEN cold, deploy-count=1, real upgrade +crossover 10.11.15→10.11.18, P4 restore non-vacuous (negative control RED on published recipe), 2 +distinct P3 functional tests, clean teardown (incl. after failure). No `## VETO`. Advances P1 +coverage (mattermost-lts enrolled). The recipe-PR `recipe-maintainers/mattermost-lts#1` is a real +restore fix — same data-loss class cc-ci already caught in immich + matrix-synapse. + +**Isolation note:** verdict from the plan (P1–P8) + the test code (ops.py/test_restore.py/test_backup.py/ +functional/{_mm,test_create_message,test_multiuser_message}.py) + the STATUS Gate-Q4.5 verification +info + my own cold PR=1 full run AND PR=0 negative control. JOURNAL-2 not consulted before this verdict.