# Architect Re-review — Upstream dual-coach port (pass 2)

Reviewed immutable revision:
`/home/cube/projects/richard/traning coach/.gjc/_session-019fadf6-bb8b-7000-a763-8db4b0d86bc6/plans/ralplan/019fadf6-bb8b-7000-a763-8db4b0d86bc6/stage-02-revision.md`

Revision SHA-256: `dd5abdf14cee0d7c5bc255da844fbde5035650a1b1754225f63521ba34b1eff6`

Pass-2 ratchet applied: review is limited to the revised resolution of the prior Critic blockers. Prior Architect status was `CLEAR` / `APPROVE`; the blocking issues below are permitted by the ratchet because they are unresolved prior verification/offline-contract blockers whose concrete failure is exposed by the revision's newly introduced A/B topology, config, and command contracts.

## Summary
The revision closes the topology-decision, ownership-ledger, worktree-collision, rollback, pre-mortem, and provenance blockers with appropriately explicit human gates. It does not yet close the offline-candidate and exact-verification blockers: the all-disabled config cannot load the standalone plugin, and the commands conflate the Hermes host worktree with the separately distributed plugin artifact.

One focused revision is required. Preserve the current human gates and safety invariants, add the plugin discovery opt-in, and split artifacts, SHAs, commands, and PR outcomes by selected topology.

## Claims
- The A/B/C placement decision is now explicit. A is the default external plugin; B is limited to generic reusable hooks and a public allowlist; C is rejected absent a written two-human exception. The `invalid` label is a hard pre-implementation GO/no-GO premise gate.
- The revised hunk ledger is a signed prerequisite rather than executor discretion. It accounts for the source file families, establishes replace/omit defaults, names target owners/config/tests, enumerates ingress ordering, and separates generic cron claims from the domain reservation ledger.
- The worktree procedure now uses `git -C` from a neutral context, stops on path/ref/worktree collision, avoids `fetch --prune`, fingerprints the original dirty worktree, and limits cleanup to recorded clean worktrees created by the run.
- The rollback matrix preserves #74072 and forbids executor merge, delete, force-update, or force-push actions across pre-push, publication, CI/review failure, wrong-head, accepted, and supersession states.
- Target evidence confirms standalone/user/entry-point plugins are opt-in through the separate `plugins.enabled` allow-list (`/home/cube/projects/richard/hermes-agent/hermes_cli/plugins.py`, `_get_enabled_plugins` and `PluginManager.discover_and_load`). The revision's `plugins.dual_coach.enabled: false` is only a behavioral flag and does not satisfy host discovery.
- Git evidence at `41a07f5b8` confirms `tests/gateway/test_adaptive_nutrition.py` is absent while `tests/gateway/test_telegram_group_gating.py` exists. The revision nevertheless requires all listed commands to run in the Hermes port worktree, although A adds no Hermes source and its private plugin owns the replacement tests and wheel.

## Analysis
### Spec compliance
The safety and maintainership shape is materially improved. The owner cannot accidentally authorize a bundled Profile-specific core port: implementation is blocked until the invalid-label maintainer and architecture maintainer select A/B/C and approve the public allowlist. The ledger, ingress-first reservation, separate identity authorities, no-resend ambiguity policy, and human-only Gate-D boundary remain aligned with the requested semantic port.

The concrete candidate contract is not executable as written. Hermes treats `plugins.enabled` as a list of manifest keys used to decide whether a standalone or pip entry-point plugin is loaded. A nested `plugins.dual_coach.enabled: false` does not register or load the plugin. Therefore `hermes dual-coach preflight` cannot be discovered under the supplied all-false configuration, even though the plan says preflight remains available while behavior is disabled.

### Architecture
The topology decision correctly separates host-generic capability from private domain behavior, but the delivery model later collapses the two repositories/artifacts back into a single port worktree and `CANDIDATE_SHA`. Under A, the file-level contract says there is no Hermes source change. There is consequently no substantive replacement Hermes PR, the legacy source test paths are absent from base `41a07f5b8`, and `python -m build` in the Hermes worktree builds `hermes-agent`, not the private dual-coach plugin. Under B, the generic Hermes hook and private plugin are independent deliverables with independent revisions, tests, builds, and rollback identities; one SHA cannot prove both.

This is not a demand for additional product scope. It is the minimum wiring needed to make the newly selected architecture and the prior Critic's exact-verification/offline-candidate requirements agree.

### Constructive synthesis
Use topology-specific evidence manifests:

- **A:** record `HERMES_BASE=41a07f5b8` as the compatibility host and a separate immutable `PLUGIN_SHA`; build/test the plugin from its named checkout/package root; install the host plus the pinned plugin wheel into the smoke environment; record that there is no Hermes replacement PR and require the named humans to decide how #74072 is dispositioned without implying an upstream replacement.
- **B:** record `HERMES_HOOK_SHA` and `PLUGIN_SHA` separately; run `scripts/run_tests.sh` in the Hermes hook worktree only against actual generic-hook/current regression paths plus the host suite; run the plugin's named tests/build in its own checkout; install both reviewed artifacts for E2E. The replacement PR contains only the generic host allowlist.
- In both cases, add `plugins.enabled: [dual_coach]` (using the exact `plugin.yaml` registry key) while retaining every domain behavior flag as false. The E2E must prove CLI registration and preflight with that exact temp-`HERMES_HOME` config.

### Code quality, security, and performance
No new code-quality, security, or performance blocker was found in the reviewed delta. The fail-closed ordering, redaction assertions, zero-network/write-set proof, immutable body pins, and recording transports are appropriate once the actual plugin is loaded and the correct artifacts are tested.

## Root Cause
The revision correctly separated product topology but did not carry that separation through configuration, build ownership, verification context, immutable identity, and PR semantics. It treats a plugin behavioral `enabled` flag as host discovery authorization and treats a two-artifact architecture as one Hermes worktree/SHA.

## Findings
### HIGH — Add the host plugin opt-in key to the all-disabled config
**Reference:** revision lines 115–129; target `hermes_cli/plugins.py` `_get_enabled_plugins` / `discover_and_load`.

**Impact:** the plugin is skipped, its top-level CLI cannot register, and the mandatory preflight cannot execute.

**Fix:** add the exact manifest key to the host `plugins.enabled` allow-list while keeping `plugins.dual_coach.enabled` and every behavioral flag false. Verify discovery, CLI registration, and preflight with the exact isolated config.

### HIGH — Split deliverables and verification by selected topology
**Reference:** revision lines 53–55, 84–87, and 131–146; Git evidence for base `41a07f5b8` test paths.

**Impact:** default A cannot produce the stated Hermes replacement PR or plugin wheel from the mandated working directory; its required legacy tests do not exist on the base. B cannot prove or roll back two artifacts with one `CANDIDATE_SHA`.

**Fix:** define per-topology repository roots, SHAs, test commands, build commands, manifests, E2E install inputs, and PR disposition. Run the Hermes wrapper only for actual host changes/regressions and the plugin's own runner/build from its package root.

## Recommendations
1. Add `plugins.enabled: [dual_coach]` using the final manifest key; retain every domain capability flag as false.
2. Branch the candidate/verification/acceptance/rollback sections explicitly for A and B. Name both checkout roots and immutable artifact identities.
3. For A, remove the requirement for a contentless Hermes replacement PR and record the human disposition of #74072. For B, scope the replacement PR and `scripts/run_tests.sh` commands to generic Hermes hooks/tests only.
4. Replace legacy source-test command paths with the approved plugin test paths, while retaining `tests/gateway/test_telegram_group_gating.py`, relevant current host regressions, and the full Hermes wrapper run where B changes Hermes.
5. Build the private wheel from its named package root; install it together with the pinned host in the temp-`HERMES_HOME` E2E; record `HERMES_HOOK_SHA`/base, `PLUGIN_SHA`, wheel SHA-256, and report SHA-256 distinctly.
6. Preserve all other revised gates and boundaries unchanged.

## Architectural Status
`BLOCK`

## Code Review Recommendation
`REQUEST CHANGES`

## Tradeoffs
| Topology | Hermes deliverable | Plugin deliverable | Verification / PR consequence |
|---|---|---|---|
| A — existing surfaces | No host diff; compatibility base `41a07f5b8` | Pinned private plugin wheel + tests | No substantive Hermes replacement PR; human disposition record for #74072; host+plugin install smoke |
| B — generic hooks | Pinned generic-hook SHA and focused/full host wrapper tests | Separate pinned private plugin wheel + tests | Replacement PR only for public allowlist; two-artifact E2E and rollback evidence |
| C — bundled behavior | Prohibited absent existing exception gate | N/A or separately approved | No change to current no-go decision |