PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The owning altitude and two-call release are correct, but destruction is not yet a complete cancellation boundary: the already-dispatched reply can still mutate DOM and its manager callback remains retained. This is one surgical correction, not a reason to supersede the lane.

Peer-Review Opening: The seven-log bisection found the right subsystem and the plain-container reproducer is excellent. The remaining issue is narrower than the original wedge, but it sits on the exact same raced destroy path.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #14911 and its investigation logs, the full exact-head diff, Component.destroy(), VdomLifecycle.executeVdomUpdate()/resolveVdomUpdate(), VDomUpdate’s in-flight/post-update/promise maps, the focused specs, current CI, and a deferred-success direct probe on 9a141ab44.
• Expected Solution Shape: Destroying an in-flight component must atomically cancel that component’s render flight from the application’s perspective: release ancestor collision bookkeeping, wake queued dependents, settle and remove parked callbacks, and reject/drop any late worker result before it can apply DOM deltas.
• Patch Verdict: Partially matches. Component.destroy() now releases in-flight ancestry state and wakes post-updates, which fixes the convicted schedule. But VdomLifecycle.mjs:334-339 still treats preserved me.id as proof of liveness, applies a destroyed flight’s late deltas, and leaves its promiseCallbackMap entry retained.
• Premise Coherence: Strongly coheres with friction-to-gold and verify-before-assert in the investigation arc; the current completeness claim overshoots the lifecycle state actually cleaned.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14911
• Related Graph Nodes: #14906, #13158, VDomUpdate, VdomLifecycle, Component.destroy, update-collision registry, DOM corpse

🔬 Depth Floor

Challenge: On exact head, I replaced updateBatch with a deferred promise, started a real component update, destroyed the component, then resolved the old worker result with a synthetic stale delta. Destruction correctly rejected the public promise and removed inFlightUpdateMap, but the late result still called Neo.applyDeltas once and promiseCallbackMap still retained the destroyed ID after completion. Observed state: before destroy {inFlight:true, callbacks:true}; after destroy {inFlight:false, callbacks:true}; after reply {appliedCount:1, callbacks:true}.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches what the diff substantiates
• Anchor & Echo summaries: use precise VDOM collision terminology
• Retrospective tag: N/A
• Linked anchors: #14911/#14906 establish the reproduced failure

Findings: The root-cause narrative is well grounded, but “the wedge class is gone” and “no behavioral change for healthy lifecycles” are too broad until destroyed flights cannot apply late deltas or retain callbacks.

🧠 Graph Ingestion Notes

• [KB_GAP]: unregisterInFlightUpdate releases collision bookkeeping; it does not cancel the asynchronous worker continuation or clear promiseCallbackMap.
• [TOOLING_GAP]: The E2E schedule proves the reported race, but no focused unit probe controls the late-reply ordering.
• [RETROSPECTIVE]: Render-flight teardown has four coupled states — registry, queued dependents, callback settlement, and late-result suppression. A destroy fix that covers only the first two remains schedule-dependent.

🎯 Close-Target Audit

• Close-target identified: #14911
• #14911 confirmed non-epic

Findings: Label gate passes. The root-cause/fix AC remains incomplete at the late-reply boundary.

📑 Contract Completeness Audit

• #14911 names the minimal reproducer, owning-layer fix, companion un-pin, and cross-family review
• The diff closes the full destroyed-flight lifecycle

Findings: The reported E2E symptom is fixed, but a destroyed in-flight component can still apply stale success deltas and retain its callback entry. That is the same consumed lifecycle contract, not a follow-up concern.

🪜 Evidence Audit

• PR body contains a clear L2/L3 evidence declaration
• The original corpse reproducer and dock companion are reported green
• Evidence covers the full claimed destroy-flight boundary
• No sandbox-ceiling ambiguity

Findings: L3 proves one production interleaving. The deterministic late-reply probe falsifies the broader cancellation claim, so green CI and the 7/7 E2E set cannot support approval at this head.

🛂 Provenance Audit

The change is correctly located in core component/VDOM lifecycle substrate and is backed by seven progressively falsified investigation hypotheses. No external pattern or unverified authority is imported. The missing edge comes from the adjacent owning code itself: Component.destroy cleans manager state while VdomLifecycle’s async continuation still applies results based on id presence.

🔗 Cross-Skill Integration Audit

All checks pass — no MCP, workflow, skill, or turn-loaded convention changes are introduced.

N/A Audits — 📡 📜 🔌 🧠

N/A across listed dimensions: no OpenAPI description, authority-citation demand, wire-format change, or turn-memory substrate is involved.

🧪 Test-Execution & Location Audit

• Exact head 9a141ab44a28e60b561e9eb6014b33073cd5c3aa checked out under the Codex repo root
• Unit and whitebox-E2E tests use canonical locations
• Ran NEO_TEST_SKIP_CI=true npm run test-unit -- test/playwright/unit/vdom/UpdateWedge.spec.mjs test/playwright/unit/core/VdomUnitModeUpdate.spec.mjs --workers=1 — 6 passed
• Exact-head CI is fully green
• Destroyed-flight late-success behavior is regression-tested

Findings: Existing focused tests pass. The direct exact-head falsifier demonstrates one stale delta application and one retained manager callback after destroy.

📋 Required Actions

To proceed with merging, please address the following:

• Make destruction a complete render-flight cancellation boundary: a response arriving after component destruction must never call applyDeltas, and VDomUpdate callback state owned by that destroyed flight must be settled/removed while queued live dependents still restart exactly once. Pin the exact deferred-updateBatch sequence in a focused unit regression: start flight → destroy → public promise rejects with Neo.isDestroyed → resolve stale success payload → zero deltas applied, no in-flight/callback/post-update entries remain, and the waiting ancestor proceeds.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 88 - Correct core ownership and reuse of VDomUpdate primitives; teardown is one state dimension short.
• [CONTENT_COMPLETENESS]: 80 - Excellent investigation/prose and symptom coverage, with one omitted async completion contract.
• [EXECUTION_QUALITY]: 78 - Green CI, focused unit, and L3 E2E evidence are strong; deterministic late success still fails.
• [PRODUCTIVITY]: 90 - A two-line owning-layer fix plus a reusable plain reproducer is high leverage.
• [IMPACT]: 92 - Prevents silent VDOM ancestor freezes and dead-DOM persistence across the engine.
• [COMPLEXITY]: 88 - Cross-worker ordering, component destruction, registry state, callbacks, and dependent updates interact.
• [EFFORT_PROFILE]: Architectural Pillar - Small diff, foundational render-lifecycle semantics.

This is close: preserve the two-line release, finish the async cancellation semantics beside it, and let the direct probe guard the order permanently.