release: v0.9.6 review round 3 — revert P2 self-heal; document contract

Reverts the round-2 self-heal (was: on stale-generation success, clear
`_registrationPromise` so the current hook re-runs). Coderabbit + cubic
both correctly flagged that this reintroduces the very race the round-1
generation guard was meant to prevent: if the stale hook resolves while
a REPLACEMENT hook is still in flight, clearing `_registrationPromise`
clobbers the newer attempt's cached promise — a third caller then starts
a second registration attempt, breaking dedup.

Every attempt to self-heal without inventing a heavier per-entry
generation scheme (or wrapping ``register()`` with a generation guard)
introduces another race. Doing that here would materially complicate
the dispatcher for a scenario that never occurs in production —
``setRegistrationHook`` is called exactly once at startup by
``native/index.ts``, and ``reset()`` is test-only. Test-authored races
that violate isolation are the caller's contract, not this module's
correctness problem.

- Revert to round-1 logic (generation guard on shared-state mutations only)
- Remove the "stale hook self-heal" adversarial test — it was locking in
  behavior we've decided not to guarantee
- Add explicit contract documentation to `dispatcher.ts` and to the
  adversarial test file's top docstring so the design decision is
  discoverable to reviewers next time

Dispatcher suite: 14/14 pass (was 15 with the deleted self-heal test).
The round-1 generation guard is retained and still tested.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: sahrizvi

packages/opencode/src/altimate/native/dispatcher.ts

@@ -61,26 +61,29 @@ export async function call<M extends BridgeMethod>(
   // recover without restarting the CLI. Generation guard prevents a stale
   // attempt from mutating state a concurrent ``reset()``/``setRegistrationHook()``
   // has since replaced. (coderabbit round 1 — release/v0.9.6 review.)
+  // Test-isolation contract: ``reset()`` and ``setRegistrationHook()`` MUST
+  // NOT be called while a ``Dispatcher.call`` is in flight — if they are,
+  // any late ``register()`` calls from the stale hook body may clobber
+  // fresh entries the new hook wrote, and there is no in-band signal we
+  // can use to self-heal it without introducing a second race (see the
+  // coderabbit + cubic round-2 exchange on release/v0.9.6). Production
+  // never triggers this: ``setRegistrationHook`` is called exactly once
+  // at startup by native/index.ts, and ``reset()`` is test-only. Tests
+  // must ``await`` outstanding calls before mutating hook state.
   if (_ensureRegistered) {
     if (!_registrationPromise) {
       const fn = _ensureRegistered
       const generation = ++_registrationGeneration
       _registrationPromise = fn().then(
         () => {
-          // Generation advanced while this attempt was in flight (concurrent
-          // ``reset()``/``setRegistrationHook()`` + another call arrived).
-          // The stale hook body may have written stale entries into
-          // ``nativeHandlers`` via late ``register()`` calls, clobbering the
-          // newer hook's. Clear ``_registrationPromise`` so the NEXT
-          // ``Dispatcher.call`` re-runs the current hook — its ``register()``
-          // calls then overwrite whatever the stale hook wrote. Hook bodies
-          // must be idempotent (they are today — ``register`` is a plain
-          // ``Map.set``). Successful current-generation attempts leave the
-          // resolved promise memoized so subsequent calls fast-path through
-          // an already-settled ``await``. (cubic round 2 on release/v0.9.6.)
-          if (generation !== _registrationGeneration) _registrationPromise = null
+          // Only clear _ensureRegistered if our generation is still current
+          // — otherwise a concurrent reset()/setRegistrationHook() already
+          // installed a replacement, and clearing would clobber it.
+          if (generation === _registrationGeneration) _ensureRegistered = null
         },
         (err) => {
+          // Same guard on the failure path: don't null a newer in-flight
+          // promise from another attempt.
           if (generation === _registrationGeneration) _registrationPromise = null
           throw err
         },

packages/opencode/test/skill/release-v0.9.6-adversarial.test.ts

@@ -3,8 +3,17 @@
  *
  * Focus: fixes that landed IN this release (not the whole PR history) —
  *  1. Dispatcher retry-after-registration-failure (v0.9.6 review gremlin fix)
- *  2. Dispatcher generation guard (coderabbit round 1)
- *  3. Dispatcher stale-register self-heal (cubic round 2)
+ *  2. Dispatcher generation guard on shared-state mutations from stale
+ *     .then handlers (coderabbit round 1)
+ *
+ * Explicitly NOT covered (test-isolation contract, see dispatcher.ts):
+ *  - Late ``register()`` from a stale hook body after a replacement hook
+ *    has already run. That scenario requires calling ``reset()`` /
+ *    ``setRegistrationHook()`` while a call is still in flight — a
+ *    production impossibility (hook is set once at startup, reset is
+ *    test-only) and a violation of the test-isolation contract. See
+ *    ``dispatcher.ts`` for the design decision and the coderabbit/cubic
+ *    round-2 exchange that arrived at it.
  *
  * Not covered here (existing test suites are authoritative):
  *  - altimate-core 0.7.0 shape corrections — see
@@ -179,50 +188,6 @@ describe("v0.9.6 release: Dispatcher registration retry", () => {
     expect(newHookAttempts).toBe(1)
   })
 
-  test("stale hook's LATE register() call is self-healed by a re-run on the next call", async () => {
-    // cubic round 2 on release/v0.9.6: the generation guard prevented a
-    // stale success handler from mutating _ensureRegistered / _registrationPromise,
-    // but the old HOOK BODY itself could still write stale entries into
-    // nativeHandlers via late register() calls after a new hook had already
-    // filled them. Fix: on stale-generation success, clear _registrationPromise
-    // so the next Dispatcher.call re-runs the CURRENT hook and its idempotent
-    // register() calls overwrite whatever the stale hook wrote.
-    let resolveOld: () => void = () => {}
-    const oldPending = new Promise<void>((r) => (resolveOld = r))
-    Dispatcher.setRegistrationHook(async () => {
-      await oldPending
-      Dispatcher.register("ping", async () => ({ status: "stale-old" }))
-    })
-    const firstCall = Dispatcher.call("ping", {} as any)
-
-    Dispatcher.reset()
-    let newRunCount = 0
-    Dispatcher.setRegistrationHook(async () => {
-      newRunCount += 1
-      Dispatcher.register("ping", async () => ({ status: "fresh-new" }))
-    })
-
-    // Second call runs the new (fast) hook to completion — registers
-    // "ping" -> fresh-new, memoized as the resolved promise.
-    const r1 = await Dispatcher.call("ping", {} as any)
-    expect(r1).toEqual({ status: "fresh-new" })
-    expect(newRunCount).toBe(1)
-
-    // Now the old hook wakes up and belatedly overwrites "ping" with
-    // stale-old via its own register() call. Without the self-heal, the
-    // next Dispatcher.call would silently return the stale handler because
-    // registration is memoized "done" and never re-runs.
-    resolveOld()
-    await firstCall.catch(() => {})
-
-    // Third call: the fix detects the stale-generation success, clears
-    // the memoized promise so the CURRENT hook re-runs and overwrites
-    // "ping" back to fresh-new.
-    const r2 = await Dispatcher.call("ping", {} as any)
-    expect(r2).toEqual({ status: "fresh-new" })
-    expect(newRunCount).toBe(2) // re-ran to self-heal the stale register()
-  })
-
   test("reset() clears both the hook and the cached in-flight promise", async () => {
     // reset() must wipe both _ensureRegistered AND _registrationPromise —
     // otherwise a failed registration in one test leaves the cached