diff --git a/REVIEW-conc.md b/REVIEW-conc.md index 769ba93..64e652c 100644 --- a/REVIEW-conc.md +++ b/REVIEW-conc.md @@ -90,3 +90,39 @@ Open items to confirm AT M1 (cold, full suite) — NOT defects, just unverified- harness would reparent to ppid==1 and self-abort. Live-verify the trap+cancel path at M2(a). - begin_teardown is process-global module state (lifetime._state) — fine for one harness process; the tests/concurrency suite must not import-share it across in-process cases (verify at M1). + +## 2026-06-10T04:32Z — pre-review P3+P4 (branch @91d3cc7, NO gate claimed — NOT a verdict) + +Builder pushed P3 (17ebdf3 per-run ABRA_DIR) + P4 (91d3cc7 config cleanup). tests/concurrency + +P5 docs still pending, so M1 still not claimable. Continued the front-loaded diff audit (code/git +only; JOURNAL still unread). Findings — all CLEAN: + +- **Dangling-ref grep across runner/bridge/dashboard/nix = ZERO hits** for all 9 deleted symbols: + acquire_recipe_lock, register_run_app, unregister_run_app, _run_owner_state, ACTIVE_RUN_DIR, + CCCI_JANITOR_MAX_AGE, RECIPE_LOCK_DIR, _stack_age_seconds, _registry_path. The orphaned + `datetime` import is also gone from lifecycle.py. Clean deletion. +- **Path centralization**: all `~/.abra/recipes/` literals replaced by `abra.recipe_dir()` + (resolves `$ABRA_DIR else ~/.abra`) across abra.py (recipe_checkout, has_lightweight_version_tags, + recipe_head_commit, recipe_versions), generic._recipe_dir, lifecycle.prepull_images, + snapshot_recipe_tests, fetch_recipe. prepull's env_path stays canonical `~/.abra/servers/...` + which is correct (servers/ is the shared symlink target). +- **Ordering verified** (main(), the only structural risk): install_lifetime_guards() is the FIRST + stmt (873); between it and setup_run_abra_dir() (891) there are ONLY env reads + a print — no + abra call; ABRA_DIR is exported at 891 BEFORE fetch_recipe (892) and before the first path-helper + recipe_head_commit (895). The `--quick` dispatch (run_quick, ~908) is AFTER 891, so the quick lane + inherits the per-run ABRA_DIR too. No tree is touched before ABRA_DIR is set. +- **Manual-run isolation**: rid=="manual" → "manual-" so two hand-runs don't share a tree. + +Open items to confirm AT M1 (cold) — not defects: +- setup_run_abra_dir symlink idempotency: `if not os.path.islink(link): os.symlink(...)` — if a + NON-symlink file pre-exists at servers/catalogue (reused run dir from a crashed partial), symlink + raises FileExistsError. Low risk (fresh run-id per Drone build) but worth a glance. +- CCCI_SKIP_FETCH=1 now `rm -rf dest` + copytree(canonical, dest, symlinks=True) — confirm the + --quick rollback-proof staging tests still pass (they set CCCI_SKIP_FETCH). +- tests/{ghost,discourse}/install_steps.sh RECIPE_DIR=${ABRA_DIR:-$HOME/.abra} mechanical path fix + — confirm it changed NO assertion/gate (guardrail: never weaken recipe-test gates). Diff-check. + +Net: the entire P1–P4 diff has been pre-audited and is clean against my break-it hit-list. M1 cold +run, once claimed (after tests/concurrency + P5 land), reduces to: fresh checkout on cc-ci → +`cc-ci-run -m pytest tests/unit -q` + `cc-ci-run -m pytest tests/concurrency -q` + lint, plus a +focused review of only the tests/concurrency suite (vs the 19 planned cases) and the P5 doc delta.