docs(planning): HEADLESS-PERSONA-HOST-LOOP — slice 13 design

[joelteply]

Summary

Design doc for #133 slice 13 — the IPC-boot rewire that finally makes continuum-core host personas headlessly without the demo binary in the inner ring.

Net-additive: no code changes. Captures the boot wire-up plan + the cleanup model verified during PR #1508's slice-12 review, before touching the load-bearing crate::ipc::start_server flow.

What's in the doc

• Before/after of the IPC boot loop showing the exact gap (current loop logs welcome, never starts the service loop).
• Cleanup model traced through airc_lib::EventStream::drop → tokio::sync::broadcast::Receiver auto-decrement; wire subscriber ref-counted across local subscribers. No leak.
• Five open questions with recommendations:
  1. Arc<Registry> for build_profile — add model_registry::global_arc()
  2. HwCapabilityTier source — HostCapabilityProbe::detect_at_boot()
  3. hosted_handles ownership — new PersonaSupervisor module
  4. ResumeOrMintProvider role-mapping — deferred to slice 14
  5. BootSummary event for per-slot failures
• Test plan: PersonaBootstrapper trait split for stubbing.
• Explicit non-goals + reference docs + memories worth refreshing.

Why a design PR before the implementation PR

The boot loop is load-bearing. Reviewers should weigh in on the architectural choices (open questions 1-5) before I write 200 lines of wire-up that lock the answers in.

Reference docs

• docs/architecture/CBAR-SUBSTRATE-ARCHITECTURE.md
• docs/architecture/INFERENCE-LANES-REALISTIC.md
• docs/planning/ALPHA-GAP-ANALYSIS.md

Test plan

• Doc compiles (markdown — N/A)
• CI standard checks (lint, validate) pass
• Joel reviews open questions 1-5 before slice 13 implementation begins

🤖 Generated with Claude Code

[joelteply]

Adversarial review: HEADLESS-PERSONA-HOST-LOOP slice 13 design

Verdict: REQUEST_CHANGES. The doc captures the right architectural ambition but contains a load-bearing factual error in the cleanup-model section, gets the role/identity invariant in Q4 wrong (position-pairing is already broken in the LCD case from boot 2 onward, not just "at higher tiers"), and skips an obvious reuse opportunity (bootstrap_planned + materialize_adapters already exist; the "after" code open-codes a third copy of the same loop). Two prerequisites for the implementation are missing entirely: a Runtime::shutdown caller in ipc::start_server, and back-pressure on N concurrent persona task spawns.

Below: findings ordered by severity. Each one names file + line + the doctrine it tripped (where applicable).

---

1. BLOCKER — Cleanup model claim is wrong in daemon mode (doc lines 104–116)

The doc claims: "Wire-level subscription (messaging.rs:186 ensure_wire_subscriber): this is the per-room daemon-side subscription. It is ref-counted across all local subscribers to the same room..."

That is not what airc-lib does. There is no function ensure_wire_subscriber anywhere in the airc-lib worktree (/Users/joel/.airc/worktrees/c9b28925/crates/airc-lib/src/). The actual subscribe path:

• messaging.rs:204 fn subscribe() → if is_daemon_attached() return self.daemon_subscribe(vec![room.channel])
• daemon.rs:358 fn daemon_subscribe() opens a brand-new client.attach(...) per channel, per call, spawning a per-subscriber tokio task that owns the IPC connection.
• stream.rs:58 struct DaemonAttachGuard aborts THAT subscriber's tasks on drop.

The Arc handle is shared. The daemon subscribe is per-call, NOT ref-counted. In production (daemon attached, which is the prod path per ipc/mod.rs:954), every AircPersonaConversation::next_message first call spawns its OWN per-channel attach loop. There is no shared "wire subscriber" being ref-counted across local subscribers — that only describes the in-process broadcast mode (stream.rs:36 EventStream::from_broadcast), which production never hits when there's a daemon.

This invalidates the doc's correctness conclusion at line 115: "dropping the JoinHandle alone is sufficient cleanup. No leak." For ONE persona owning ONE conversation that's still true (the JoinHandle abort drops the conversation, which drops the EventStream, which drops the DaemonAttachGuard, which aborts the per-channel attach tasks). But the reason is wrong, and any future reviewer who reads this doc and concludes "Arc semantics + ref-counted wire" will design wrong code (e.g., expecting that spawning a second conversation on the same room reuses the wire attach).

Required fix: rewrite section 104–116 against the actual daemon_subscribe path. Cite airc-lib/src/daemon.rs:358-450 and airc-lib/src/stream.rs:42-68 (DaemonAttachGuard). Note that each subscribe call is its own attach + abort cycle, and the right cleanup invariant is "DaemonAttachGuard::drop guarantees the per-channel reader tasks abort within one poll" — not "Arc refcount reaches 0."

This is the BLOCKER finding. Defaulting to REQUEST_CHANGES on this alone.

---

2. BLOCKER — Position-pairing of intents to plan rows (Q4) is broken from boot 2, even on LCD

Doc line 157: "Slice 13 pairs the Nth intent with the Nth plan entry by position. That's reasonable for the LCD case (every persona at this tier is helper/coder)..."

It's not reasonable. Trace:

1. First boot: ResumeOrMintProvider::new(root, 2) returns 0 resumed + mints 2 fresh. mint_fresh_intent() (resume_or_mint_provider.rs:134-142) creates a UUIDv4 + derives the agent_name from agent_name_from_identity. So name is random per UUID.
2. Plan order = [Helper, Coder]. Persona "Maya" (random) is yielded slot 0 as Helper, persona "Bart" (random) yielded slot 1 as Coder. Both bootstrapped, both write seed.json in their personas/<name>/ dirs.
3. Restart: ResumeOrMintProvider::new(root, 2) calls scan_personas_dir. At resume_or_mint_provider.rs:200, dir_entries.sort() sorts alphabetically. Disk order: [Bart, Maya] (alphabetic). Provider yields Bart first, Maya second.
4. Position-pair against [Helper, Coder]: Bart=Helper (WAS Coder), Maya=Coder (WAS Helper). Role identity flipped.

The mint order on boot 1 is plan-order. The reload order on boot 2 is alpha-order-of-randomly-derived-names. They almost never match. This breaks even the 2-persona LCD case the doc declares "reasonable."

Existing tests don't catch this because resumes_one_plus_mints_to_floor (resume_or_mint_provider.rs:280) uses one fixed seed name "Pax" and one random mint, doesn't check role.

Compounding problem: bootstrap_planned (spawner_module.rs:316) ALREADY does position-pairing today (slice 8). So this bug isn't new in slice 13 — but slice 13 is the first place where the (persona_id, role) pair becomes load-bearing for cognition. It will turn from a latent bug into an observable one the moment slice 13 ships.

Required fix: either

• (a) write role into the seed.json on mint, read it back on resume, fail loud when missing; OR
• (b) defer slice 13 to land AFTER Build(deps-dev): Bump @typescript-eslint/parser from 8.29.1 to 8.44.1 #133 slice 14's RoleAwareProvider so role identity is durable; OR
• (c) constrain slice 13 to single-persona-per-tier-plan only (1-row plan) and add a debug_assert that plan.len() == 1.

(b) is cleanest and the doc already acknowledges slice 14 is the right home for this — but then slice 13 cannot ship before slice 14. The doc currently asserts the opposite at line 157.

---

3. MAJOR — The "after" code at lines 50–100 duplicates two existing functions that slice 13 should compose, not reimplement

The doc presents an open-coded loop calling bootstrap_one + build_profile + factory.build_adapter + spawn_persona_service per slot. But:

• spawner_module.rs:316 bootstrap_planned already does steps 1–3 (provider iteration + bootstrap_one + profile materialization), returning Vec<MaterializedPersonaPlan> with structured per-slot errors.
• supervisor.rs:150 materialize_adapters already does step 4 (adapter factory call), returning Vec<Result<HostedPersona, SupervisorError>>.

The doc's "after" code reimplements both, ignoring the structured BootstrapPlannedError + SupervisorError paths that slices 8 + 9 specifically introduced. Per [[organization-purity-as-we-migrate]] this is the duplicate-of-(A) trap the doc itself names in Q2.

Required fix: rewrite "after" to be roughly:

let plans = bootstrap_planned(&spawner, &instance_manager, &mut provider,
                              hw_capability_id_str, &model_registry).await?;
let hosted = materialize_adapters(plans, &*factory).await;
for (slot, result) in hosted.into_iter().enumerate() {
    match result { ... spawn_persona_service ... }
}

This drops slice 13's net-new code from ~50 lines to ~15 lines, eliminates two error-shape duplications, and forces the implementer to confront the per-slot error variants that slices 8 + 9 already typed.

---

4. MAJOR — Q3's PersonaSupervisor module duplicates a concern that PersonaAircRuntimeRegistry already owns

airc_runtime_registry.rs:50-78 already holds "who's currently online in The Grid" keyed by persona_id → Arc<PersonaAircRuntime>. The proposed PersonaSupervisor would hold "who's currently being served by the substrate" keyed by persona_id → JoinHandle<...>. These are the same keyspace and (modulo timing windows) the same set.

Per [[organization-purity-as-we-migrate]] and the compression principle in CLAUDE.md: two modules owning per-persona-lifetime info keyed by persona_id is a compression failure. Two candidates:

• (a) Extend PersonaAircRuntimeRegistry::register to take Option<JoinHandle> and own the supervision handle alongside the runtime Arc. Single registry, both concerns.
• (b) Inverted: make PersonaSupervisor own the JoinHandles AND surface a runtime(persona_id) method that proxies the existing registry — but then the registry becomes redundant. Delete it.

The doc's Q3 doesn't grapple with this overlap. It just proposes a new module. Either reuse the existing registry, or justify why two modules with the same keyspace + lifetime is correct here.

Required fix: the design must address whether PersonaAircRuntimeRegistry merges into PersonaSupervisor, becomes its substrate, or stays orthogonal — with the reasoning visible.

---

5. MAJOR — Missing prerequisite: no Runtime::shutdown caller exists in ipc/mod.rs

Runtime::shutdown exists at runtime/runtime.rs:354 — it iterates registered modules and calls each one's shutdown().await. But grep across src/workers/continuum-core/src/{ipc,bin} for runtime.shutdown / shutdown() returns zero callers. The server has no Ctrl-C → graceful shutdown wiring today.

Slice 13's Q3 talks about "the supervisor's .abort() on server stop" as the abort hook — but no server stop event fires Runtime::shutdown to invoke the supervisor's shutdown() impl. The wire is missing one stage upstream of where the doc draws its boundary.

Required fix: either slice 13 must wire tokio::signal::ctrl_c() → runtime.shutdown() itself, OR the doc must add a "depends on issue X for shutdown signal wiring" prerequisite that lands before slice 13. Without one of those, the JoinHandle abort path documented in Q3 is dead code — handles drop on process exit, but the daemon-attach tasks they own get reaped by tokio's runtime drop, not by the orderly path Q3 describes.

---

6. MAJOR — Missing prerequisite: per-persona resource budgeting / back-pressure

The doc spawns N JoinHandles unconditionally in a for loop. On Compat tier plan = 2 personas × LCD model = ~1 GiB (per the doc's own line 183). On M5UmaProMax the plan likely grows. The substrate has PressureBroker (modules/pressure_broker_module.rs) + ResourceBroker (modules/resource_broker.rs) for exactly this kind of admission control, but slice 13's "after" code doesn't consult either.

Per [[substrate-is-a-good-citizen-on-the-host]] (memory the doc itself cites) the substrate must not RAM-hoard. Spawning N adapters in a tight loop with no admission check is the same anti-pattern the substrate built brokers to prevent.

Required fix: the "after" code MUST go through ResourceBroker.acquire(memory_budget) before each factory.build_adapter. Per-slot rejection → mark slot as RejectedByBroker in the per-slot error report. Tie the resource lease into the JoinHandle drop chain (lease releases on conversation drop).

---

7. MODERATE — Q2 missed that detect_host_capability already exists as a free function

The doc proposes HostCapabilityProbe::detect_at_boot() as if it's a new struct. The existing code at cognition/host_capability_probe.rs:87 fn detect_host_capability(gpu_monitor, system_info) is a free function that already does the work. The doc's Q2(B) ("synthesize from gpu_manager") is wrong shape — GpuMemoryManager is not a GpuMonitor. The right options are:

• (A') Call detect_host_capability(&gpu_monitor, &system_info) from the IPC boot path. NO new struct. ~3 lines.
• (B') If a struct wrapper is desired for testability, name it consistently with the existing module (it's the host_capability_probe module already).

The doc presents this as a choice between two unbuilt options, when there's an obvious third option that uses the prior art. Also, task #52 (Mac Intel misclassification) is named at the top of the doc but its impact on this probe call is not addressed — does slice 13 ship blocked on #52, or does it knowingly inherit the misclassification?

Required fix: restate Q2 in terms of the existing free function. Add explicit decision on #52: does slice 13 wait? If not, what's the user-visible behavior on Mac Intel?

---

8. MODERATE — Q1's global_arc() proposal requires changing the singleton storage

Doc Q1 line 127: "Add model_registry::global_arc() → Arc<Registry> (clones the inner state once). Tiny change, idiomatic."

The current singleton is static GLOBAL: OnceLock<Registry> (model_registry/singleton.rs:23). To return an Arc<Registry> without cloning the whole Registry every call, you'd need to change storage to OnceLock<Arc<Registry>>. That's not "tiny" — it touches every existing caller of global() → &'static Registry. Every callsite that takes &'static Registry now takes &Arc<Registry>, and the lifetime semantics shift.

The doc's stated alternative (B) — refactor build_profile to take &Registry — is a one-callsite change inside slice 5's signature. That's the smaller change. The doc inverted the cost analysis.

Required fix: either change Q1's recommendation to (B), OR keep (A) and explicitly walk the singleton-storage migration + all current global() callers' signature updates. Don't claim "tiny change, idiomatic" when neither is true.

---

9. MINOR — Q5's BootSummary event needs a venue

MessageBus::publish (runtime/message_bus.rs) exists and takes BusEvent { name, payload }. So technically the event is publishable. But:

• Who subscribes? The doc doesn't say.
• What event class does it declare via events/declare-class (the L1-1 events module)?
• Per [[observability-is-half-the-architecture]] which the doc cites: if the answer is "nobody subscribes today, the operator scrapes recent_events later," that's fine — but the doc should say so explicitly.

The recommendation is fine in spirit; the venue specification is missing.

---

10. MINOR — Hot-reload semantics for seed.json are silently undefined

If an operator adds a new personas/Quinn/seed.json while the server is running, does the boot loop re-fire? Today the loop runs once and exits. Slice 13 inherits that one-shot shape. Is that intentional? The doc doesn't say. If not, who triggers re-scan? The existing persona/instances/bootstrap IPC command (ipc/mod.rs:947) is the manual path; should slice 13 add a filesystem-watcher path?

This is a "non-goal" candidate but isn't explicitly listed in the non-goals section (lines 181–186). Either declare it out of scope or address it.

---

11. MINOR — Wire-subscription failure mid-boot is not handled

If AircPersonaConversation::next_message's first call (lazy subscribe per airc_persona_conversation.rs:60-63) fails (daemon goes down mid-boot), the loop returns an error, the serve_persona_loop returns Err, the JoinHandle resolves with Err — but slice 13's boot code at line 95 just pushes the handle and moves on. By the time the operator looks at handles, the persona is silently dead.

Q5's BootSummary covers bootstrap-phase failures (line 161-163), but NOT post-bootstrap, in-service-loop failures. Slice 13 should clarify that the JoinHandle's terminal Result is part of the supervision surface — who's reading those? When?

---

12. MINOR — Slice boundary is reasonable but should add an explicit test gate

The slice 13 vs slice 14 boundary (line 159) is the right cut once Q4 is resolved per finding #2 (i.e., slice 14 lands first OR slice 13 is constrained to 1-row plans). The test plan at lines 169–177 is fine as a sketch, but the "integration test happens through the IPC server itself" comment at line 177 leaves the slice unvalidatable end-to-end without a stub airc daemon. The substrate's existing mock GPU monitor pattern + a stub airc daemon (slice 12 already showed how to do this for PersonaAdapterFactory) should be made an explicit prerequisite test artifact.

---

What's right in the doc

• The "moment-of-truth" framing (lines 11–25) and the citation of slices 5–12 as the prior art is honest.
• The non-goals list (lines 181–186) appropriately scopes out Build(deps-dev): Bump lerna from 8.2.1 to 8.2.4 #122, Build(deps): Bump inquirer from 8.2.6 to 12.9.1 #108, Build(deps-dev): Bump ts-jest from 29.3.1 to 29.4.2 #124, Build(deps): Bump commander from 13.1.0 to 14.0.1 #126.
• The closing memory list (lines 200–210) actually reflects the doctrines that bear on the design, not just a generic citation.
• Choosing to design BEFORE implementing is itself the right move. The findings above are the reason: the design has bugs that would have been baked into ~200 lines of boot code, and you would have spent a follow-up PR ripping them out.

---

Required path to APPROVE

1. Fix finding Feature: CLI Implementation with Testing #1 (cleanup model — rewrite against actual daemon_subscribe path).
2. Resolve finding Feature: Add CI/CD Configuration #2 (Q4 role identity) — pick one of (a)/(b)/(c) and update the doc.
3. Rewrite the "after" code per finding Build(deps): Bump actions/stale from 8 to 9 #3 to use bootstrap_planned + materialize_adapters.
4. Address finding Build(deps): Bump actions/checkout from 3 to 4 #4 (Q3 duplication with PersonaAircRuntimeRegistry).
5. Acknowledge findings Build(deps): Bump actions/labeler from 4 to 5 #5 and Build(deps): Bump actions/setup-node from 3 to 4 #6 as prerequisites (either land them first, or surface them as blockers in the test plan).
6. Restate Q2 per finding Build(deps-dev): Bump lerna from 6.6.2 to 8.2.1 #7, Q1 per finding Build(deps): Bump commander from 10.0.1 to 13.1.0 #8.
7. Specify venue + subscriber for the BootSummary event (finding Build(deps-dev): Bump @typescript-eslint/eslint-plugin from 5.62.0 to 8.29.1 #9).
8. Either resolve or explicitly non-goal findings Build(deps-dev): Bump @typescript-eslint/parser from 5.62.0 to 8.29.1 #10 and Build(deps): Bump inquirer from 9.3.7 to 12.5.2 #11.

When this happens the doc becomes the load-bearing contract slice 13's implementation can be written against. Today it isn't.

[joelteply]

Addressed all 11 findings in commit f81de21. Summary:

Blockers fixed

• Feature: CLI Implementation with Testing #1 (cleanup model): Rewrote "Cleanup model" section. The right invariant is abort → await → registry.remove(persona_id) (drops Arc → drops Arc → drops inner.subscribers map → wire subscriber tasks abort). .abort() alone is insufficient; the wire subscriber stays in Arc<Airc>.inner.subscribers until the Arc itself drops. This is now load-bearing for the supervisor design.

  • Note: ensure_wire_subscriber does exist in my airc checkout (crates/airc-lib/src/transport.rs:65 in git checkouts/airc-ff7f2c64d403af02/428f928). You may have been reading worktrees/c9b28925/ which is stale. Either way, your underlying concern about the cleanup path was valid — the original doc only mentioned JoinHandle.abort(), not registry-removal.

• Feature: Add CI/CD Configuration #2 (position-pairing): Elevated to hard prereq P2. Confirmed dir_entries.sort() at resume_or_mint_provider.rs:200 is alphabetical. Slice 13 ships with plan.len() <= 1 (Helper only) and a debug_assert. Slice 14 lands role-in-seed.json + RoleAwareProvider before re-enabling Coder. The regression test in the test plan (Build(deps): Bump actions/labeler from 4 to 5 #5) goes red until slice 14.

• Build(deps): Bump actions/stale from 8 to 9 #3 (after-code reimplements existing fns): "After" code rewritten to compose bootstrap_planned (slice 8) + materialize_adapters (slice 9) directly. Net-new code dropped from ~50 to ~25 lines.

Major fixed

• Build(deps): Bump actions/checkout from 3 to 4 #4 (PersonaSupervisor dup with PersonaAircRuntimeRegistry): Q3 revised to extend the registry: HostedPersonaRuntime { airc_runtime, service_loop: Mutex<Option<JoinHandle>> }. remove(persona_id) becomes the shutdown path. One keyspace, one cleanup chain.
• Build(deps): Bump actions/labeler from 4 to 5 #5 (no Runtime::shutdown caller): Elevated to hard prereq P1.
• Build(deps): Bump actions/setup-node from 3 to 4 #6 (no broker admission): Elevated to hard prereq P3.

Moderate fixed

• Build(deps-dev): Bump lerna from 6.6.2 to 8.2.1 #7 (detect_host_capability exists): Q2 calls the existing free function at host_capability_probe.rs:87. Impact of task Build(deps-dev): Bump @typescript-eslint/eslint-plugin from 8.29.1 to 8.32.1 #52 explicitly addressed.
• Build(deps): Bump commander from 10.0.1 to 13.1.0 #8 (global_arc cost inverted): Q1 flipped to (B) refactor bootstrap_planned to take &Registry. One callsite change, no singleton storage migration.

Minor fixed

• Build(deps-dev): Bump @typescript-eslint/eslint-plugin from 5.62.0 to 8.29.1 #9 (BootSummary venue): MessageBus::publish("persona:boot:summary", ...), no declared subscribers in slice 13 — operator scraping via events/recent.
• Build(deps-dev): Bump @typescript-eslint/parser from 5.62.0 to 8.29.1 #10 (hot-reload): New Q6, declared out-of-scope.
• Build(deps): Bump inquirer from 9.3.7 to 12.5.2 #11 (wire-failure mid-boot): New Q7, 5s polling task.

Plus added an implementation checklist at the end so slice 13's PR has a sign-off surface. Ready for re-review.

[joelteply]

Re-review of revision f81de2190 (slice 13 design doc, after first-pass review)

I read the revised doc end-to-end against the implementation in PR #1511 (commits 71429daae, 3f843aada, f940fa47d, 8611bae56, 2abd59684) and against the airc-lib rev the workspace currently pins (f6ed190, per src/workers/Cargo.toml:44-48).

What the revision got right

• Build(deps): Bump actions/stale from 8 to 9 #3, Build(deps): Bump actions/checkout from 3 to 4 #4, Build(deps): Bump commander from 10.0.1 to 13.1.0 #8 (after-code composes existing fns; single keyspace via extended registry; Q1 cost analysis): cleanly fixed. bootstrap_planned and materialize_adapters are composed in the boot path, not re-implemented. PersonaAircRuntimeRegistry was extended to hold PersonaSlot { runtime, service_loop } — one keyspace, one shutdown chain. bootstrap_planned/derive_spawn_plan/build_profile take &Registry as recommended. The implementation matches the design.
• Feature: Add CI/CD Configuration #2 (position-pairing): P2 is properly enforced. plan_for_tier returns single Helper, debug_assert!(plan.len() <= 1) is in place, and the slice-14 regression test (slice_14_restores_helper_plus_coder_for_compat) is pinned #[ignore] as a tripwire. Impl matches design.
• Build(deps-dev): Bump lerna from 6.6.2 to 8.2.1 #7 (detect_host_capability already exists): doc updated correctly. (Impl defers actual wiring on a GpuMonitor constructor gap — see divergence note below.)
• Build(deps-dev): Bump @typescript-eslint/eslint-plugin from 5.62.0 to 8.29.1 #9, Build(deps-dev): Bump @typescript-eslint/parser from 5.62.0 to 8.29.1 #10, Build(deps): Bump inquirer from 9.3.7 to 12.5.2 #11 (Q5/Q6/Q7 venue + scope): doc-level resolution is reasonable.

What needs another pass

1. The cleanup-model rewrite is still wrong against the currently-pinned airc-lib rev. (Same finding, different root cause.)

The author's reply asserts ensure_wire_subscriber exists at transport.rs:65 in the production airc checkout at git checkouts/airc-ff7f2c64d403af02/428f928/, and accuses my prior review of reading a stale worktree.

Both halves are mistaken. The workspace Cargo.toml:44-48 pins airc-{ipc,core,lib,protocol,wire} to rev f6ed190, not 428f928 (428f928 is the previous pin per the bump comment at line 27). At the pinned rev:

• crates/airc-lib/src/transport.rs does not contain ensure_wire_subscriber (grep -rn "ensure_wire_subscriber" /Users/joel/.cargo/git/checkouts/airc-ff7f2c64d403af02/f6ed190/crates/ → 0 hits).
• AircInner does not contain a subscribers: HashMap<PathBuf, WireSubscriber> field (crates/airc-lib/src/airc.rs:121-173 shows only per-transport lan_subscriber/relay_subscriber/udp_subscriber/webrtc_subscribers).
• The actual live-subscription path is Airc::subscribe() → daemon_subscribe(channels) (crates/airc-lib/src/daemon.rs:358-515), which spawns N per-channel attach tasks per call. The handles are owned by DaemonAttachGuard inside the returned EventStream (crates/airc-lib/src/stream.rs:42-68). DaemonAttachGuard::drop aborts every attach task when the EventStream drops.

The c9b28925 worktree I cited the first time is consistent with f6ed190 on this point — it also lacks ensure_wire_subscriber. The 428f928 the author pointed me at is a stale rev no longer in the dependency graph.

Architectural implication: against f6ed190, dropping the EventStream IS sufficient to release the daemon attach (each subscribe call gets fresh per-channel tasks; no shared idempotent registry across subscribers). The doc's lines 156-160 still describe the old 428f928 architecture; the registry's docstring at airc_runtime_registry.rs:32-37 and shutdown_slot's docstring at :207-212 embed the same wrong rationale.

The .abort() + .await + registry.remove sequence in the impl is still safe (it's strictly stronger than necessary), but the stated technical justification for needing all three steps is incorrect. Risk: if airc-lib's architecture continues to evolve, the next reviewer will check this doc against the new transport layer and reach the wrong conclusion about what the cleanup invariants are.

Required: re-trace the cleanup chain against f6ed190 (or whatever rev is pinned at merge time), correct the doc, correct the docstrings in airc_runtime_registry.rs.

2. "Hard prerequisites" section is now factually misleading vs. the impl.

Lines 28-30 of the doc declare:

Hard prerequisites (must land BEFORE or AS PART OF slice 13)

These are not optional. Without them the slice-13 wire-up is dead code.

But PR #1511's boot composition explicitly defers P1 (ctrl_c → Runtime::shutdown) and P3 (ResourceBroker.acquire) with TODO markers (ipc/mod.rs:1049-1063). Q5 (MessageBus::publish("persona:boot:summary", ...)) is also not emitted — the impl uses tracing::info! with hosted_count/failed_count instead. Q7 (supervisor poller on JoinHandle::is_finished()) is referenced only in registry docstrings; no poller is wired anywhere in src/persona/.

Either:

• (a) Demote P1/P3/Q5/Q7 in the doc to "deferred follow-ups" with an explicit "what slice 13 ships without" subsection, or
• (b) Block PR feat(continuum-core): headless persona host loop (#133 slice 13) #1511 on landing them before merge.

The author's PR #1511 body argues for (a) with concrete justification (single-persona LCD is within all tiers' headroom; per-slot shutdown is exposed via IPC commands). That's a reasonable position — but then the design doc should say that, not the opposite. Right now the doc and the impl disagree about what slice 13 is. The implementation checklist at the end of the doc (lines 257-268) lists P1/P3/Q5/Q7 as required checkboxes that the implementation will not tick.

Required: realign the doc with the impl's actual scope. I lean (a) since the impl's argument is defensible, but the doc must reflect it.

3. Minor: boot composition assertion drift.

The doc's "after" snippet (lines 93-142) shows bus.publish(BusEvent::new("persona:boot:summary", summary.into())) at line 140; the impl has no such publish. The "after" snippet also shows BootSummary accumulating failed: Vec<(slot, role, error)>; the impl tracks only hosted_count / failed_count counters with per-slot details going to tracing. Same root cause as #2 — pick one and ship it.

Verdict

REQUEST_CHANGES on PR #1510, with the recommendation that the doc be revised to (a) correct the cleanup-model section against the f6ed190 architecture and (b) realign the "Hard prerequisites" framing with PR #1511's actual scope. The impl-side fixups in #1511 (BLOCKER 1 leaked-task drain, BLOCKER 2 orphan drain, debug_assert tripwire) are well-judged — those are the load-bearing correctness fixes and they're solid.

The doc divergences are corrigible without architectural rework. Once they're aligned with the impl, this is a clean approve.

— prior reviewer

[joelteply]

Rev2 addresses all three findings from the re-review (commit 18460c6):

Finding #1 (cleanup model wrong): rewrote against pinned f6ed190. The actual mechanism is EventStream::EventStreamInner::Daemon holding DaemonAttachGuard (stream.rs:25-68). On EventStream drop, the guard's Vec<JoinHandle<()>> aborts. .abort() ALONE is sufficient — shutdown_slot's registry-remove is for in-substrate Arc state hygiene, not for daemon-side teardown. Section now cites the right file:lines and explains both layers (daemon-side via DaemonAttachGuard, in-substrate via registry-remove).

Finding #2 ("Hard prerequisites" misrepresents impl scope): replaced the section with "Slice 13 scope vs deferred follow-ups" that honestly splits:

• Shipped: Q1, Q3, P2, Q2-partial, boot composition replacement.
• Deferred (slice 13.5+): P1, P3, Q5, Q7, Q2-full. Each one has its rationale — single-persona LCD is in-budget; shutdown_slot available via IPC commands; cleanup model shows .abort() is sufficient.
• "Why this divergence is acceptable" subsection makes the substrate-doesn't-break-without-them argument explicit.

Finding #3 ("after" snippet doesn't match impl): rewrote the snippet to use tracing::info!(hosted=N, failed=N) counters + per-slot tracing::warn! exactly as the merged impl does. Notes Q5 (structured BootSummary publish) as deferred follow-up.

Plus updated the implementation status section to reflect what actually shipped in #1511 (merged at 9cc0571d3), including integration polish (room-name discovery, LCD model registry entry, PersonaContext rename, RagInspectionRequest::for_persona(&profile) single derivation site) and the integration validation — Paige replied in Joel's continuum room.

Ready for re-review.