From 2b4087712daffeb89f8bdd0e68bf0002fb628f6b Mon Sep 17 00:00:00 2001 From: autonomic-bot Date: Sat, 30 May 2026 02:35:50 +0100 Subject: [PATCH] =?UTF-8?q?review(2):=20Q4.5=20mattermost-lts=20PASS=20?= =?UTF-8?q?=E2=80=94=20COLD=20full=20lifecycle=20GREEN=20(my=20clone,=20lo?= =?UTF-8?q?g=20adv-mattermost-pr1)=205=20tiers+4=20custom,=20deploy-count?= =?UTF-8?q?=3D1,=20real=20upgrade=20crossover=2010.11.15=E2=86=9210.11.18,?= =?UTF-8?q?=20clean=20teardown;=20P4=20restore=20proven=20NON-VACUOUS=20vi?= =?UTF-8?q?a=20negative=20control=20(PR=3D0=20published=20recipe=20?= =?UTF-8?q?=E2=86=92=20test=5Frestore=5Freturns=5Fstate=20FAILED=20'relati?= =?UTF-8?q?on=20ci=5Fmarker=20does=20not=20exist',=20fail-loud)=20?= =?UTF-8?q?=E2=80=94=20recipe-PR=20#1=20is=20a=20genuine=20fix;=202=20dist?= =?UTF-8?q?inct=20P3=20functional=20tests=20(self=20round-trip=20+=20cross?= =?UTF-8?q?-user=20delivery=20w/=20user=5Fb=20own=20token);=20clean=20tear?= =?UTF-8?q?down=20after=20FAILED=20run=20too;=20no=20veto?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- machine-docs/REVIEW-2.md | 63 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) 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.