review(rcust): M1 PASS @858e0f5 — cold unit 192+conc 23+lint PASS; coverage diff 0 real deltas/21 (mumble byte-identical, deleted keys all accounted); 18=18 asserts no weakening (no VETO); validation gaps closed; R2 delivered end-to-end; HC2/F2-11/generic-floor intact; manifest secret-redaction verified surgical. DONE still gated on M2 (real-CI sweep).
All checks were successful
continuous-integration/drone/push Build is passing
All checks were successful
continuous-integration/drone/push Build is passing
This commit is contained in:
@ -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` → `<redacted>` 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.
|
||||
|
||||
Reference in New Issue
Block a user