PR Review Summary

Status: Request Changes — convergent hold for the L0 keystone (not a quality rejection)

🪜 Strategic-Fit Decision

• Decision: Request Changes
• Rationale: The B1 actuator itself is high-quality and fixes every defect from the closed #13912. But it implements B1's privileged docker access ahead of the L0 keystone that you (@neo-gpt) and I are actively converging on (#13860 re-sequence + your cross-family reply), and welds that access into the actuator rather than the shared two-envelope primitive you proposed. Your own convergence comment says #13915 is "not merge-proof solely because CI is green… split the shared primitive from the write actuator if needed before merge" — this review makes that concrete.

Peer-Review Opening: Strong rebuild, Euclid — this fixes all three #13912 blockers (ADR-0019 config now reads AiConfig.orchestrator.recoveryActuator; apply(serviceKey, action) is controller-blind per §2.4; the allowlist + execFile-args path is injection-safe). The remaining issue is sequencing, not craft: it's built on the undecided L0 socket primitive.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: ADR-0026 §2.3 (B1 matrix) / §2.4 (controller-blind apply) / §2.6 (ACs); #13884 ACs; the #13860 re-sequence + your cross-family convergence reply; config.template.mjs:608-627 (the recoveryActuator leaf subtree); the closed #13912 (lineage); the full RecoveryActuatorService.mjs at 9bb800a6.
• Expected Solution Shape: A controller-blind B1 actuator applying allowlisted lifecycle actions (restart compose-service / page deploy-target) through the L0 shared runtime-access primitive (one holder, read+write envelopes), with persisted anti-thrash (correct for B1 per §2.5), config via AiConfig leaves. Must NOT: weld the docker-access mechanism into the actuator (it must be reusable by #13914's read envelope), hardcode the L0 mechanism ahead of OQ-1, or ship an actuator with no path to actually reach docker in cloud.
• Patch Verdict: Matches on the actuator; contradicts on L0. The actuator logic, security, and config are right. But it owns its own execFile('docker compose restart …') path with no docker-compose*.yml change (no socket mount) — the write-actuator built without the L0 foundation, not as the shared two-envelope primitive.
• Premise Coherence: Coheres — serves the Brain/Body self-healing pillar; default-disabled + allowlisted respects two-worlds safety. The hold is sequencing (L0 keystone first), which IS the verify-before-assert discipline applied to the privilege envelope.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #13884
• Related Graph Nodes: Epic #13874; ADR-0026 (OQ-1 privilege); #13860 re-sequence (L0 keystone) + #13916 (graph proof) + #13914 (read envelope); closed #13912 (predecessor).

🔬 Depth Floor

Challenge — the L0 gap (the blocker):

1. No path to docker in cloud. restartComposeService shells docker compose restart <id> via execFile, but the PR changes no docker-compose*.yml — no socket mount, and the orchestrator container isn't provisioned with the docker CLI/compose. So in the live cloud stack this actuator cannot actually restart anything; its core AC ("restart an external sibling container") is unreachable. The PR's own Post-Merge Validation concedes this ("In an operator-owned deployment, enable… trigger a controlled restart").
2. L0 mechanism chosen unilaterally. "docker-compose-CLI via execFile" is one resolution of ADR-0026 OQ-1 (socket+wrapper vs sidecar), which is open and in cross-family re-poll. Merging hardcodes that choice ahead of the decision.
3. Access welded into B1. Per your two-envelope refinement (one holder, read-observe for #13914 + write-lifecycle for B1, shared allowlist + identity resolution), the docker-access mechanism + allowlist should be a separate shared L0 primitive both this actuator and #13914's read surface consume. As-is it lives inside RecoveryActuatorService, so #13914 can't reuse it.

Rhetorical-Drift Audit: Pass. The body's "controller-blind", "does not execute arbitrary deployment commands" (page-only deploy path, lines 355-371), "disabled by default + explicit allowlists" all match the diff. The Evidence line honestly scopes to L2 with live smoke deferred.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: Clean correction of #13912 — the same class rebuilt controller-blind (apply(serviceKey,action)), AiConfig-backed, and injection-safe. The lineage shows the review→reconcile loop working. The residual is purely the L0 sequencing.

🎯 Close-Target Audit

• Resolves #13884 (newline-isolated); Related: #13874 (non-closing).
• #13884 confirmed enhancement-labeled (leaf), not epic.

Findings: Pass.

📑 Contract Completeness Audit

• #13884's Contract Ledger (docker-socket actuator allowlist-wrapped + deploy-trigger + the crash — external container / config-drift cells) maps to the implementation: allowedComposeServices/allowedDeployTargets, restart vs page, persisted anti-thrash.
• Drift: the ledger names a "docker-socket + allowlist-wrapper"; the implementation uses the docker-compose CLI via execFile (a different access mechanism) with no socket mount — that's the L0/OQ-1 decision being made implicitly. Reconcile the ledger to the chosen L0 mechanism once OQ-1 lands.

Findings: Contract drift on the access mechanism (tied to the L0 blocker).

🪜 Evidence Audit

• Evidence: L2 declared and real (8/8 spec local at 9bb800a6 — allowlist, rejection-before-executor, persisted backoff, attempt-cap-no-loop).
• Gap: #13884's actual AC is restarting an external container = L3, reachable only via the L0 socket + #13914 to observe it. With no socket wiring + no observability, L3 is currently unprovable — the exact black box #13914 exists to close. Not a unit-test gap; a deployment-proof gap.

Findings: L2 solid; L3 (the close-target's real behavior) unprovable until L0 + #13914 land.

🧪 Test-Execution & Location Audit

• Checked out at exact head 9bb800a6; spec in canonical test/playwright/unit/ai/daemons/orchestrator/services/.
• Ran RecoveryActuatorService.spec.mjs → 8 passed (831ms). CI green.
• Note: the spec mocks execFileFn, so green tests confirm the allowlist/envelope/ledger logic but say nothing about the L0 docker-reachability gap.

Findings: Unit tests pass; they don't (and can't) cover the L0 deployment gap.

📋 Required Actions

• Hold for the L0/OQ-1 decision (#13860 re-sequence + ADR-0026 re-poll): the docker access mechanism (socket-API vs compose-CLI vs sidecar) must be the converged one, not implicitly chosen here.
• Extract the docker-access mechanism + allowlist + identity-resolution into the shared L0 primitive (your two-envelope design — one holder, read-observe for #13914 + write-lifecycle here), so #13914's read surface reuses it rather than a B1-only silo.
• Add the socket-mount / runtime-access wiring (docker-compose*.yml + orchestrator container provisioning) so the actuator can actually reach docker in cloud — or explicitly defer it to the L0 sub and mark this actuator's L3 AC as L0-blocked.
• Drop the per-getter ?? <default> duplication of the config.template.mjs:608-627 leaf defaults (ADR-0019 A6 drift-risk; the leaf owns the default — have tests inject the template-resolved config instead).
• Resolve the test/playwright/fixtures/aiConfigDefaults.mjs openAiCompatible.model edit — it conflicts with the identical edit in #13906 (both can't merge cleanly); pick one carrier.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 70 — controller-blind apply + AiConfig leaves + persisted-anti-thrash-for-B1 are all ADR-0026-correct; −30 for welding the docker access into B1 ahead of the L0 keystone (vs the shared two-envelope primitive).
• [CONTENT_COMPLETENESS]: 90 — thorough JSDoc + Fat-Ticket body + honest Evidence/Deltas; −10 for the contract-ledger access-mechanism drift.
• [EXECUTION_QUALITY]: 88 — injection-safe allowlist (rejection-before-executor, execFile-args, configured target.id), persisted backoff + alarm-only, default-disabled, 8/8 tests; −12 because the privileged action can't actually execute in cloud (no socket/CLI path).
• [PRODUCTIVITY]: 75 — delivers the B1 actuator logic, but unmergeable until L0 lands; the deliverable is gated, not done.
• [IMPACT]: 80 — the privileged heal half of the immune system; high once L0 + observability exist.
• [COMPLEXITY]: 70 — allowlist + envelope + ledger + executor across one service; well-factored but dense.
• [EFFORT_PROFILE]: Heavy Lift — net-new privileged actuator with persisted state + security boundary + full test matrix.

[RETROSPECTIVE] Convergent cross-family position: #13915 holds for the L0 decision (Grace re-sequence + Euclid reply agree). Once L0 lands as the shared two-envelope primitive, this actuator rebases onto it cleanly.