review(M1): PASS — branch @d3fe9e2 cold-verified (unit 138, conc 20, lint, 0 dangling refs, gate-integrity, independent flock probe)
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:
@ -126,3 +126,71 @@ Net: the entire P1–P4 diff has been pre-audited and is clean against my break-
|
||||
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.
|
||||
|
||||
## M1: PASS @2026-06-10T04:38Z — implementation verified (branch restructure/concurrency @d3fe9e2)
|
||||
|
||||
Verdict formed from the plan (SSOT), the code/git, the STATUS claim's verify recipe, and my own
|
||||
COLD acceptance run — WITHOUT reading JOURNAL first (anti-anchoring honored; noting here that I had
|
||||
NOT consulted JOURNAL-conc at verdict time).
|
||||
|
||||
COLD ENVIRONMENT: fresh `git clone --branch restructure/concurrency` into /tmp/adv-m1 on cc-ci
|
||||
(NOT the Builder's tree); `git rev-parse HEAD == d3fe9e26bb0fbaedb37383539ba3973bc1c80aff` (matches
|
||||
claim), `git status` clean. Ran via the host `cc-ci-run` pyEnv (pytest 8.3.3 + playwright) and the
|
||||
pinned `.#lint` devshell.
|
||||
|
||||
ACCEPTANCE RESULTS (expected → observed):
|
||||
- `cc-ci-run -m pytest tests/unit -q` → 138 passed in 4.72s ✓ (claim: 138 passed)
|
||||
- `cc-ci-run -m pytest tests/concurrency -q` → 20 passed in 9.91s ✓ (claim: 20 passed)
|
||||
- `nix develop .#lint --command bash scripts/lint.sh` → `lint: PASS` ✓
|
||||
- `pytest tests/unit --collect-only` concurrency items → 0 ✓ (suite NOT in default gate)
|
||||
- dangling-ref grep (register_run_app, unregister_run_app, _run_owner_state, ACTIVE_RUN_DIR,
|
||||
CCCI_JANITOR_MAX_AGE, acquire_recipe_lock, RECIPE_LOCK_DIR, _stack_age_seconds) over
|
||||
*.py/*.nix/*.yml/*.sh → ZERO hits outside docs/ ✓
|
||||
|
||||
GATE-INTEGRITY (guardrails honored):
|
||||
- `RUN_APP_RE` regex unchanged (lifecycle.py:26, identical pattern); warm/canonical apps still
|
||||
never become probe candidates (test_11 asserts no lockfiles even created for warm names).
|
||||
- `services_converged()` / paused-is-settled / `backup_app()` waits: NOT in the code diff — all
|
||||
RUN_APP_RE/services_converged/paused diff hits are docs/concurrency.md prose (P5 rewrite).
|
||||
- `teardown_app` ordering untouched; only its trailing unregister call removed (registry gone).
|
||||
- Only `tests/<recipe>/` change is the mechanical `RECIPE_DIR=${ABRA_DIR:-$HOME/.abra}/...` line
|
||||
in ghost+discourse install_steps.sh — NO assertion/gate touched (diff-confirmed). Guardrail
|
||||
"never weaken recipe-test gates / touch tests/<recipe>/ content" honored.
|
||||
- P4: `concurrency.limit` block removed from .drone.yml; drone-runner.nix comment makes
|
||||
DRONE_RUNNER_CAPACITY the single knob.
|
||||
|
||||
ADVERSARIAL DIFF REVIEW (P1–P4 pre-audited in the two notes above; refuted: green-on-red exit-code
|
||||
masking [empirically tested], unlink/reacquire inode race [fstat==stat identity recheck],
|
||||
half-reaped coexistence [reap-under-probe-lock], signal-mid-teardown reentrancy [begin_teardown
|
||||
first line of both finally blocks], guard/ABRA_DIR/fetch ordering [no abra call pre-export]).
|
||||
|
||||
TEST-SUITE AUDIT vs the 19 plan cases: real kernel flocks, NEVER mocked (only teardown_app +
|
||||
abra-discovery stubbed, both disclosed). Coverage complete: cases 1–4 test_locks, 5–12
|
||||
test_janitor, 13–16 test_lifetime, 17–19 test_abra_dir, +test_18b (manual-pid isolation) = 20.
|
||||
Assertions are substantive, not tautological: exact funnel exit codes 142/143 (test_15/16),
|
||||
reap-vs-new-run timestamp ordering + fresh-inode `lock_state=="held"` (test_7), two-janitor
|
||||
arbitration via separate open()s (test_8 — valid: flock binds the open file description, so
|
||||
threads-with-distinct-fds model processes), long-held mtime-backdate flag-not-steal (test_10),
|
||||
PEP 446 fd non-inheritance with a surviving child (test_3), divergent per-run trees + canonical
|
||||
untouched (test_18).
|
||||
|
||||
INDEPENDENT PROBE (my own driver, NOT the Builder's helpers.py): drove the real
|
||||
`lifecycle.acquire_app_lock` from a standalone script with a sandbox CCCI_APP_LOCK_DIR on cc-ci →
|
||||
state `held` after acquire; a second acquirer BLOCKED while the first held (no ack2 after 1.5s);
|
||||
after `SIGKILL` of the holder the second acquired within 10s (kernel auto-release). Core invariant
|
||||
confirmed against the real code, not just the Builder's tests.
|
||||
|
||||
NON-BLOCKING NOTES (carry to M2 live-verify; none gate M1):
|
||||
- setsid-fork edge in the .drone.yml trap wrapper: if `setsid` ever forks (only when it's a pgrp
|
||||
leader — not the case for a backgrounded job in a non-job-control drone shell), $PID would be the
|
||||
intermediate and the harness could reparent (ppid==1) and self-abort. MUST be live-verified by
|
||||
the actual drone-cancel path at M2(a) — the plan already flags this ("verify drone exec runner
|
||||
signal delivery; the trap must fire on drone cancel"). Not unit-testable here.
|
||||
- End-of-janitor stale-lockfile tidy sweep (appless leftover lockfile unlink) is not directly
|
||||
covered by a named test (not one of the 19); low risk (tidiness only). Noted, not a defect.
|
||||
- test_14 (ppid race) depends on the helper reparenting to pid 1; under a subreaper it marks
|
||||
NEVER_REPARENTED and FAILS VISIBLY (never false-passes). Passed in this env.
|
||||
|
||||
CONCLUSION: M1 — implementation verified — PASS. M2 (merge to main + live verification a–d) is
|
||||
unblocked. Reminder for both loops: recipe-mirror PRs are !testme targets only — never merge/push
|
||||
them. (After this verdict I may consult JOURNAL-conc to contextualize, per §6.1.)
|
||||
|
||||
Reference in New Issue
Block a user