# Owner-Isolated Diagnostic Mode — Stage 26 Critic Review

Reviewed immutable revision `/home/cube/projects/richard/traning coach/.gjc/_session-019f8455-334a-7000-99ca-318dfd0e06b1/plans/ralplan/019f8455-334a-7000-99ca-318dfd0e06b1/stage-26-revision.md`. The RALPLAN index records that path as revision stage 26 with SHA-256 `1f30eaf3b39e9d63341ff1bf000e0d116b1037a56bbbf80ee643d480cb190e33`, matching the assignment. Review was read-only; no tests or live Telegram/deployment actions were run.

## Verdict
**ITERATE**

## Claim Checks

- **Stage-25/earlier boundary corrections are substantially incorporated.** Revision 26 preserves ordinary same-user/distinct-full-triple legality, chooses a hot-attached in-process topology, gives the isolated registry an explicit marker, requires ordinary loaders to reject it, disables ambient trainer-DM inference, preserves ordinary `GateDPreflightReceipt` v1, adds a non-creating read lock, enumerates incident classes and bounded inputs, names synthetic-provenance enforcement targets, closes promotion categories, and retains the human-only live Telegram/manual P2–P6/deployment boundary.
- **The identity claim matches current code.** `checkin_cli/customer_coaching.py` rejects equal full triples and equal chat/topic spaces but not equal `user_id` alone. Revision 26 correctly avoids tightening ordinary identity semantics.
- **The proposed ordinary/diagnostic loader split fits the current runtime.** Profile `customer_admin.load_runtime_customer_registry` validates the one canonical ordinary registry, and gateway `nutrition_coaching.load_committed_customer_registry` calls it. Current `NutritionCoachingCoordinator._configure_registry` exposes enabled customers only. A marked diagnostic loader plus transactional isolated enablement is therefore the right shape, and both ordinary-loader rejection points are correctly named.
- **The private-DM target is real.** Current gateway coordinator construction creates a trainer private-DM bridge from trainer `user_id`, separately from the pinned trainer group/topic route. Revision 26 explicitly removes that ambient diagnostic path and names text/callback/active-binding provider-zero tests.
- **The read-only lock correction is implementable.** Existing `profile_authority_lock` creates/chmods `data/.adaptive-authority.lock` and takes an exclusive lock. A separate read-only/shared lock that requires the pre-existing private lock, as Revision 26 specifies, can meet the no-mutation tree assertion.
- **The bootstrap-to-runtime data flow is not yet executable without invention.** Current `NutritionCoachingConfig` contains only the ordinary relative registry path, `AdaptiveNutritionConfig` contains review/delivery fields, and `NutritionCoachingCoordinator` owns only its ordinary `_profile_root`. The proposed `attach_diagnostic_runtime(capability)` must call `load_diagnostic_runtime_customer_registry(root, capability)`, but the enumerated capability carries only an isolated-profile digest and the plan defines neither the `root` source nor a closed configured/spec mapping from that digest to a physically separate profile. Passing the coordinator's existing root would contradict the separate-root requirement; accepting a caller path or scanning would contradict the admission rules.
- **Bot ownership is unresolved at the chosen topology boundary.** One running `TelegramAdapter` owns one bot connection. Revision 26 requires a test bot distinct from production while hot-attaching routes to an already-running ordinary gateway, but it does not say whether that adapter is already the test-bot gateway, how its bot identity is pinned without live-network automation, or whether a second adapter is intended. Executors cannot prove `test_bot_separate` or route the three test destinations until this is fixed.
- **Cross-root authority and boot fencing remain underspecified.** Session/registry writes are said to run under `profile_authority_lock`, but canonical owner/config pins belong to the ordinary root while diagnostic registry/session rows belong to the isolated root. No deterministic two-root lock order, append-adjacent live revalidation rule, or root ownership is given. Revision 26 also uses a gateway boot epoch/digest without defining its cryptographically random generation, lifetime, injection into profile APIs, or stable test seam; those details existed more clearly in Revision 25 and are required for restart fencing.
- **Snapshot/replay is much more concrete, but its source authority is still ambiguous.** The incident map names source domains and bounded facts, yet it does not state whether snapshots read the isolated disposable customer or a separate ordinary/production incident source, how that source root/customer is authorized by a capability bound to the isolated profile, or which canonical revision token applies to mutable JSON documents that have no terminal JSONL row digest. This prevents a deterministic implementation of the multi-ledger fence and the claimed no-production-reach boundary.
- **Promotion trust roots are not exact enough.** “Profile source workspace and Hermes repository” is descriptive rather than a fixed resolved root contract, and “explicit migration source files” are not enumerated. The existing profile workspace and Hermes files exist, but an executor must still invent how the manifest derives those roots and proves a caller cannot substitute another repository.
- **Verification is categorized but not self-contained.** The current repositories contain all named existing profile/gateway tests and modules; the proposed `diagnostic_isolation.py` and `test_diagnostic_isolation.py` are appropriately new and do not yet exist. Revision 26 omits the exact cwd/interpreter/commands retained in the earlier consensus plan, does not say whether gateway full `tests/gateway` is mandatory, and uses “zero lifecycle rows” without distinguishing diagnostic session/transition rows from nutrition lifecycle/delivery rows.

### Representative implementation simulation

1. **Prepare and attach:** adding the four typed `AdaptiveOperatorService` actions is straightforward, but the service cannot build the listed pins or call the diagnostic loader because no exact diagnostic spec/config source provides the isolated root, disposable key, current adapter bot identity, or three raw destinations. The digest-only capability cannot locate or validate the separate registry by itself.
2. **Activate, expire, restart and close:** the crash journal ordering is testable within one isolated root. Execution becomes ambiguous when canonical owner/config changes concurrently in the ordinary root, when the new gateway boot epoch must be compared to persisted rows, and when detach/disable must be coordinated between an in-memory gateway object and the profile transaction. The plan names a 30-second tick but does not assign task creation, cancellation, exception handling, or startup-before-attachment ordering to an exact owner.
3. **Export and replay:** adding a strict snapshot and provenance tag is feasible in the named profile modules. The executor still must choose the source root and invent per-source revision tokens for non-append-only documents, then decide how a diagnostic capability authorizes that source without violating the stated production-isolation principle. Fingerprint assertions cannot be wired confidently until those decisions are explicit.

## Missing Evidence

Definitely missing:

1. A closed `DiagnosticIsolationSpec`/configuration schema and authenticated resolution flow for the isolated root/registry, disposable customer, test bot and three destinations, including digest-to-live-value comparison and path containment.
2. A declaration of which Telegram adapter/bot hosts diagnostics and how `test_bot_separate` is proven against the production bot without creating a second unplanned live connection.
3. Boot-epoch generation/ownership and a deterministic ordinary-root/isolated-root authority lock/revalidation protocol.
4. Exact ownership and sequencing for attach/detach, startup recovery, the 30-second expiry task, clean shutdown, and crash-between-detach-and-disable recovery.
5. Snapshot source-root/customer authorization and a per-incident list of exact ledger/document paths, validators, and before/after revision tokens, including mutable non-JSONL documents.
6. Fixed promotion root derivation and the explicit migration-source allowlist.
7. Self-contained verification commands and exact row-delta terminology/matrices for prepared, active, restart-unvalidated, terminal, duplicate, post-reservation, and recovery paths.

Possibly unclear rather than definitely wrong: whether `operator_destination` is Topic 59 or a fourth operational space, and whether “ordinary gateway” means an already-running gateway authenticated as the test bot rather than the production bot. Both materially affect route-disjointness and preflight and should be stated.

## Approval Boundary

Execution may retain the marked-registry design, current-boot TTL session, crash-journal ordering, ordinary-loader and synthetic-provenance rejection, bounded incident classes, non-mutating snapshot intent, manifest-only promotion, and the no-live/no-deployment boundary. Product implementation must not begin from this revision until bootstrap root/bot/spec resolution, cross-root authority, runtime task ownership, snapshot source authority, and exact verification are specified. No live Telegram, credential handling, manual P2–P6, production customer action, or deployment is approved.

## Summary

- **Clarity:** Strong state/security intent; bootstrap root, bot ownership, and snapshot source remain ambiguous.
- **Verifiability:** Exact schemas and crash cases improved; command and row-delta contracts are incomplete.
- **Completeness:** Most prior findings are closed, but the admission data path and cross-root lifecycle are load-bearing gaps.
- **Big Picture:** Hot attachment can satisfy solo diagnostics only if the running adapter is explicitly the test-bot boundary and production evidence access is separately defined.
- **Principle/Option Consistency:** Default-off/no-config-only authority is consistent; digest-only admission currently conflicts with the need to locate the isolated root.
- **Alternatives Depth:** Permanent master and mandatory three accounts are addressed; separate diagnostic adapter/process versus hot attachment is not analyzed at the bot/root boundary.
- **Risk/Verification Rigor:** Strong crash/red-team categories, but insufficient exact ownership, revision fencing and executable commands.

## Required Changes

1. **Close the bootstrap/spec data path.** Define the exact `DiagnosticIsolationSpec` fields and storage/config source, who reads it, how the authenticated Topic-59 action selects exactly one spec without raw identifiers, and how `attach_diagnostic_runtime` receives/resolves the isolated root. State that config supplies pinned data only and cannot activate; require live digest equality, containment, privacy and symlink checks.
2. **Resolve the bot/topology alternative.** State whether the existing adapter is the test-bot gateway and how its identity is pinned against a separately stored production-bot digest. If a second adapter is intended, revise the topology, files and tests. Briefly reject the unchosen separate-process/adapter option on explicit isolation/usability grounds.
3. **Specify cross-root authority and boot identity.** Name the canonical-owner root and isolated root, deterministic lock order or optimistic revision protocol, final append/provider revalidation points, boot-epoch generation and injection, and failure outcomes for concurrent owner/config/spec changes.
4. **Assign runtime lifecycle ownership.** Give exact methods/state containers for keyed child coordinators and route lookup; define collision rejection, attach idempotency, detach-before-disable ordering, startup recovery before any attach, tick task creation/cancellation, and clean shutdown. Add tests for crashes at the gateway/profile handoff.
5. **Finish the snapshot/replay contract.** State the allowed source root/customer relationship to the diagnostic session, how production evidence can or cannot be read, and exact file/API plus revision-token rules for each incident source. Bind `DiagnosticReplayMapping` storage/approval/digest and source-to-replay fingerprint execution to named entry points.
6. **Fix promotion and verification exactness.** Pin the two repository roots by canonical derivation, enumerate migration sources, restore exact cwd/interpreter commands for profile full+compile, gateway focused/full decision+compile, docs/link tests, and no-network enforcement. Define row deltas by ledger class and expected reason/check values per session state.
7. **Make parallel execution ownership safe.** The proposed profile-state and snapshot/replay executors both need the new `diagnostic_isolation.py`; split interfaces/files or sequence those slices so parallel agents do not edit the same load-bearing module.