# Owner-Isolated Diagnostic Mode — Stage 24 Critic Review

Reviewed immutable planner artifact `/home/cube/projects/richard/traning coach/.gjc/_session-019f8455-334a-7000-99ca-318dfd0e06b1/plans/ralplan/019f8455-334a-7000-99ca-318dfd0e06b1/stage-24-planner.md`. The RALPLAN `index.jsonl` records that exact path at planner stage 24 with SHA-256 `ef53cd64f24670de26f2408eaef1bc6f0ce132e4b371e68306e9eef9af3c1965`, matching the assignment. Product files were inspected read-only and no tests were run.

## Verdict
**REJECT**

## Claim Checks

- **The safety intent is sound but not yet executable.** The plan preserves default-off behavior, mode-bound authority, separate destinations, provider-at-most-once behavior, human-only live operations, and the prohibition on a natural-language Telegram diagnostic UI. Those principles fit the canonical profile runbook and the stage-23 production contracts.
- **The proposed active-session state contradicts the current runnable path.** The plan requires diagnostic activation to start with a disabled disposable customer, but current `NutritionCoachingCoordinator._configure_registry` skips disabled customers and `AdaptiveOperatorService._enabled_customer_keys` exposes only enabled entries. The canonical runbook likewise selects an enabled eligible customer, and current `audit_gate_d_preflight` adds `customer_not_enabled` for a disabled customer. No diagnostic-only runtime registry, overlay, or controlled state transition is selected, so an executor cannot make the promised complete P2–P6 workflow run while preserving the stated disabled/zero-production-visibility boundary.
- **Ordinary identity semantics are misstated.** Current `CustomerSpec`/`RegistryDocument` validation in `checkin_cli/customer_coaching.py` rejects equal full Telegram triples and equal chat/topic spaces, not equal `user_id` values. Current `audit_gate_d_preflight` similarly compares full triples and destination pairs, so the same user with three distinct destinations can already pass that particular check. The plan says ordinary mode “preserves” a three-distinct-user rule and simultaneously proposes a unit test that ordinary mode rejects a shared user. That is a new tightening, not preservation. The file containing the authoritative validators, `checkin_cli/customer_coaching.py`, is absent from the target list, and the plan does not say whether ordinary registry loading, activation, preflight, or gateway startup owns the new rule.
- **The gateway insertion points exist, but the capability contract is incomplete.** `AdaptiveOperatorCapability` currently binds one review operator, canonical owner, customer/action and live pins; customer and trainer ingress is address-routed and does not carry that capability. The plan requires mode/session/role/destination pins and revalidation before every lifecycle, reservation and provider operation, but does not define whether this extends the existing operator capability, introduces per-role diagnostic ingress authority, or supplies a session-bound coordinator context. Exact operations and route sources are therefore left to the executor. `telegram.py` already reserves Topic 59 across every update kind, so that invariant can be reused, but the three diagnostic role destinations and their relationship to the review operator/canonical owner are not specified.
- **Restart failure is not represented by the proposed state machine.** The listed states are `prepared|active|expired|revoked|closed`, yet restart is supposed to make an active session unavailable until explicit owner revalidation. There is no suspended/unvalidated state, no transition table, no named prepare/activate/revalidate/revoke/close APIs, no numeric maximum TTL, no idempotency/concurrency rule, and no definition of how a boot epoch is generated and compared across profile and gateway processes. A persisted boot value that is not advanced safely would not provide the claimed restart fence.
- **The snapshot “read-only under the profile authority lock” claim conflicts with current code.** `profile_authority_lock` creates/chmods `data`, opens `.adaptive-authority.lock` with `a+`, and chmods the lock file. Calling it can mutate the profile. Existing `audit_gate_d_preflight` deliberately avoids that lock and has a byte-for-byte filesystem read-only test. The plan does not select a non-mutating shared-lock/read protocol or require a pre-existing lock, so its snapshot read-only acceptance criterion cannot be implemented as written.
- **Incident reproduction is not defined at a representative level.** The proposed snapshot contains event/state sequences, counts, digests and booleans, but no selected bounded inputs or temporal/concurrency facts sufficient to deterministically reproduce incomplete check-in, low-adherence, safety-hold, KST rollover, authority mutation, duplicate callback, timeout/audit-pending, or reservation-race behavior. `scope` and `mapping` are untyped; supported incident classes, source ledgers, snapshot staleness, KST rebasing, expected reproduction fingerprint, and equivalence criteria are absent. Counts and digests alone cannot prove that replay recreated the incident rather than only a superficially similar ledger shape.
- **Snapshot authorization and isolation need exact contracts.** `requested_by` is present but is not required to equal the freshly loaded canonical owner. The plan does not specify whether snapshot/incident IDs are caller-supplied or generated, where the separately approved mapping is stored, what digest binds it, or how raw diagnostic destinations are resolved from stored digests. It also does not state how production loaders reject synthetic replay state by provenance/schema rather than convention.
- **Promotion scope is unresolved and potentially unsafe.** “Repository code paths, approved schema migrations and explicit config changes” spans the profile workspace and the separate `hermes-agent` repository, but trust roots, allowed file/config fields, symlink handling, and secret/destination exclusions are not enumerated. A generic config allowance conflicts with the requirement that credentials and destinations can never be promoted. The plan also leaves the snapshot/replay/promotion module target as “prefer an existing module,” even though no existing snapshot/replay/promotion module was found; executors would have to choose both topology and API location.
- **The existing verification base is strong but the new acceptance matrix is too high-level.** Current focused behavior lives especially in `tests/gateway/test_adaptive_nutrition.py`, with ingress reservation in `test_telegram_group_gating.py` and profile preflight in `tests/test_customer_admin.py`. The plan does not name the first file in its gateway targets, define exact ordinary/diagnostic receipt schemas and reason codes, or give row/provider expectations for pre-session, expired, restart-unvalidated, wrong-role and production-injection paths. “Snapshot export is byte-for-byte read-only” also needs a concrete before/after filesystem assertion while accounting for the lock issue.
- **The incident reporting boundary is only implicit.** The intent reconciliation prohibits a new Telegram natural-language diagnostic UI, but the observability section asks for health checks without naming their output surface. This must be explicitly limited to typed returns/local development logs or metrics; otherwise an executor could add Telegram diagnostic messaging contrary to the assignment.
- **Referenced existing files were verified.** `customer_admin.py`, `customer_coaching.py`, `__init__.py`, profile `tests/test_customer_admin.py`, canonical profile `PILOT_RUNBOOK.md`, gateway `nutrition_coaching_config.py`, `nutrition_coaching.py`, `telegram.py`, and gateway tests `test_adaptive_nutrition.py`, `test_nutrition_coaching.py`, and `test_telegram_group_gating.py` exist. The proposed diagnostic session, snapshot, replay and promotion APIs/modules do not yet exist.

### Representative implementation simulation

1. **Activate a solo diagnostic session:** profile preparation can record the shared owner and three distinct spaces, but keeping the customer disabled means gateway registry construction creates no customer/trainer routes and the operator menu has zero eligible customers. Enabling through the ordinary activation path makes it an ordinary runtime entry and does not establish the proposed diagnostic-only visibility. The plan provides no safe bridge between those states.
2. **Mint and use a diagnostic send capability:** extending `AdaptiveOperatorCapability` with mode/session/role pins is straightforward for Topic-59 operator actions, but customer/trainer ingress never presents that capability. An executor must invent a new ambient coordinator/session context and decide every validation point. Without an exact runtime/session API, a missed path could become the production bypass this change is meant to prevent.
3. **Export and replay an incident:** reading while holding `profile_authority_lock` may create or chmod the lock artifact, violating read-only. Even after export, a sequence/count-only snapshot cannot reconstruct the bounded data or injected timing that chose a branch; there is no specified assertion connecting source incident fingerprint to replay outcome. The future incident-reproduction goal is therefore unproven.

## Missing Evidence

Definitely missing:

1. A coherent disabled/prepared/active/restart-unvalidated state model that is compatible with the current enabled-only runtime and preflight.
2. Exact typed APIs, ledger/config schemas, transition rules, TTL maximum, boot-epoch source, route source and file ownership for diagnostic sessions.
3. A decided ordinary identity rule and all authority/runtime callsites that enforce it without admitting same-user production routing.
4. A non-mutating synchronization protocol for snapshot export.
5. A bounded incident schema with supported incident classes and observable source-to-replay equivalence criteria.
6. Exact promotion trust roots and per-field allowlists.
7. Named tests and exact expected receipt keys/reason codes, rows and provider counts for every new state and cross-mode injection.

Possibly unclear rather than definitely wrong: whether “three roles” means review operator/customer/trainer or canonical owner/customer/trainer, and whether the diagnostic gateway is a separate process/profile instance or a mode branch in the production gateway. That topology decision materially affects isolation and must be selected.

## Approval Boundary

The high-level principles, rejected permanent-master option, reuse of current Topic-59 first-match behavior, human-only live operations, development-session-only issue reporting, and synthetic/de-identified incident direction may be retained. Product execution must not begin from this artifact because the authority/runtime topology, state transitions, replay sufficiency and read-only mechanism remain unresolved. No production or diagnostic capability, registry relaxation, snapshot exporter, replay writer, promotion builder, Telegram UI, live Telegram operation, credential action, manual P2–P6 step, or deployment is approved.

## Summary

- **Clarity:** Strong principles; critical runtime/session mechanics are ambiguous.
- **Verifiability:** Existing production tests are strong; new exact outcomes and source-to-replay proof are missing.
- **Completeness:** Fails on disabled-to-runnable flow, restart semantics, read-only locking, replay inputs and promotion trust roots.
- **Big Picture:** The goal is appropriate, but the chosen topology does not yet prove both solo rehearsal and future incident reproduction without a production bypass.
- **Principle/Option Consistency:** Default-off and least privilege are consistent; disabled activation versus enabled-only execution and “preserved” versus newly tightened identity semantics are not.
- **Alternatives Depth:** Permanent master and three-account-only options are covered, but separate isolated gateway/profile versus in-process diagnostic overlay—the material implementation alternatives—are not compared.
- **Risk/Verification Rigor:** Good pre-mortem categories, but several preventions lack executable mechanisms and exact acceptance tests.

## Required Changes

1. **Select and specify the isolation topology.** Compare and choose between a separate diagnostic gateway/profile runtime and a session-bound in-process overlay. State exactly which process/config can load diagnostic routes and prove that ordinary production startup cannot load them.
2. **Resolve the customer-state contradiction.** Provide a state table for disabled preparation, diagnostic activation, lifecycle delivery enable/revoke, expiry/restart suspension, close and cleanup. Define how a diagnostic customer becomes routable without appearing as an ordinary enabled production customer, and update preflight expectations for each state.
3. **Define the complete session API and schema.** Name prepare/activate/revalidate/revoke/close/load functions, ledger path, strict transition table, row fields, idempotency, predecessor/digest validation, concurrent activation winner, numeric TTL cap, current-time behavior and process boot identity. Include a restart-unvalidated state or an equivalent explicit rule with tests.
4. **Correct identity semantics and scope.** Explicitly decide whether ordinary mode now requires pairwise-distinct `user_id` values. If yes, identify `checkin_cli/customer_coaching.py` and all activation/runtime/gateway validators as targets, document the compatibility impact, and test that same-user/different-space ordinary startup fails while the exact owner succeeds only under a live diagnostic session.
5. **Define role/destination authority.** Specify the three role names, their raw-address source, how digest-to-live-route comparison works, whether capabilities are operator-only or per-role, and every ingress/lifecycle/reservation/provider callsite that must revalidate mode, session, role, destination, customer, bot and profile pins. Require production capabilities at diagnostic gateways and diagnostic capabilities at production lifecycle APIs to fail with zero rows/provider calls.
6. **Replace the mutating read-lock assumption.** Select a read protocol that cannot create/chmod/write any profile object, or make a pre-existing immutable lock an explicit prerequisite. Add a before/after tree test covering files, modes and bytes and assert export appends zero operational rows.
7. **Make incident reproduction concrete.** Enumerate supported incident classes and source ledgers; define strict `scope`, snapshot and approved-mapping types; include only the bounded enum/numeric/relative-time facts needed to reproduce each class; define staleness and KST rebasing; and require the isolated replay to yield the same bounded incident fingerprint, ordered states, reason codes and provider-attempt booleans. Unsupported incidents must reject rather than produce an approximate replay.
8. **Specify replay anti-promotion/anti-production guards.** Choose the module/file, schema marker and provenance rules; require ordinary production loaders to reject synthetic replay state; define isolated-root ownership/symlink/path checks and mapping approval authentication/digest; and prove no production capability, ledger, receipt, destination or customer identifier is accepted.
9. **Bound or defer the promotion manifest.** Either provide exact trusted repository roots, allowed paths/extensions/config fields, symlink rules and explicit secret/destination/runtime exclusions for both codebases, or move promotion manifest work to a separately approved plan. Generic config inclusion is not acceptable.
10. **Write exact acceptance and verification matrices.** Name `tests/gateway/test_adaptive_nutrition.py`, `test_nutrition_coaching.py`, `test_telegram_group_gating.py`, profile `tests/test_customer_admin.py`, and any chosen new module tests. For ordinary, prepared, active, expired, restart-unvalidated, revoked and closed states, specify receipt keys/reason codes, route result, row deltas and provider counts. Include all ingress kinds, cross-mode capability injection, production route/bot/customer mutation, concurrency, cleanup and snapshot/replay property tests.
11. **Make reporting containment explicit.** Health/violation reporting may use typed API results, local logs/metrics and the development session only. It must never add a Telegram natural-language diagnostic command, menu, issue report or automatic live operation.