acdream/docs/research/2026-08-04-c4-route-3-architecture-review-round2.md

# C4 route 3 — architecture / adversarial review, ROUND 2 (delta) — 2026-08-05

**Verdict: FAIL.**

Reviewed: the uncommitted working tree at HEAD **`cd3129e9`**, +2,570/-232 across
16 files. Round-1 report:
[`2026-08-04-c4-route-3-architecture-review.md`](2026-08-04-c4-route-3-architecture-review.md).
`dotnet build AcDream.slnx -c Debug` exits 0.

**Round-1 findings closed: A4, A6, A9, A10(a).** A1, A2, A3 were addressed with
real design work that is directionally right — the A1 fix in particular
(inverting the readiness feed instead of touching the sequencer) is the correct
architectural answer to a hard constraint, and I want that stated plainly.

**The FAIL is one defect, present symmetrically on both hosts, introduced by
the A1/A3 fixes themselves:** both new "am I committed yet?" gates infer
*commit* from *the drive controller's global pending slot being empty*. That
slot empties on at least three paths that do **not** commit — including the one
the drive's own doc comment names as the *expected* outcome of a park. When it
does, the graphical controller latches `_placementCommitted = true` and the
headless projection reports `IsCollisionReady: true`, and both hosts then march
the full completion sequence against an unmoved body. That is round-1's A1/A3
restored, and on the graphical side it is now *worse*, because
`AcknowledgePortalMaterialized` succeeds where it previously failed its
invariant.

Numbering continues as **B*n*** to avoid collision with round 1.

---

## MAJOR — the FAIL

### B1 — `PendingCount == 0` is not "committed"; both hosts infer commit from a signal that is also set by three non-committing paths

**Severity: MAJOR (FAIL basis). Both hosts. Uncovered by any test.**

Graphical, `src/AcDream.App/Streaming/LocalPlayerTeleportController.cs:646-658`:

```csharp
if (_awaitingDeferredWake)
{
    if (_acceptedPositionDrive.PendingCount != 0)
        return false;
    _awaitingDeferredWake = false;
    _placementCommitted = true;      // <-- infers commit from "not pending"
    return true;
}
```

Headless, `src/AcDream.Headless/Hosting/HeadlessSessionWorldProjection.cs:816-821`:

```csharp
if (_awaitingPortalWake)
{
    committed = _acceptedPositionDrive.PendingCount == 0;
    if (committed)
        _awaitingPortalWake = false;
}
```

`PendingCount` is
`RuntimeAcceptedPositionDriveController.cs:330` — `_pending is null ? 0 : 1`.
It is (a) **global**, not portal-scoped, and (b) cleared by every terminal path,
committing or not. `Advance()` clears `_pending` at five sites; three of them
run **without** a portal commit:

| site | condition | committed? |
|---|---|---|
| `:975` | *"The watch died — most likely a subsequent accepted Position's merge-time `Forget`"* | **no** |
| `:920` | A2's new abandon-at-wake (`!IsPortalAuthorityCurrent`) | body moved by `RetryDeferred`, **suffix skipped** |
| `:998` | A2's new abandon at the prepare-retry branch (`CancelToken`) | **no — nothing ever placed** |

**The `:975` path is the modal case, not a corner case.** The method's own
doc comment (`:880-892`) says it verbatim:

> `RuntimeEntityObjectLifetime.TryApplyPosition` calls `Forget` on EVERY
> accepted Position for this entity … ACE broadcasts at 5-10 Hz, so a
> `DeferredCell` park surviving past one broadcast interval is cancelled
> before its collision generation can ever commit it — **the exact
> far-destination case the park exists to serve**.

So: park → within ~100-200 ms an ordinary broadcast `Forget`s it → `Advance`
clears `_pending` → `PendingCount == 0` → both gates declare success.

**Concrete failure scenario (graphical).** Portal to a landblock whose
collision generation has not committed. `TryExecuteAcceptedPortalArrival`
returns `DeferredCell`; `_awaitingDeferredWake = true`. One ACE broadcast
later the park is Forgotten and `_pending` clears. Next `Tick`:
`_placementCommitted = true` → `placementReady = true` → the sequencer leaves
`Tunnel` and fires `Place` → the `if (!_placementCommitted) return;` guard at
`:557` **passes** → `_placement.Place(_pendingRotation)` writes the render
entity from the *unmoved* `controller.Position` and rebuckets to the *source*
cell → `ObserveMaterialized(_pendingRevealGeneration, sequence, _pendingCell)`
**succeeds** (the reveal is still active and current) → `PlayExitSound` reveals
the world viewport → `FireLoginComplete` sends LoginComplete, and
`_transit.Complete(generation)` now **passes** its
`portal-complete-before-materialized` check because materialization was falsely
acknowledged. The player is released into the world standing at the
pre-teleport position, the transit reports a clean completion, and nothing logs
an invariant failure. Round 1's A1 at least tripped
`FailInvariant("portal-complete-before-materialized")`; this does not.

**Concrete failure scenario (headless).** Identical shape:
`PrepareDestination` returns `IsCollisionReady: true`, so
`RuntimeLiveEntitySessionController.TryAdvancePortalCompletion` runs
`AcknowledgeDestinationReadiness` → `AcknowledgePortalMaterialized` →
`Complete` → `TerminalProjected` → `LoginComplete` → `EndTeleport`, and sets
`controller.State = PlayerState.InWorld`, all with the body unmoved. That is
round-1 A3 verbatim.

**Secondary hazard from the same root:** because the slot is global, a portal
park killed by a merge-time `Forget` can be immediately replaced by the force
arm's own `RetainPending` from that same merge
(`TryExecuteAcceptedLocalPosition` → `RetainPending`). The portal gate then
polls a **ForcePosition** operation, waits for it, and latches "portal
committed" when the force operation settles.

**Why no test caught it.** Both new park tests
(`PortalDeferredCell_ParksThenCommitsExactlyOnceOnTheCollisionGenerationWake`,
`HeadlessPortalPrepareDestinationParksThenCommitsOnCollisionGenerationWake`)
commit the destination's collision generation so the park resolves by
committing. `PortalDeferredCell_WakeAbandonsInsteadOfReconcilingWhenAuthorityWentStale`
asserts the **drive's** behaviour (`Assert.Empty(gameActions)`) and stops
there — it never asks what the *host gate* concludes from the resulting
`PendingCount == 0`. The seam between "the drive retired the park without
committing" and "the host decides the portal is placed" is exactly where the
defect lives and is exactly what no test crosses.

**Fix direction.** Stop inferring. The drive already knows the answer with
certainty — `ReconcileAndAcknowledgePortal` runs on commit and only on commit.
Publish that fact:

- add a portal-commit observable to `RuntimeAcceptedPositionDriveController`
  — e.g. `bool TryConsumePortalCommit(long revealGeneration, ushort teleportSequence)`
  latched in `ReconcileAndAcknowledgePortal` and cleared on consumption, or an
  `Action<RuntimePortalPlacementAuthority>` commit callback supplied at
  construction alongside `isPortalAuthorityCurrent`;
- have both host gates consume **that**, keyed on the reveal
  generation/sequence they are waiting for, so a force pending, a Forgotten
  park, and an abandoned park are all correctly "still not committed";
- give the park a terminal "abandoned" outcome the host can see, so it can
  either re-attempt cleanly (`_awaitingDeferredWake = false` and try Begin
  again next tick) or converge through the existing transit cancellation
  rather than silently succeeding.

**Required tests (both must fail against the current code):**
1. Graphical: park, then kill the park with a merge-time `Forget` (an ordinary
   accepted Position — the fixture's `OfferDestination` already performs the
   merge) instead of committing the collision generation; drive 100 ticks;
   assert `Placement.Called == false`, `Movement.Controller.Position` unchanged,
   `Reveal.PortalMaterializationCount == 0`, `Session.LoginCompleteCount == 0`,
   `Controller.IsActive == true`.
2. Headless: same, asserting `PrepareDestination` keeps returning
   `IsCollisionReady: false` and `controller.State` stays `PortalSpace`.
3. Graphical: park, then let the force arm take the pending slot; assert the
   portal gate does not latch when the *force* operation settles.

---

## MEDIUM

### B2 — A2's re-validation does not close the FIFO wedge it was written for; it only narrows reachability

The implementer's own note is accurate and I confirm it: `RetryDeferred` →
`CommitCanonical` publishes the `Place` receipt asynchronously, and
`IsPortalAuthorityCurrent` runs only afterwards, inside `Advance`, gating the
*suffix*. But the wedge was never in the suffix — it is in the receipt:

- `Advance:920`'s abandon branch is reached only **after**
  `TryPeekAcknowledgedPlacement` succeeded, i.e. the sink **already accepted**
  the receipt. On that path there was never a wedge to prevent.
- The wedge path is the one where the sink **refuses**:
  `RuntimePlacementPresentationSink.TryApply` →
  `RuntimeWorldTransitState.IsCurrentPlacementAuthority` false → return false →
  `RuntimePlacementProjectionSubscription.OnPlacement` leaves it at the FIFO
  head → every later placement receipt for every entity is blocked and the
  drive's pending never converges. `IsPortalAuthorityCurrent` never runs on
  that path, because `TryPeekAcknowledgedPlacement` never yields.

What the A1 fix *does* buy is reachability: with `placementReady` false during
a park, the sequencer cannot reach `FireLoginComplete`, so the transit no
longer ends underneath an outstanding park in the ordinary flow. The remaining
entries are mid-transit supersession (a second F751 while parked →
`OnTeleportStarted` → `ResetTransit` → `EndTeleport`), `ResetSession`, and
`ResetGenerationPresentation` — i.e. exactly the §9 "honest gap" cases.

That is a genuine narrowing and I credit it. It is not closure, and the
consequence remains unbounded (whole-FIFO stall + non-convergent shutdown), so
contract stop condition 4 ("P3 finds a portal receipt no mechanism can consume
or retire — the FIFO-wedge shape changes the design, not the test") still
applies.

**Fix direction.** The retire path has to exist at the receipt, not the suffix.
Either (a) let the sinks treat a `Place` whose entity/versions are current but
whose portal authority is dead as acknowledge-and-ignore (the same shape
`Discard`/`ExecutorCompleted`/`WithdrawalRestored` already use, and for the
same stated reason — refusing wedges the ordered stream), or (b) have
`CommitCanonical` drop a portal suffix it can already see is not current rather
than publishing a receipt nothing can consume. (a) is smaller and matches the
existing precedent in both sinks' own doc comments.

### B3 — Headless `_awaitingPortalWake` is not reset across teleports within one session

`HeadlessSessionWorldProjection.cs:762`. The graphical twin
(`_awaitingDeferredWake`) is cleared in `ResetTransit:932`, which
`OnTeleportStarted` calls — clean. The headless field has no equivalent: it is
cleared only when the poll declares success, and `HeadlessSessionWorldProjection`
is constructed per *session*, not per teleport.

**Scenario.** Teleport 1 parks → `_awaitingPortalWake = true`. Before the next
`PumpPortalCompletion`, a new F751 arrives; `TryCompletePortal` overwrites
`_pendingPortalCompletion` with reveal 2. The next pump calls
`PrepareDestination(reveal 2)`, which takes the **poll** branch left over from
reveal 1 — so reveal 2's placement is never even attempted, and if
`PendingCount` happens to be 0 it is immediately declared committed. This is
B1's inference bug plus a stale latch, on a path that does not require a
Forgotten park.

**Fix direction.** Key the latch to the reveal generation (or clear it in
`BeginTeleport`, which already runs per teleport on this host).

### B4 — Both hosts' retry loops are unbounded and undiagnosable

A refusal loop (`Contention`, stale reveal, or a park that never converges)
now retries forever with no timeout and no terminal path.

- **Graphical**: the player holds in the tunnel with the retail wait cue after
  5 s. This is a *modelled* end state — AD-2 already documents "predicate never
  satisfies → portal transit remains in the authored tunnel and presents the
  centered wait cue" — so an infinite stall is strictly better than round 1's
  silent release, and I do not consider it blocking. Two gaps, though: AD-2
  attributes that state to streaming/DAT failure only, and now a *placement*
  refusal produces the identical user-visible state; and `_holdSeconds` /
  `ObserveWait` are now driven by `!placementReady` rather than `!dataReady`,
  which is a real semantic change to the wait cue's meaning. The
  `[tp-probe] REFUSED cause=…` line does distinguish them in the log — good —
  but AD-2 should say so.
- **Headless**: worse, because there is no cue and no bound. A bot whose
  destination collision never becomes resident sits in `PlayerState.PortalSpace`
  indefinitely, connected and healthy-looking; `PumpPortalCompletion` is
  entirely silent. Under K4's 30-session envelope this is an invisible stuck
  session. At minimum emit one probe line on the first N retries; ideally bound
  the wait and fail loudly.

### B5 — §8 items 8/9/10 (the committed-receipt presentation suite) — **does NOT block**, with conditions

Answering the coordinator's direct question.

**Does not block route 3.** Reasons, in order of weight:

1. The render-entity half now has a *proven mechanism*, not a claim.
   `LiveEntityRuntime.TryApplyRuntimePlacementPlace:1394-1402` performs
   `entity.SetPosition(projection.WorldPosition)`, `entity.Rotation = …`,
   `entity.ParentCellId = token.ExactCellId` and `RebucketLiveEntity`,
   synchronously inside `CommitCanonical`, before `TryPublishPlace` snapshots.
   That path is shared with route 2's force arm and C3c's first entry and has
   existing coverage. Round 2 correctly rewrote the class doc to say so
   (round-1 A10(a) closed).
2. The restored `Movement.Controller.Position`/`CellId` assertions prove the
   canonical body resolved the offered destination — which is the half route 3
   actually changed ownership of.
3. The shadow half is a **pre-existing route-2 defect**, not a route-3
   regression: `LocalPlayerShadowState.Set` (written by the sink) updates only
   the dedup cache, never `PhysicsEngine.ShadowObjects`, while
   `LocalPlayerShadowSynchronizer.SyncPose` dedups against that same cache. It
   self-heals on the player's first >1 cm move. Route 3 widens the window (via
   `LocalPlayerProjectionController.Project:102`'s PortalSpace early return) but
   does not change its kind.

**Conditions — all three, or it does block:**

- It is recorded as an **open issue with a number** (#312's layer / route 2's
  B2 gap, now two campaigns old) and carried explicitly into C5's parity-test
  scope. Not a comment; a tracked item.
- The connected gate is **not** scored as covering it. The probe fields
  (`leash`, `autorun`, `hookTail`) say nothing about the shadow; there is no
  visual for it. If the user's session passes, the shadow claim remains
  test-verified-nowhere and must be reported that way (the §9 "honest gap"
  discipline).
- The register gets one line under AD-2 or a sibling row naming the
  cache-without-publish asymmetry, so the next reader does not assume
  `LocalPlayerShadowState.Current` means "published".

**Concretely, what the test needs** (in
`tests/AcDream.App.Tests/World/RuntimePlacementPresentationSinkTests.cs`, whose
fixture already owns `BeginPortal` and a `LocalShadow`):

1. **Committed local-player portal Place through the real sink.** Pre-seed the
   render `WorldEntity` at a *different* (wire) pose — this is also §8 item 9's
   T8-ordering half. Drive a `Place` receipt for the local player carrying a
   VALID portal authority. Assert, after: `entity.Position`/`Rotation`/
   `ParentCellId` equal the receipt's `WorldPosition`/`Orientation`/
   `ExactCellId` (the wire pose did not survive); the spatial bucket moved to
   the destination landblock; `LocalShadow.Current` equals the resolved pose;
   **and `PhysicsEngine.ShadowObjects` actually holds a row for the player at
   the destination cell** — that last assertion is the one that discriminates
   cache-only from published, and is the whole point.
2. **Discrimination half.** The same receipt with a stale/superseded portal
   authority must be refused **and then retired** — not left at the FIFO head.
   This is also B2's regression test.
3. **Refused-Place presentation (§8 item 10).** Already partly covered by
   `RefusedPlace_HoldsTheStreamAndConvergesOnlyAfterContentionClears`; extend it
   to assert the pre-teleport pose is still the *presented* pose (entity +
   world snapshot store), not only that the body is unmoved.
4. **Sabotages that must fail:** remove the sink's `entity.SetPosition` → (1)
   fails; leave the shadow write as cache-only → (1)'s `ShadowObjects`
   assertion fails; make the stale-authority receipt return `false` forever →
   (2) fails.

---

## MINOR

### B6 — `isPortalAuthorityCurrent` should be a required constructor parameter

The coordinator's specific concern, checked: **both production sites wire it** —
`SessionPlayerComposition.cs:597-603` and `HeadlessSessionHost.cs:665-671`,
both to `RuntimeWorldTransitState.CanPlacePortalDestination`. Four test sites
do not, which is fine.

The residual is that a null default silently restores the round-1 defect, and
nothing catches a future production site that forgets it — there is no
architecture guard for this the way
`RuntimePhysicsOwnershipTests.ProductionHostsUseSharedPlacementSubscriptionWithoutDirectChannel`
guards the placement channel. With only 2 production + 4 test constructions,
making the parameter required (tests pass `static _ => true`) converts a silent
regression into a compile error for ~6 lines of churn. Same argument will apply
to B1's commit observable.

### B7 — `_placementCommitted` is checked once and never re-validated

`LocalPlayerTeleportController.cs:557`. Round 1's Place handler ran
`CanPlacePortalDestination` immediately before mutating; round 2 moved that
check into `TryAdvancePortalCommit`'s **non-deferred** branch only
(`:668-676`). Once `_placementCommitted` latches, the only guard before
`_placement.Place()` / `ObserveMaterialized` is `IsCurrentLifetime`. If the
reveal is cancelled between the commit and the Place event,
`ObserveMaterialized` refuses (`IsCurrentPortalDestination`) but the
presentation suffix has already run and the stream continues to
`FireLoginComplete` with an unmaterialized reveal. Much less severe than B1 —
the body genuinely is at the destination — but it is the same family. Cheap
fix: keep the `CanPlacePortalDestination` re-check in the Place handler
alongside `_placementCommitted`.

### B8 — A8 (round 1) confirmed still open and confirmed non-blocking

Five portal tests still call `ConvergePortalHost` as the last statement of the
test body rather than in a `finally`, so an assertion failure is still masked
by a `Dispose()` throw during unwinding. Correctly flagged rather than silently
dropped. Test hygiene only — no production effect. It does mean that when B1's
new tests are written, a genuine failure may again present as a teardown throw;
fixing the `try/finally` first would save that debugging round.

### B9 — round-1 A11/A12 unchanged

`PhysicsDiagnostics.LocalTeleportHostKind` process-global (accepted);
`AddSyntheticIndoorCell` geometry-free (accepted — and less load-bearing now
that A6's position assertions are restored).

---

## Closed since round 1

| round-1 finding | status | evidence |
|---|---|---|
| **A4** — route facts unread, inversions hardcoded | **CLOSED** | `RuntimeAuthoritativePositionRoute.RunsTeleportHook:164` / `.ConstrainAfterRouting:170`; `ReconcileAndAcknowledgePortal` reads both; `CommitCanonicalTeleportFrame(bool zeroVelocity, bool rearmConstraintLeash)` branches on them. `ConstrainPhase.None` sabotage now fails `PortalCommitted_MovesBodyArmsLeashOnceCancelsAutorunAndSendsExactlyOneMovementEvent`'s stale-anchor assertion, as the contract's §8 item 11 intended. |
| **A6** — superseded-teleport discriminator removed | **CLOSED** | `NewerStart_ReplacesOldDestinationWithoutReusingIt` asserts `Movement.Controller.Position.X/Y == 2` and the cell; `SameLandblockDestination_…` asserts `(20,30,4)` + `0x20210123`; the Z-vs-X/Y reasoning in the comment is sound. |
| **A9** — two sequence sources | **CLOSED** | The authority now uses `destination.TeleportSequence`, with a `Debug.Assert` pinning it equal to the caller's copy. |
| **A10(a)** — class doc claimed the suffix was the render entity's only mover | **CLOSED** | Doc rewritten to state the writes are redundant repeats of the canonical receipt's mutation, with the ordering cited. |
| **A1** — refused Place did not stop the anim stream | **partially** — the mechanism is right (readiness inversion, sequencer untouched), and the `Contention` path is now correctly held and tested (`RefusedPlace_HoldsTheStreamAndConvergesOnlyAfterContentionClears`). **B1 reopens it for the `DeferredCell` path.** |
| **A2** — no wake re-validation | **partially** — D-T2.4's re-validation now exists and is wired in both hosts and tested; **B2** shows it does not close the FIFO wedge. |
| **A3** — headless discarded the status | **partially** — status now honoured, throws on `Rejected`/`NotApplicable` and on a missing drive, retryable pump added. **B1/B3 reopen it.** |
| **A5** — presentation suite | **still open** — see B5 for the definite blocks/does-not-block answer. |
| **A7** — dual-host parity | **CLOSED** | `HeadlessPortalPrepareDestinationParksThenCommitsOnCollisionGenerationWake` drives a real wired drive end to end and asserts body position, cell, and `PlayerState`. |
| **A8** | open, non-blocking (B8). |
| **A11 / A12** | unchanged (B9). |

## Also verified this round

- `_pendingDestination` lifetime — round-1 judgment stands, not re-litigated.
- The sequencer's own invariants survive the late `worldReady`: `Tunnel` is
  explicitly a hold state (*"Hold here until worldReady"*), `TickTunnel` still
  runs on the hold path so `CurrentAnimationFrame` keeps advancing into
  `TunnelContinue`'s exit window, and `maxForce` at 5 s covers a stale frame.
  `worldReady` has exactly one consumer (`case TeleportAnimState.Tunnel`), so
  no other transition changed meaning.
- No double-Begin headless: `TryCompletePortal`'s own
  `TryGetAcceptedTeleportDestination`/`TryBeginPortalReveal` prefix cannot
  succeed twice for one reveal (the destination slot is consumed), and
  `_awaitingPortalWake` suppresses a second concurrent `Begin` while parked.
  (The stale-latch problem is B3, a different failure.)
- `_placementCommitted` / `_awaitingDeferredWake` are both cleared in
  `ResetTransit:931-932`, so the graphical latches do not survive a new F751,
  a session reset, or a generation reset.
- Build green.

---

## Summary

| # | Severity | Finding |
|---|---|---|
| B1 | MAJOR | `PendingCount == 0` inferred as "committed" on both hosts; three non-committing paths clear it, including the drive's own documented modal outcome. Reintroduces A1 (graphical, now with a *successful* false materialization) and A3 (headless). Untested. |
| B2 | MEDIUM | A2's re-validation gates the suffix, not the receipt; the FIFO wedge is narrowed (supersession/reset only) but not closed. |
| B3 | MEDIUM | Headless `_awaitingPortalWake` is not reset per teleport; a stale latch can skip reveal N+1's placement attempt entirely. |
| B4 | MEDIUM | Unbounded, undiagnosable retry loops on both hosts; headless has no cue, no bound, and no log. |
| B5 | — | §8 8/9/10 presentation suite: **does not block**, subject to three stated conditions; required test spelled out. |
| B6 | MINOR | Make `isPortalAuthorityCurrent` (and B1's commit observable) required constructor parameters. |
| B7 | MINOR | `_placementCommitted` never re-validated before the presentation suffix. |
| B8 | MINOR | A8 still open (5 tests), confirmed non-blocking. |
| B9 | MINOR | A11/A12 unchanged. |