docs: architecture review — deepening ADRs 0011–0015

[ryanntannn]

Architecture review — deepening opportunities

Records five architectural deepening opportunities as proposed ADRs
(0011–0015) plus a consolidating review doc (docs/architecture-review-2026-06.md).
Docs only — no code changes.

The recurring shape

SynergyPlus has an excellent domain glossary, but several concepts it names
(Run Queue, Claim, Content Hash, per-User cap) own no module.
Their definitions are copied as raw SQL strings and hash recipes across three
runtimes (Go, Python, KEDA YAML), bound together only by CONTRACT.md prose. Each
ADR proposes the same move: give the concept one home, consumed through a narrow seam.

Candidates

ADR	Candidate	Strength
0011	Claim eligibility predicate triplicated (db.py:43-45, runnerpool_controller.go:167, scaledobject.yaml:31) — violates ADR-0005's own stated consequence	Strong
0012	Content Hash derived twice (queue.go:27 Go / loop.py:26 Python), no shared vector	Strong
0013	Lease lifecycle transitions + runner_id fence split across Runner (Python) and Reaper (Go)	Worth exploring
0014	API-key hash duplicated TS↔Go untested; allow-list tripled in portal	Worth exploring
0015	Engine Version (~8 sites) & cap (~7) scattered; derive from RunnerPool CR	Speculative

Top recommendation: ADR-0011. Highest blast radius (a claim/scaler drift is a
production autoscaling bug), and it's the keystone — fixing it dissolves the cap
literal (0015) and anchors the lease transitions (0013).

Provenance

Findings came from a multi-agent validation pass and were then re-verified
first-hand — every file:line confirmed by direct reading. The review doc also
records, for honesty, that the automated QA loop self-contaminated (an agent with
write access materialized a proposed migration the loop then "verified"); that
artifact was removed and no conclusion here depends on it. The ADRs rest only
on duplication that exists in the committed source.

Status

All five ADRs are marked proposed — recommendations for the team to ratify or
reject, not yet-accepted decisions.

🤖 Generated with Claude Code

https://claude.ai/code/session_019p51SPdbrjmGXJVeZnwTrD

[ryanntannn]

Re-validated under worktree isolation ✅

Re-ran the five-candidate adversarial QA loop with each agent confined to its own git worktree off master, strictly read-only, with a mandatory git ls-files contamination guard (every "already exists in-tree" claim must be proven against committed source).

Isolation held cleanly — all five worktrees ended with zero writes, the real tree was never touched, and the contamination check passed for every candidate. The independent pass also corrected several first-pass claims (now folded into the ADRs):

• 0011 — the naive app.eligible_simulations(…) FOR UPDATE SKIP LOCKED wrap silently drops the row lock on Postgres 16 (verified via EXPLAIN + a two-session concurrency test). Corrected to a membership-gate design that keeps FOR UPDATE OF <base table> SKIP LOCKED on the caller. Cap-50 framing softened — it's largely single-sourced; the predicate is the real duplication.
• 0013 — the Reaper does not fence on runner_id (expiry-only); the fence is two Python sites, not three.
• 0014 — the two auth.ts sites share one isAllowedEmail helper; the duplication is the domain list (defined 3×), not the logic.
• 0015 — the "live KEDA query doesn't derive from the CR" premise was refuted — it does (controller.go:161,165-169). Residuals narrowed to the in-CR image tag + the Compose path.

The first pass had self-contaminated (an agent wrote a proposed migration the loop then "verified"); the worktree isolation in this pass is what prevented a recurrence. Method note is in docs/architecture-review-2026-06.md.