From 01f9f709701787eb3bd0f56ed8762c2a941a0f3a Mon Sep 17 00:00:00 2001 From: autonomic-bot Date: Wed, 10 Jun 2026 19:27:49 +0000 Subject: [PATCH] =?UTF-8?q?review(rcust):=20M1=20PASS=20@858e0f5=20?= =?UTF-8?q?=E2=80=94=20cold=20unit=20192+conc=2023+lint=20PASS;=20coverage?= =?UTF-8?q?=20diff=200=20real=20deltas/21=20(mumble=20byte-identical,=20de?= =?UTF-8?q?leted=20keys=20all=20accounted);=2018=3D18=20asserts=20no=20wea?= =?UTF-8?q?kening=20(no=20VETO);=20validation=20gaps=20closed;=20R2=20deli?= =?UTF-8?q?vered=20end-to-end;=20HC2/F2-11/generic-floor=20intact;=20manif?= =?UTF-8?q?est=20secret-redaction=20verified=20surgical.=20DONE=20still=20?= =?UTF-8?q?gated=20on=20M2=20(real-CI=20sweep).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- REVIEW-rcust.md | 84 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/REVIEW-rcust.md b/REVIEW-rcust.md index a4c31bc..6844ef7 100644 --- a/REVIEW-rcust.md +++ b/REVIEW-rcust.md @@ -194,3 +194,87 @@ effective customization unchanged); recipe-test diffs mechanical-only across the HC2/F2-11/generic-floor integrity at final head; AND — at the M1 dashboard check — confirm the SECRET_KEY_BASE-named field on the real dashboard is the accepted public dummy (or redacted). These wait for `claim(rcust): M1`. + +## M1 — implementation verified: **PASS** @2026-06-10T19:27Z (branch `restructure/recipe-custom` @ 858e0f5) + +Cold-verified from TWO fresh clones on the cc-ci host (NEW=858e0f5, OLD=main pre-restructure; +merge-base 49fb818 confirmed → `main..858e0f5` is exactly P1–P6). Verdict formed from the phase plan +(SSOT), the code/git history, the STATUS verification facts, and my own cold re-runs — NOT from +JOURNAL rationale (isolation discipline; I did not need to consult JOURNAL). + +**All M1 Definition-of-Done items PASS:** + +1. **Cold test suites — match claim exactly.** Fresh clone @858e0f5: + `cc-ci-run -m pytest tests/unit -q` → **192 passed**; `tests/concurrency -q` → **23 passed** + (untouched by this plan, proven); `nix develop .#lint --command scripts/lint.sh` → **lint: PASS**. + +2. **Coverage diff (cardinal risk) — 0 REAL deltas / 21 recipes.** Wrote throwaway extractors that + resolve EVERY recipe's effective customization in BOTH worlds — OLD via the legacy loaders + (`_load_meta` + `lifecycle._recipe_extra_env` + `deps.declared_deps` + `_recipe_meta_flag`), + NEW via `meta.load()` + `meta.extra_env/upgrade_extra_env` — for the common keys (HEALTH_*, + timeouts, DEPS, EXTRA_ENV resolved at a fixed domain, UPGRADE_EXTRA_ENV, BACKUP_CAPABLE, + EXPECTED_NA, UPGRADE_BASE_VERSION, READY_PROBE/BACKUP_VERIFY presence). Diff = **0 behavioral + deltas**; the only raw diffs were 20× `UPGRADE_EXTRA_ENV: None→{}` (unset default representation, + behaviorally identical) and mumble (most-customized: callable EXTRA_ENV→dict, UPGRADE_EXTRA_ENV, + READY_PROBE) is **byte-identical** old↔new. + Deleted keys accounted for (no silent loss): `SKIP_GENERIC` (0 recipe users); `CHAOS_BASE_DEPLOY` + → overlay-presence (discourse+ghost, exactly the two shipping compose.ccci.yml — perfect 1:1, no + change either direction); `OIDC_AT_INSTALL` → install-time made universal (drive+meet were + already install-time). **lasuite-docs** declared DEPS but NOT OIDC_AT_INSTALL → OLD post-install, + NEW install-time: an INTENTIONAL P2b consolidation, not a drop — flagged below for M2 validation. + +3. **Assertion weakening (VETO-class) — NONE.** Full branch diff over all recipe test files + (excl. harness unit/concurrency/regression): 18 removed asserts, 18 added. After mechanical + normalization (`domain`→`ctx.domain`, `deps_creds`→`deps`, `MAX_USERS`→`_MAX_USERS`, whitespace) + the removed and added assert sets are **IDENTICAL** — zero unmatched in either direction. Every + change is a pure signature/fixture/constant rename; no expected value altered, no assert deleted. + Spot-confirmed discourse/ghost `_psql(domain,…ci_marker…) in (…)` → `ctx.domain` only (expected + tuple + SQL byte-identical). **No VETO.** + +4. **Deleted-code fallout — clean.** No dangling LIVE refs to any of the 13 deleted symbols + (`_recipe_meta`/`_load_meta`/`_recipe_extra_env`/`_recipe_meta_flag`/`declared_deps`/ + `is_canonical_enrolled`/`OIDC_AT_INSTALL`/`CHAOS_BASE_DEPLOY`/`SKIP_GENERIC`/`setup_custom_tests`/ + `deps_apps`/`deps_creds`/`deployed_app`). Only residue: stale DOC/comment mentions of + `OIDC_AT_INSTALL` + `setup_custom_tests.sh` in PARITY.md files (non-blocking P6 cosmetic nit). + +5. **Validation gaps — closed.** Cold-probed `meta.load()` with synthetic bad metas: typo'd key, + str-on-int, bool-as-int, callable-on-data-key, legacy hook sig `READY_PROBE(domain)`, and unknown + key ALL → `MetaError` (clear, names the offending file/key). Clean + underscore-private-helper + metas load fine (no false positives). No silent pass. + +6. **R2 fixed end-to-end.** Cold proof through the REAL load path: a recipe declaring + `def SCREENSHOT(page, ctx)` is surfaced by `meta.load()` and resolved callable by + `screenshot._load_screenshot_hook` (old L1 allowlist dropped it — now arrives); orchestrator wires + it `run_recipe_ci.py:1029 capture(…, recipe_meta=meta)` → `hook(page, hook_ctx(domain, meta))`. + Absent recipe → None (default landing-page path). Legacy `SCREENSHOT(page, domain, meta)` sig + rejected at load. + +7. **HC2 / F2-11 / generic-floor integrity — preserved.** Cold-probed `discovery.custom_tests` + + `install_steps`: UNAPPROVED repo-local → `[]` / `None` (default-deny holds); APPROVED → surfaced. + `sso_dep_unverified` (F2-11) logic UNCHANGED (only a comment edited) — a deps-not-ready run that + skips ≥1 `requires_deps` test still suppresses the green signal. Generic floor `_skip_generic` + default = run (additive); opt-out now env-only (same env vars as before; the 0-user meta key + removed) and surfaced LOUDLY in CI + flagged `!!` in the manifest — strictly stronger, never + silent. + +8. **(Bonus) P5 secret-surface heads-up RESOLVED + verified.** The Builder landed `858e0f5` + redacting secret-named meta values in the manifest (my P5 BUILDER-INBOX ask). Cold-verified: + `plausible.EXTRA_ENV.SECRET_KEY_BASE` → `` in BOTH the log block and results.json; + recursive into nested dict keys; word-segment `(^|_)KEY(_|$)` regex avoids over-match + (KEYCLOAK_* passes). All-21-recipe sweep: exactly 1 redaction, ZERO over-redaction, ZERO + under-redaction (no secret-shaped value remains). Regression test + `test_manifest_redacts_sensitive_named_values` present. + +**Verdict: M1 PASS.** No findings filed, no VETO. + +**This does NOT clear `## DONE`.** Per the phase DoD, DONE requires a fresh Adversary PASS for BOTH +M1 *and* M2. M2 (merged-main real-CI regression sweep vs the committed baseline matrix) is still +unverified. M2 watch-items I will specifically re-check from run logs: +- **lasuite-docs OIDC is now install-time** (post→install change above) — must pass a real run with + OIDC wired at install (skip-count 0 on its `requires_deps` tests). +- the customization spot-checks the plan §M2.4 enumerates (mumble READY_PROBE tcp lines, cryptpad + SANDBOX_DOMAIN, ghost/discourse BACKUP_VERIFY + overlay copy + auto-chaos base deploy, lasuite-* + deps provisioning + OIDC tests ran, immich ops.py seeds, manifest block present in every log, + screenshot.png where capture succeeded). +- canary suite (RED canaries still caught at intended tier) + per-recipe level == baseline matrix. +- zero leaked apps after teardown.