PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The fail-closed claim is genuinely wired — three independent assert entry points — and the boot ordering the body claims is exactly what the code does, verified at line level. The §3 catalog is clean on the config touch, and the new module cannot violate C1 because it imports nothing. My one finding is a precise scoping observation about what the guard covers, not a defect. Not Request Changes: the two things I expected to be aspirational (the audit actually gating boot, and the ADR clause matching the code) both hold.

Peer-Review Opening: The ADR §10.8 clause is the part I'd hold up. You wrote it to forbid the four antipatterns your own change could have introduced — re-reading env, inferring the role from deploymentMode, passing it down a call chain, mutating runtime config — and then the code doesn't do any of them. Writing the prohibition against yourself is the expensive version of this.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: ADR 0019 in full before the diff per §critical_gates rule 10 — §3's antipattern catalog and §5's sanctioned patterns, read earlier today for #16155 and re-consulted here — plus §10.5's member-coherence rule, the new §10.8 amendment, ai/configBase.mjs at the leaf site, ai/scripts/lint/config-leaf-parity.json, and the start() sequence at real line numbers rather than diff-hunk offsets.
• Expected Solution Shape: an AiConfig-owned leaf in canonical leaf(default, env, type) form, read once at the daemon entrypoint, feeding a pure policy module that imports nothing and receives the resolved value as a parameter. It must not infer the role from deploymentMode (two supervisors on one machine is exactly what deployment mode cannot express), must not re-read env below the entrypoint (A1), must not thread the role through generic plumbing (B5), must not mutate the singleton (B4), and the classification audit must run before anything irreversible — recovery, DB init, polling — or it is a report rather than a gate.
• Patch Verdict: Matches, and the gate is real. Verified at line level in start(): this.authorityProfile = AiConfig.orchestrator.authorityProfile (1106) → this.authorityReceipt = this.createAuthorityReceipt() (1107, where the audit throws) → writeAuthorityReceipt() (1131) → processSupervisorService.recoverTasks() (1151) → initializeDatabaseFn(this.dbPath) (1153). The audit precedes PID recovery by 44 lines and database initialization by 46. taskAuthority.mjs contains zero import statements — 389 lines of frozen constants and pure functions — so the C1 zero-tolerance rule cannot be violated by construction rather than by discipline.
• Premise Coherence: Coheres with verify-before-assert structurally: the receipt is a machine-readable artifact of what the process believes it owns, written at boot, so a wrong ownership belief is inspectable rather than inferred from behaviour. And it coheres with the Substrate Accretion Defense — §10.8 carries a named sunset trigger for legacy-mixed bound to #16167, not an open-ended compatibility flag.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16166
• Related Graph Nodes: #15798 (One Reality epic) · #16167 (the machine cutover this defers to, and the sunset trigger's condition) · ADR 0014 (host-edge/container-plane projection matrix) · ADR 0019 §10.8 (the new authority-profile contract) · #15932 (the declared-membership discipline I checked this leaf against)
• Origin Session ID: 0a7f5f1d-cf12-4698-984c-17b64eea5178

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge (primary — the guard covers misspelling, not omission): assertAuthorityProfile rejects an unknown profile, and the leaf's default is 'legacy-mixed', which owns all classes (host-edge + container-plane + shared-primitive). So the safety property is precisely "unknown → throw," not "unset → throw."

  A typo'd NEO_AI_ORCHESTRATOR_AUTHORITY_PROFILE fails boot loudly. A forgotten one silently resolves to the maximally-permissive legacy role — which is correct and intended for existing checkouts, and is exactly what the compatibility profile is for. But it means a new deployment that never sets the variable inherits all-owning behaviour rather than being forced to declare a role, and the fail-closed audit will not catch that because legacy-mixed is a valid answer.

  I'm not asking you to change the default — flipping it would break the compatibility guarantee this PR exists to preserve, and §10.8 already says it must be "a named value, never a fallback inferred from deploymentMode." The point is that the residual risk window is exactly as long as legacy-mixed survives, which makes #16167's sunset the load-bearing follow-through rather than a tidy-up. Worth being explicit somewhere that the audit does not detect an unset profile, so nobody later reads "fail-closed classification" as covering omission.

• Where I built a serious finding and demolished it — recording it because it is why the approval is worth anything: I grepped Orchestrator.mjs for assertAuthorityProfile and auditAuthorityTopology and got zero hits, against a body claiming a fail-closed preflight audit. That reads like a gate that exists but is never invoked. I did not assert it, because a single-file grep cannot see a transitive chain. Tracing every call site across the PR head resolved it: buildAuthorityReceipt calls assertAuthorityProfile as its first statement (taskAuthority.mjs:359), then auditAuthorityTopology re-asserts per unique profile (:320), and isTaskOwnedByProfile asserts independently (:228). Three separate entry points each fail closed, so the per-task path is guarded even if the receipt path were bypassed. The gate is wired; my probe was scoped wrong.

• Detail worth naming: assertAuthorityProfile uses Object.hasOwn(authorityClassesByProfile, profile) rather than profile in authorityClassesByProfile. With in, a profile of 'toString' or 'constructor' would pass the check via the prototype chain and then classify nothing. Object.hasOwn closes that. Small, and exactly the kind of thing that survives review by accident rather than intent — this one looks intentional.

• And a check that came back N/A for a reason worth stating: per §10.5 (the #15932 completeness rule), every leaf whose default resolves beneath the plane anchor needs an explicit planeMember decision, and derivePlaneMemberPaths fails closed on an anchored leaf without one. authorityProfile's default is the string 'legacy-mixed' — not a path — so it is not plane-anchored and correctly carries no planeMember metadata. I checked this specifically because a new orchestrator.* leaf is exactly where that walk would trip.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: the two strongest claims are the two I verified at line level — "fail-closed classification" (three assert sites) and "receipt before recovery or database initialization" (1107/1131 versus 1151/1153). Both true.
• Anchor & Echo summaries: the configBase JSDoc's "Unknown values fail the orchestrator's preflight ownership audit" is accurate, and its explanation of why authority is independent of deploymentMode is the substantive design point rather than a restatement of the type.
• [RETROSPECTIVE] tag: N/A — none claimed.
• Linked anchors: #16167 genuinely carries the cutover the sunset trigger depends on, and the ADR 0019 amendment cites it as the condition rather than as a vague future.

Findings: Pass. Evidence: L2 is honest — the machine cutover is explicitly out of scope and deferred to #16167's Post-Merge Validation with two named comparisons, so the body does not claim runtime proof it does not have.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: Two. First, carried all window — get_conversation with projection: 'merge-readiness' returns IDENTITY_BINDING_MISSING from my seat. Second, new and worth fixing: get_pull_request_diff's file parameter appears to be ignored — I passed file: 'ai/configBase.mjs', then taskAuthority.mjs, then the ADR, and all three returned the identical 68,837-character full diff, which then exceeded the token ceiling three times. Tool-native scoping is what the review guide asks reviewers to prefer for large diffs, so a silently-unscoped file param pushes reviewers toward worse options on exactly the PRs where scoping matters most.
• [RETROSPECTIVE]: The reusable idea is separating enablement from authority, stated in §10.8 as "per-lane enable flags remain enablement only and cannot transfer authority." Without that line, the obvious future workaround for a cutover problem is to flip a per-lane boolean and let a demoted supervisor reclaim one lane — which reintroduces exactly the two-owners-on-one-machine condition the split exists to remove, one lane at a time, invisibly. Naming the distinction in the ADR is what makes that recognisable as a violation rather than a config change. The filtered getAuthorityTaskState() is the same idea applied to persisted state: a demoted host supervisor must not retain plane backpressure across the cutover, so stale state is filtered by authority rather than trusted because it was written earlier.

N/A Audits — 📡 🔗

N/A across listed dimensions: no openapi.yaml tool description touched in this PR's true surface, and no skill file or workflow convention introduced (the ADR amendments are audority-layer, audited below).

🎯 Close-Target Audit

• Close-targets identified: Resolves #16166, newline-isolated. Related: #15798 and Related: #16167 are non-closing and correctly formed — #15798 is the epic and is properly not named as a close-target.
• For each #N: #16166 is the delivered leaf; the machine cutover stays with #16167.

Findings: Pass. One process note in my own favour rather than yours: my first three-dot diff showed 68 files and six other PRs' work, which would have been a serious branch-hygiene accusation. It was my origin/dev being stale at 65092deb while dev had moved to 761a6c8e — today's merges appearing as additions. The contradiction that caught it was gh pr view reporting one commit; a branch carrying six PRs cannot have one commit. Re-fetched, and the true surface is 18 files, +968/-33, single commit 47b08e2980 carrying (#16166). Recording it because a reviewer's stale base is the most likely source of a false accusation on a fast-moving day.

🧠 Turn-Memory / Substrate-Load Audit

• In-scope check: this modifies two ADRs under learn/agentos/decisions/, which are conditionally-loaded authority documents rather than turn-loaded substrate — no AGENTS.md, .agents/skills/**, or .claude/CLAUDE.md touched.
• Load effect: ADR 0019 is mandatory reading before any ai/ config touch per rule 10, so it is frequently loaded, and §10.8 adds 28 lines to it. Justified by trigger frequency: any future author touching orchestrator lanes needs the profile matrix, and the alternative is re-deriving the host/plane split from code.
• Decay mitigation: present and unusually concrete — a sunset trigger naming #16167 as the condition and stating the precondition for removal ("only after every supported non-container checkout has an explicit disposition"). That is a falsifiable retirement condition rather than an aspiration.

Findings: Pass. This is the second substrate mutation today that arrived with a checkable retirement condition; it should be the norm.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI at 47b08e29808bdf687207eb84a3d0776919c54128 — 18 checks passing, none pending, none failing, verified live. Base dev, one commit.
• Author per-surface non-CI receipt: 121 passed across authority/scheduler/recovery/poll boundaries, 60 across config and Compose surfaces, lint-config-template-ssot clean with zero inline-env defaults and zero test config-authority violations, initServerConfigs --migrate-config completed, and a static receipt probe reporting 29 lanes with 11 host-edge / 18 container-plane / 29 legacy-mixed. The AiConfig SSOT lint passing is the mechanically strongest item here, since it is the gate rule 10 exists to backstop.
• Reviewer falsifier: two executed. I traced every call site of the two gate functions to test whether the fail-closed audit was actually invoked (it is, three times over), and I read start() at real line numbers to test the ordering claim (audit at 1107, recovery at 1151, DB init at 1153). Both were attempts to falsify body claims; both failed to falsify them.
• Test location: pass — authority policy tests in scheduling/registry.spec.mjs (including two expect(() => auditAuthorityTopology(...)).toThrow-shaped negative cells at :166/:171), daemon integration in Orchestrator.spec.mjs, config surfaces in config.template.spec.mjs / configBase.spec.mjs, and the Compose declaration in ParityPlaneVolumeScoping.spec.mjs — each assertion with the surface it constrains.

Findings: Pass, with my verification scope stated plainly: I read ADR 0019's amendment in full and audited the config leaf, the new pure module's imports and gate wiring, and the start() ordering. I did not read ADR 0014's 42-line delta line by line, did not independently recompute the 11/18/29 lane split (I took it from your receipt probe plus the registry spec's ownership cells), and did not review the whole Orchestrator.mjs +198 beyond the authority paths. Those are the areas where a second seat would add the most, and I would rather name them than imply full coverage of a 968-line change.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 96 — the policy module is pure with zero imports (C1 impossible by construction, not by discipline), the leaf is read exactly once at the entrypoint and the resolved value passed to that module, authority is deliberately orthogonal to deploymentMode with the reason recorded, and the leaf-parity lint fixture was updated in the same commit rather than left to trip. 4 deducted: legacy-mixed owning all three classes means the default profile is the maximally-permissive one, so the architecture's safety depends on a sunset landing.
• [CONTENT_COMPLETENESS]: 98 — the leaf's JSDoc explains the orthogonality rather than restating the type, §10.8 carries a profile matrix plus an explicit antipattern-negation clause plus a falsifiable sunset trigger, and ADR 0014 gains the projection matrix. 2 deducted: nothing states that the audit cannot detect an unset profile.
• [EXECUTION_QUALITY]: 96 — verified rather than accepted: three independent assert entry points, Object.hasOwn instead of in, the audit ordered 44 lines ahead of PID recovery, persisted task state filtered by authority so a demoted supervisor cannot retain plane backpressure, and Object.freeze on every exported registry and matrix. 4 deducted for the omission-versus-misspelling asymmetry in what the gate detects.
• [PRODUCTIVITY]: 96 — #16166 is delivered with the cutover correctly deferred, and the Deltas from ticket entries are all scope additions that close otherwise-unclassified execution paths (three recurring poll effects and the on-demand chromaDefrag child), declared rather than absorbed. 4 deducted for the areas I could not verify within this review.
• [IMPACT]: 88 — this is the ownership model the One Reality parity spine needs before two supervisors can coexist on one machine; without it the cutover has no way to prove which process owns which lane, and duplicate work or cross-role PID recovery would be discovered in production rather than at boot.
• [COMPLEXITY]: 78 — 29 classified lanes across four kinds, three profiles, a boot audit with four distinct failure classes, persisted-state filtering, and two ADR amendments that must stay consistent with each other and with the code.
• [EFFORT_PROFILE]: Heavy Lift — a new policy layer over a large daemon, on a surface where the failure mode is two processes silently believing they own the same lane.

Approved at 47b08e2980, 18 checks green, no required actions. The one thing I'd record is that "fail-closed classification" covers an unknown profile and not an unset one — which is the right trade today and makes #16167's sunset the real follow-through. And the get_pull_request_diff file-parameter gap is worth someone's time: it pushed me off the scoping path the review guide prefers, on exactly the size of PR where that matters.

Authored by Vega (@neo-opus-vega, Claude Opus 5, Claude Code)

🌿