From 4334e19a7bb65829dc18049370fa18f1fe89a932 Mon Sep 17 00:00:00 2001 From: autonomic-bot Date: Thu, 28 May 2026 03:30:24 +0100 Subject: [PATCH] =?UTF-8?q?review(1e):=20E1/HC3=20FAIL=20=E2=80=94=20opt-o?= =?UTF-8?q?ut=20surfaces=20backup/restore=20race=20(F1e-1);=20additive+cou?= =?UTF-8?q?nt=3D1=20confirmed,=20PASS=20withheld?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- machine-docs/BACKLOG-1e.md | 33 ++++++++++++++++++++++++++++++++- machine-docs/REVIEW-1e.md | 28 ++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/machine-docs/BACKLOG-1e.md b/machine-docs/BACKLOG-1e.md index 3c7ea9b..aa6e28b 100644 --- a/machine-docs/BACKLOG-1e.md +++ b/machine-docs/BACKLOG-1e.md @@ -16,4 +16,35 @@ Phase-namespaced backlog. Builder edits `## Build backlog`; Adversary edits `## cold-verify of HC1–HC4; flip STATUS-1e → ## DONE on full PASS. ## Adversary findings -(none yet) + +- [ ] **F1e-1 [adversary]** — *opt-out is NOT behavior-neutral: the backup/restore data-continuity + overlays are racy and `exec_in_app` silently swallows a failed exec → a healthy recipe goes RED.* + Found cold-verifying E1/HC3 (commit b7e6cbd). My cold e2e of custom-html + `STAGES=install,upgrade,backup,restore,custom`: + - **default** (generic additive): all tiers PASS, deploy-count=1. ✓ + - **`CCCI_SKIP_GENERIC=1`** (opt-out): generic skipped on every tier (0 `_generic/` files ran), + deploy-count=1 ✓ — BUT **backup=FAIL**: `tests/custom-html/test_backup.py::test_backup_captures_state` + → `AssertionError: '' == 'original'` (the `exec_in_app(... cat ci-marker.txt)` returned **empty**). + **Root cause (static):** `lifecycle.exec_in_app` runs `docker exec …` and returns + `proc.stdout` **without checking `returncode`**. When backup-bot cycles the app container during + the backup op, `_app_container` resolves a container that is mid-transition, `docker exec` fails, + stdout is empty, and the failure is silently returned as `''`. The backup/restore overlays read + the marker via `exec_in_app` **immediately after** the container-cycling op with **no readiness + wait/retry**, despite their docstrings claiming immunity ("immune/robust to the post-backup/restore + serving race"). In the **default** path the generic `assert_backup_artifact` pytest runs first + (~1s spawn), an accidental timing buffer that lets the container settle; **opt-out removes that + buffer and the race surfaces.** So `CCCI_SKIP_GENERIC` changes observable behavior and can flip a + GREEN recipe to RED — contradicting "additive/opt-out is safe" and the Builder's E1 claim that the + opt-out run was "clean." + **Why it matters:** (1) a flaky false-RED blocks legitimate PRs and erodes trust; (2) `exec_in_app` + swallowing a failed exec is itself unsafe (an exec error masquerades as empty data — could also + make a real failure *pass* in a different assertion). Per plan guardrails: add real readiness/retry + robustness to the harness (and check the exec returncode / raise on failure), do **not** weaken or + delete the assertion. + **Repro:** `cd && CCCI_SKIP_GENERIC=1 RECIPE=custom-html STAGES=install,backup,restore + cc-ci-run runner/run_recipe_ci.py` → backup tier intermittently `'' == 'original'`. + **Status:** isolated (no-concurrency) reproduction in flight to rule out the confound that the + Builder was running parallel custom-html e2e at the same time (which would ALSO be a finding — + concurrent runs must not collide on backup-bot, §6/D-gate). Closing this finding requires: exec + returncode checked + a bounded readiness/retry on the post-op volume read, re-verified cold under + opt-out (and concurrency). **E1/HC3 PASS withheld pending fix.** diff --git a/machine-docs/REVIEW-1e.md b/machine-docs/REVIEW-1e.md index 3162c1b..59d51d6 100644 --- a/machine-docs/REVIEW-1e.md +++ b/machine-docs/REVIEW-1e.md @@ -44,3 +44,31 @@ Verdict: **PASS** — default-secure, centralized gate, flips only on explicit p hostile repo-local code provably not executed under the shipped default. No finding. **Note (not a defect):** orchestrator still uses single-file override `resolve_op` (1d semantics); the additive generic floor (HC3) is E1 in-flight — will re-check the gate survives the HC3 refactor. + +### E1 / HC3 — additive generic + op/assertion split — FAIL (PASS WITHHELD) @2026-05-28 +Builder claim (STATUS-1e gate, commit b7e6cbd): generic runs additively alongside overlays; +orchestrator owns each op (once); opt-out via `CCCI_SKIP_GENERIC[_]`/`recipe_meta.SKIP_GENERIC`; +deploy-count stays 1; two e2e (default + opt-out) "clean." + +**Cold verification (own clone HEAD=b7e6cbd shipped to cc-ci `/tmp/adv-1e`, run via `cc-ci-run`):** +- **Structure (PASS):** read the refactor — `run_lifecycle_tier` performs the op ONCE + (`_perform_op`→`generic.perform_{upgrade,backup,restore}`, none call `deploy_app`), then runs generic + (unless `_skip_generic`) + overlay as separate pytests vs the shared post-op state. Generic+overlay + test files are assertion-only; seeding moved to `ops.py pre_`. `assert_upgraded` keeps the + non-vacuous move check (F1d-2). `_record_deploy()` lives only in `deploy_app`. +- **Default e2e** (custom-html, all stages): EVERY tier ran BOTH `assert (generic)` AND + `assert (cc-ci)`; pre_upgrade/backup/restore seeds fired; **deploy-count=1**; install/upgrade/backup/ + restore all PASS; custom=skip; clean teardown (no leftover stack/volume). ✓ additive confirmed. +- **Opt-out e2e** (`CCCI_SKIP_GENERIC=1`): generic skipped on every tier (**0** `_generic/` files ran), + overlay-only, **deploy-count=1** ✓ — **but backup=FAIL**: `test_backup_captures_state` → + `AssertionError: '' == 'original'`. Same code/recipe; only diff is the opt-out flag. + +**Verdict: FAIL — opt-out is not behavior-neutral.** Opting out of the generic removes an accidental +~1s timing buffer (the generic pytest spawn) and surfaces a real race: the backup/restore overlays +read the marker via `exec_in_app` immediately after a container-cycling op with no readiness/retry, and +`exec_in_app` silently returns empty stdout on a failed `docker exec` (returncode ignored). A healthy +recipe can thus be reported RED under opt-out. Filed **F1e-1 [adversary]** (BACKLOG-1e) with root cause ++ repro + fix direction (check exec returncode + bounded readiness retry; do NOT weaken the assertion). +Isolated (no-concurrency) reproduction in flight to rule out the parallel-Builder-run confound — which +would itself be a concurrency-collision finding. **HC3 PASS withheld until F1e-1 is fixed + re-verified +cold under opt-out.**