review: M3 PASS (live: !testme 12s trigger, re-run, !testmexyz no-trigger, org-auth); close A4 (cap=1 mitigates)
This commit is contained in:
12
BACKLOG.md
12
BACKLOG.md
@ -130,7 +130,17 @@ Two single-writer sections (§6.1): Builder edits only `## Build backlog`; Adver
|
||||
doesn't need the `.env`. *Re-test:* run install, kill the process mid-deploy, verify the next
|
||||
run (or janitor) leaves zero residual service/volume/secret. Adversary closes after re-test.
|
||||
|
||||
- [ ] **[adversary] A4 — Concurrent same-recipe runs collide on the shared recipe checkout.**
|
||||
- [x] **[adversary] A4 — Concurrent same-recipe runs collide on the shared recipe checkout.**
|
||||
**CLOSED @2026-05-27T03:13Z — mitigated by the runtime concurrency cap.** The Builder's
|
||||
resource-safety change sets `DRONE_RUNNER_CAPACITY=1` (verified live: runner logs `capacity=1`)
|
||||
+ the recipe-CI pipeline has `concurrency:limit:1`, so recipe-CI builds **serialize** — two
|
||||
runs never overlap, hence the shared `~/.abra/recipes/<recipe>` checkout collision cannot
|
||||
occur via the production trigger path. The §6 "two concurrent runs don't collide" guarantee
|
||||
holds by serialization (an explicitly endorsed design per plan §4.2). **Latent caveat:** the
|
||||
checkout is still *not* per-run isolated, so raising `DRONE_RUNNER_CAPACITY`>1 (the module
|
||||
comments allow it) would reintroduce the collision — fix the per-run abra home/checkout before
|
||||
ever doing so. (A positive "two triggers serialize & both complete" check folds into the M10
|
||||
concurrency verification.)
|
||||
Found by review (M6 verify); to confirm empirically. Per-run isolation is correct for the app
|
||||
**domain/volume/secret** (hashed `<recipe[:4]>-<6hex(recipe|pr|ref)>`), but the recipe *source
|
||||
checkout* is a single shared path `~/.abra/recipes/<recipe>`: `run_recipe_ci.fetch_recipe`
|
||||
|
||||
24
REVIEW.md
24
REVIEW.md
@ -202,3 +202,27 @@ Caveat: current Drone logs are hello-world + self-test; the full M7/D6 test must
|
||||
app-generated secrets (e.g. keycloak DB passwords) in recipe-run logs AND the dashboard (M8). This
|
||||
is a clean baseline, not the final D6 verdict. (DB copy was scanned off-box and deleted; no secret
|
||||
value printed or committed.)
|
||||
|
||||
## M3 — Comment bridge: PASS @2026-05-27T03:13Z
|
||||
|
||||
Verified cold against the NEW design (orchestrator change: polling-PRIMARY + org-membership auth;
|
||||
webhook now optional). Re-reviewed `bridge/bridge.py` (256 lines) — sound — then live-probed the
|
||||
running bridge + Drone:
|
||||
- **`!testme` triggers a run ≤60s:** I posted `!testme` (comment 13708) on PR #1 at epoch
|
||||
1779847690 → bridge `[poll] triggered build 35` → Drone build 35 created at 1779847702 =
|
||||
**12s** latency. (Build is `failure` only because `RECIPE=cc-ci` has no `tests/cc-ci/`; the
|
||||
trigger + event=custom recipe-CI pipeline fired correctly — integration is live.)
|
||||
- **Re-commenting re-runs:** my new comment 13708 → build 35, distinct from the earlier
|
||||
comment 13705 → build 26. Distinct comment ids each fire once (dedup via `_claim`).
|
||||
- **Other comments do NOT trigger:** I posted `!testmexyz` → **no** build created, no bridge
|
||||
trigger log. Exact trimmed match enforced.
|
||||
- **Auth enforced (org-membership, fail-closed):** `GET /orgs/recipe-maintainers/members/<u>` —
|
||||
autonomic-bot & notplants → 204 (allowed), `definitely-not-a-member-zzz9` → 404 (rejected).
|
||||
`is_authorized` returns True only on 204/allowlist; anything else (incl. errors) → False.
|
||||
- **Link back:** bridge posted run-link comment 13706 ("cc-ci: started CI run … → drone…/recip…").
|
||||
- **Concurrency cap live:** runner `capacity=1` (`DRONE_RUNNER_CAPACITY=1`) + pipeline
|
||||
`concurrency:limit:1` — recipe-CI builds serialize.
|
||||
|
||||
Verdict: **M3 PASS.** (Polling is outbound read+comment only — no repo-admin; webhook optional.)
|
||||
Note: full bridge→3-stage-recipe-CI E2E on a *real recipe* PR is the Builder's in-flight
|
||||
integration item / D10 — build 35 shows the pipeline wiring works; green-on-a-real-recipe is M10.
|
||||
|
||||
Reference in New Issue
Block a user