102 lines
7.0 KiB
Markdown
102 lines
7.0 KiB
Markdown
# REVIEW-rcust.md — Adversary ledger for the recipe-customization restructure phase
|
||
|
||
SSOT for this phase: `/srv/cc-ci/cc-ci-plan/recipe-custom-restructure-full-plan.md`.
|
||
Gates: **M1** (implementation verified — branch `restructure/recipe-custom`, unit+concurrency+lint
|
||
green on cold clone, resolved-customization diff clean for all 21 recipes, adversarial diff review)
|
||
and **M2** (merged + real-CI regression sweep matching baseline matrix). DONE requires fresh PASS
|
||
for both with no open VETO.
|
||
|
||
I own this file and the `## Adversary findings` section of BACKLOG-rcust.md only.
|
||
|
||
---
|
||
|
||
## Standing watch items (what I will hunt at M1/M2)
|
||
|
||
- **Coverage loss** (cardinal risk): for every migrated recipe, old loaders' effective customization
|
||
values must equal new `meta.load()` values. Throwaway diff script over all 21 recipe dirs; any
|
||
delta = finding.
|
||
- **Assertion weakening** in `tests/<recipe>/` diffs — migrations must be mechanical only (signatures,
|
||
fixture/key renames, underscore prefixes). Any changed assert/expected value = VETO.
|
||
- **Deleted-code fallout** — dangling refs to `_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`.
|
||
- **Validation gaps** — typo'd key / wrong type / callable-on-data-key must raise MetaError, not pass.
|
||
- **R2 fixed end-to-end** — orchestrator load path delivers SCREENSHOT to screenshot.py.
|
||
- **HC2 / F2-11 integrity** — repo-local default-deny, requires_deps skip-report, generic floor
|
||
semantics all unchanged.
|
||
|
||
---
|
||
|
||
## Verdicts
|
||
|
||
_(no GATE verdict yet — M1 is not claimed. M1 only claims after P1–P6 are all on the branch;
|
||
Builder has landed P1 (472a68b) + P2 (8cd72fd) and is mid-P3. The interim pre-review below is
|
||
front-loaded break-it work on the FROZEN P1/P2 commits — NOT an M1 PASS.)_
|
||
|
||
### Interim pre-review of frozen P1+P2 (branch @ 8cd72fd) — @2026-06-10, cold from upstream clone
|
||
|
||
Done as idle-time break-it work while no gate is pending. P1/P2 phase commits won't be rewritten
|
||
(Builder adds P3+ on top), so reviewing them now is non-wasted and front-loads M1. Cold clone of
|
||
`origin/restructure/recipe-custom` into `/tmp/rcust-verify` from the true upstream remote.
|
||
|
||
**No defects found so far.** Results:
|
||
|
||
1. **Deleted-code fallout — CLEAN.** Grepped `runner/ tests/ scripts/` for live refs to every deleted
|
||
symbol (`_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`). All hits are comments/docstrings
|
||
explaining the deletion, test names, or the intentionally-RETAINED `CCCI_SKIP_GENERIC*` env form
|
||
(kept per P2c). Zero live call-sites. `setup_custom_tests.sh` files gone.
|
||
2. **All-recipes-load-clean (typo gate) — PASS, independently.** Ran `meta.load()` (pure stdlib) over
|
||
all 21 recipe dirs cold via plain python3 (did NOT trust the Builder's test_meta.py). All 21 load;
|
||
non-default key sets sane. Every ALL-CAPS key used in any recipe_meta.py is in the 14-key registry.
|
||
3. **Coverage-loss diff (CARDINAL check) — ZERO deltas on data keys + hook presence.** Throwaway
|
||
harness (`/tmp/diff_meta.py`) reproduces main's six-loader effective resolution (`_load_meta`,
|
||
`declared_deps`, `is_enrolled`, `_recipe_extra_env`) from MAIN's recipe_meta files and diffs vs the
|
||
BRANCH's `meta.load()` for all 21 recipes. After correcting one harness artifact (EXTRA_ENV default
|
||
is `{}` not None), **0/21 recipes show any delta** for HEALTH_PATH/HEALTH_OK/DEPLOY_TIMEOUT/
|
||
HTTP_TIMEOUT/BACKUP_CAPABLE/EXPECTED_NA/UPGRADE_BASE_VERSION/DEPS/WARM_CANONICAL + presence of
|
||
READY_PROBE/BACKUP_VERIFY/UPGRADE_EXTRA_ENV/EXTRA_ENV/SCREENSHOT.
|
||
4. **Validation gaps — CLOSED.** Crafted tmp recipe_metas: typo'd key → MetaError (with "did you mean
|
||
DEPLOY_TIMEOUT?"); wrong type (`DEPLOY_TIMEOUT="str"`) → MetaError; callable on data key
|
||
(`DEPLOY_TIMEOUT=lambda ctx:...`) → MetaError; `_PRIVATE`/lowercase-helper → loads clean (exemption
|
||
works). All four behave per the locked decision.
|
||
5. **meta.py read** — single `exec()`, frozen `RecipeMeta` generated from `KEYS`, `_coerce` rejects
|
||
bool-as-int and callable-on-data-key; `non_default` compares vs registry default. No issues.
|
||
|
||
**Still UNVERIFIED for M1 (do NOT treat above as M1 PASS):** full `pytest tests/unit -q` +
|
||
`pytest tests/concurrency -q` + `scripts/lint.sh` cold on the cc-ci host; R2 end-to-end through the
|
||
real orchestrator screenshot path; P3 ctx-hook signature migration (assert byte-identical, legacy
|
||
`lambda domain:` raises clear MetaError); P4/P5/P6; re-run the coverage diff on the FINAL branch
|
||
(P3 changes hook signatures); recipe-test diffs are mechanical-only (no assertion weakening);
|
||
HC2/F2-11/generic-floor integrity. These wait for the `claim(rcust): M1`.
|
||
|
||
### Interim pre-review of frozen P3 (branch @ fd02d9f) — @2026-06-10, cold from upstream clone
|
||
|
||
Builder landed P3 (uniform ctx hook convention) and moved to P4, so P3 is frozen. Pre-reviewed it.
|
||
**No defects found.**
|
||
|
||
1. **Mechanical-migration discipline — HELD (no VETO trigger).** `git diff 8cd72fd..fd02d9f` over
|
||
`tests/*/` shows ZERO changed assert/expected literals. Every hook change is purely
|
||
`def HOOK(domain[, meta])` → `def HOOK(ctx)` + `domain` → `ctx.domain` in the body. Spot-checked
|
||
cryptpad/mumble/ghost/lasuite-drive recipe_meta.py + lasuite-drive ops.py: seeded values, return
|
||
dicts, paths, status codes, and the `pre_restore` `assert _psql(...) in (...)` are byte-identical
|
||
apart from the `ctx.` deref.
|
||
2. **HookCtx — present + complete.** `meta.HookCtx` frozen dataclass has all 5 documented fields
|
||
(`.domain`, `.base_url`, `.meta`, `.deps`, `.op`); `meta.hook_ctx(domain, meta, op=…)` factory
|
||
builds it and pulls `deps` from `$CCCI_DEPS_FILE`. All call sites migrated: run_recipe_ci
|
||
`pre_<op>`, BACKUP_VERIFY; lifecycle `extra_env` + READY_PROBE; screenshot `SCREENSHOT(page, ctx)`.
|
||
(NB my first pass falsely flagged "no HookCtx" — that was a STALE WORKTREE at P2; corrected by
|
||
checking out fd02d9f. Logged here for honesty.)
|
||
3. **Legacy-signature guard (P3.4) — PRESENT + works, live-probed.** `meta.check_hook_signature`
|
||
exact-matches positional params and raises a CLEAR MetaError naming the P3 migration + HookCtx
|
||
fields. Wired into both `load()` (recipe_meta hooks; SCREENSHOT expects `(page, ctx)`, rest
|
||
`(ctx)`) and the orchestrator (ops.py `pre_<op>`). Crafted tmp metas: legacy `READY_PROBE(domain)`,
|
||
`SCREENSHOT(page, domain, meta)`, `EXTRA_ENV(domain)` all → MetaError at load; `READY_PROBE(ctx)`
|
||
loads clean. No silent mid-run TypeError path.
|
||
4. **Coverage diff re-run at P3 head — still 0/21 deltas** (hook presence + all data keys unchanged).
|
||
|
||
Net: P1+P2+P3 all clean under cold adversarial probing. M1 still gated on full unit+concurrency+lint
|
||
on the cc-ci host, P4–P6, R2 end-to-end via the real screenshot orchestrator path, and a final
|
||
coverage re-diff. No findings filed; no VETO.
|