fix(client): ре-ревью #509 — 5 пунктов Do-листа (стабильность/регрессии) (#488)

1 [stability] Позитивные attach-исходы гвардятся по ИСХОДНОЙ фазе. Одного epoch-
фильтра мало: POLL_TERMINAL использует to() (epoch не инкрементит) и не шлёт
abortAttach, поэтому медленный GET, вернувший live 2xx уже ПОСЛЕ того как армленный
poll увёл машину в idle, воскрешал осевший ран в фантомный streaming. RECONNECT_
ATTACHED теперь bail'ит если фаза != reconnecting; ATTACH_LIVE/ATTACH_NONE — если
!= attaching. Тест на гонку (POLL_TERMINAL до RECONNECT_ATTACHED → idle) + mutation.

2 [regressions] ownership сбрасывается в "local" на ВСЕХ терминальных переходах
(FINISH_CLEAN/ABORT/ERROR, POLL_TERMINAL, RUN_FACT{null}→idle, honor-in-stopping,
disconnect→idle). Иначе observer-attach + очередь + clean-финиш → idle, но
ownership навсегда observer → «Send now» скрыт при свободном композере. Безопасно
для I2: рантайм захватывает wasObserver из machineRef ДО dispatch. Тест + mutation.

3 [coverage] Тест happy-path CAS-supersede: SUPERSEDE_READY-dispatch (200-исход
transport.fetch) раньше НЕ исполнялся ни в одном тесте — риск залипания в
superseding на весь стрим B. Тест вводит в superseding, гонит A.onFinish→B, затем
POST 200 → SUPERSEDE_READY → streaming, проверяет повторный supersede (не залип).
Сиблинги: 409 SUPERSEDE_TARGET_MISMATCH → getRun/verify; plain-409
A_RUN_ALREADY_ACTIVE → классифицированный баннер. Mutation (no-op READY → красный).

4 [regressions] Inactivity-бэкстоп для poll, армленного в stopping. STOP_REQUESTED
армит poll и входит в stopping, но idle-cap покрывал только polling/reconnecting →
observer-стоп без SDK-стрима и без серверного терминала поллил БД вечно. Добавлен
stopping в фаза-чек эффекта + переход POLL_IDLE_CAP: stopping→idle+disarm (НЕ
stalled — Stop уже нажат). FSM + компонент-тест (idle-cap→disarm) + mutation.

5 [docs] Счёт §2 «Net»: 8→FSM (#1-6,#11,#13) + 3 deleted + reconnectTimerRef
(effect-owned) + mountedRef (retained) = 13; attachAbortRef вне набора #1-13.

Не трогал DROP-блок (мёртвый FINISH_* в superseding, ErrorKind.kind, неиспользуемые
enum-варианты, epochRef-зеркало). Всё прошлое цело (disconnect-first, epoch, honor-
in-stopping, render-gate, supersede). Полный ai-chat 35 файлов / 388 / 0; tsc 0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

apps/client/src/features/ai-chat/components/chat-thread.test.tsx

@@ -353,6 +353,102 @@ describe("ChatThread — send now", () => {
     expect(screen.queryByText(/reconnecting/i)).toBeNull();
   });
 
+  it("#488 review-2: an observer turn's clean finish re-exposes 'Send now' (ownership reset to local)", () => {
+    // Mount attach -> observer. The queued message shows only "Remove" while
+    // observing; once the attached run finishes CLEAN the FSM resets ownership to
+    // local, so "Send now" becomes available again (composer is free).
+    // MUTATION-VERIFY: drop `ownership:"local"` from FINISH_CLEAN -> stays observer
+    // -> "Send now" stays hidden -> red.
+    renderThread({ autonomousRunsEnabled: true, initialRows: streamingTail() });
+    fireEvent.click(screen.getByTestId("queue-btn"));
+    expect(screen.queryByLabelText("Send now")).toBeNull(); // observer -> hidden
+    act(() => {
+      h.state.onFinish?.({
+        message: { id: "a1", role: "assistant", parts: [{ type: "text", text: "done" }] },
+        isAbort: false,
+        isDisconnect: false,
+        isError: false,
+      });
+    });
+    expect(screen.getByLabelText("Send now")).toBeTruthy(); // ownership local again
+  });
+
+  it("#488 review-3: a successful CAS supersede POST (200) exits `superseding` (Send now works again)", async () => {
+    // Happy-path of the transport 409/CAS block: SUPERSEDE_READY was never executed
+    // in tests (reviewer confirmed no-op mutation stayed green). If it does not fire,
+    // the machine stays `superseding` for the whole B stream and "Send now" is dead.
+    startLocalStreamWithRun();
+    fireEvent.click(screen.getByTestId("queue-btn")); // X
+    fireEvent.click(screen.getByLabelText("Send now")); // -> superseding, stop() #1
+    expect(h.state.stop).toHaveBeenCalledTimes(1);
+    // A's onFinish sends B (clears the pending-supersede text) — the realistic
+    // no-overlap sequence.
+    await act(async () => {
+      h.state.onFinish?.({
+        message: { id: "a1", role: "assistant", parts: [] },
+        isAbort: true,
+        isDisconnect: false,
+        isError: false,
+      });
+      await Promise.resolve();
+    });
+    // B's CAS POST returns 200 -> SUPERSEDE_READY -> streaming.
+    vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response("{}", { status: 200 })));
+    await act(async () => {
+      await h.state.transport!.fetch!("http://x", { method: "POST", body: "{}" });
+    });
+    // We LEFT `superseding`: a fresh Send now supersedes AGAIN (still stuck in
+    // superseding -> LOW-2 guard would no-op it). MUTATION-VERIFY: no-op
+    // SUPERSEDE_READY -> stuck superseding -> stop stays at 1 -> red.
+    fireEvent.click(screen.getByTestId("queue-btn")); // Y
+    fireEvent.click(screen.getByLabelText("Send now"));
+    expect(h.state.stop).toHaveBeenCalledTimes(2);
+  });
+
+  it("#488 review-3 sibling: a 409 SUPERSEDE_TARGET_MISMATCH fires the /run verify", async () => {
+    startLocalStreamWithRun();
+    fireEvent.click(screen.getByTestId("queue-btn"));
+    fireEvent.click(screen.getByLabelText("Send now")); // -> superseding
+    h.state.getRun.mockClear();
+    vi.stubGlobal(
+      "fetch",
+      vi.fn().mockResolvedValue(
+        new Response(
+          JSON.stringify({ code: "SUPERSEDE_TARGET_MISMATCH", runId: "run-x" }),
+          { status: 409 },
+        ),
+      ),
+    );
+    await act(async () => {
+      await h.state.transport!.fetch!("http://x", { method: "POST", body: "{}" });
+    });
+    // SUPERSEDE_MISMATCH -> error(supersede-mismatch) + postRun(verify) -> getRun.
+    expect(h.state.getRun).toHaveBeenCalledWith("c1");
+  });
+
+  it("#488 review-3 sibling: a plain 409 A_RUN_ALREADY_ACTIVE shows the classified banner", async () => {
+    renderThread({ autonomousRunsEnabled: true, initialRows: settledTail() });
+    // The SDK sets useChat error to the 409 body on the failed POST.
+    h.state.error = {
+      message:
+        '{"message":"active","code":"A_RUN_ALREADY_ACTIVE","statusCode":409}',
+    };
+    vi.stubGlobal(
+      "fetch",
+      vi.fn().mockResolvedValue(
+        new Response(JSON.stringify({ code: "A_RUN_ALREADY_ACTIVE" }), {
+          status: 409,
+        }),
+      ),
+    );
+    await act(async () => {
+      await h.state.transport!.fetch!("http://x", { method: "POST", body: "{}" });
+    });
+    // RUN_ALREADY_ACTIVE -> phase error(kind) -> the phase-gate lets errorView show
+    // the classified banner.
+    expect(screen.getByText("The agent is already answering")).toBeTruthy();
+  });
+
   it("Send now is HIDDEN while observing a resumed run and VISIBLE on a local stream", () => {
     // Resumed (mount attach) -> observer -> hidden.
     renderThread({ autonomousRunsEnabled: true, initialRows: streamingTail() });
@@ -972,4 +1068,25 @@ describe("ChatThread — live reconnect + stalled", () => {
     expect(screen.getByText(/the run stopped responding/i)).toBeTruthy();
     expect(screen.getByText("Retry")).toBeTruthy();
   });
+
+  it("#488 review-4: an observer-stop's armed poll is bounded by the idle cap (exits to idle, disarm)", () => {
+    // STOP_REQUESTED arms the poll and enters `stopping`; an observer stop has no
+    // SDK stream to fire onFinish, and the server stop may never drive the run
+    // terminal. Without a backstop the DB would poll forever. The idle cap must give
+    // `stopping` a bounded exit -> idle + disarm (NOT stalled). MUTATION-VERIFY: drop
+    // `stopping` from the idle-cap effect / the POLL_IDLE_CAP branch -> no disarm.
+    const { onResumeFallback } = renderThread({
+      autonomousRunsEnabled: true,
+      initialRows: streamingTail(), // mount attach -> observer
+    });
+    onResumeFallback.mockClear();
+    fireEvent.click(screen.getByLabelText("Stop")); // STOP_REQUESTED -> stopping + armPoll
+    expect(onResumeFallback).toHaveBeenCalledWith(true);
+    onResumeFallback.mockClear();
+    // No terminal row ever arrives; the inactivity cap bounds it.
+    act(() => {
+      vi.advanceTimersByTime(10 * 60_000);
+    });
+    expect(onResumeFallback).toHaveBeenCalledWith(false); // POLL_IDLE_CAP -> idle -> disarm
+  });
 });

apps/client/src/features/ai-chat/components/chat-thread.tsx

@@ -878,8 +878,10 @@ export default function ChatThread({
       clearTimeout(idleCapTimerRef.current);
       idleCapTimerRef.current = null;
     }
+    // Review #4: `stopping` also arms the poll and needs a bounded exit (the FSM
+    // maps POLL_IDLE_CAP from `stopping` -> idle, not stalled).
     const p = phase.name;
-    if (p !== "polling" && p !== "reconnecting") return;
+    if (p !== "polling" && p !== "reconnecting" && p !== "stopping") return;
     idleCapTimerRef.current = setTimeout(() => {
       dispatchRef.current({ type: "POLL_IDLE_CAP" });
     }, DEGRADED_POLL_IDLE_MAX_MS);

apps/client/src/features/ai-chat/state/run-fsm.spec.md

@@ -128,10 +128,12 @@ holds. **Pending column: empty.**
 | NEW | `pendingSupersedeRef` | **data** (send-plumbing) | the runId injected into the next `POST /stream {supersede}`; the single replacement for the 3 DELETED one-shots (#8/#9/#10) — net −2 refs |
 | NEW | `idleCapTimerRef` | **effect-owned timer** | the stalled inactivity cap → `POLL_IDLE_CAP` (commit 4a); not a flag |
 
-Net: the 13 lifecycle flags (#1–#13) are eliminated (7 → FSM phase/ctx/epoch/event,
-3 deleted, `reconnectTimerRef`/`attachAbortRef` become effect-owned controllers,
-`mountedRef` retained as React liveness). Two effect-owned timers + one send-plumbing
-data ref are added — none is a boolean lifecycle latch.
+Net: the 13 lifecycle flags (#1–#13) are eliminated: **8** → FSM phase/ctx/epoch/event
+(#1–#6, #11, #13), **3** deleted (#8/#9/#10), **`reconnectTimerRef` (#7)** becomes an
+effect-owned controller, and **`mountedRef` (#12)** is retained as React liveness
+(8 + 3 + 1 + 1 = 13). (`attachAbortRef` (#16) is outside the #1–#13 set — it was
+already an effect-owned controller.) Two effect-owned timers + one send-plumbing data
+ref are added — none is a boolean lifecycle latch.
 
 ---
 

apps/client/src/features/ai-chat/state/run-fsm.test.ts

@@ -363,6 +363,75 @@ describe("run-fsm — stop (I4: exit by data)", () => {
     m = reduce(m, { type: "RUN_FACT", runFact: null, epoch: m.ctx.epoch });
     expect(m.phase.name).toBe("idle");
   });
+
+  // Review #4: `stopping` arms the poll but had no inactivity backstop.
+  it("review-4: POLL_IDLE_CAP in `stopping` exits to idle (bounded), NOT stalled", () => {
+    let m = reduce(withRunFact(), { type: "STOP_REQUESTED" });
+    expect(m.phase.name).toBe("stopping");
+    expect(hasEffect(m, "armPoll")).toBe(true);
+    // MUTATION-VERIFY: drop the `stopping` branch in POLL_IDLE_CAP and this hangs
+    // in `stopping` (poll forever) -> red.
+    m = reduce(m, { type: "POLL_IDLE_CAP" });
+    expect(m.phase.name).toBe("idle");
+    expect(hasEffect(m, "disarmPoll")).toBe(true);
+    expect(m.ctx.ownership).toBe("local");
+  });
+});
+
+// Review #1: positive attach outcomes must be guarded by the SOURCE phase — the
+// epoch filter alone is insufficient because POLL_TERMINAL uses to() (no epoch
+// bump) and does not abort the in-flight GET.
+describe("run-fsm — review-1: attach outcomes guarded by source phase", () => {
+  it("a late RECONNECT_ATTACHED after POLL_TERMINAL stays idle (no phantom streaming)", () => {
+    let m = withRunFact("run-1");
+    m = reduce(m, { type: "FINISH_DISCONNECT", hasVisibleContent: true, epoch: m.ctx.epoch });
+    m = reduce(m, { type: "RECONNECT_ATTEMPT", attempt: 1, epoch: m.ctx.epoch }); // attach GET
+    const epoch = m.ctx.epoch;
+    // The armed degraded poll reaches the terminal row FIRST (epoch unchanged).
+    m = reduce(m, { type: "POLL_TERMINAL" });
+    expect(m.phase.name).toBe("idle");
+    expect(m.ctx.epoch).toBe(epoch); // POLL_TERMINAL did NOT bump the epoch
+    // The slow GET returns live 2xx under the SAME epoch — must NOT resurrect.
+    m = reduce(m, { type: "RECONNECT_ATTACHED", epoch });
+    expect(m.phase.name).toBe("idle");
+  });
+
+  it("a late ATTACH_LIVE / ATTACH_NONE after leaving `attaching` is ignored", () => {
+    let m = reduce(initialMachine(), { type: "ATTACH_START", runId: "r" });
+    const epoch = m.ctx.epoch;
+    m = reduce(m, { type: "ATTACH_NONE", epoch }); // attaching -> polling
+    m = reduce(m, { type: "POLL_TERMINAL" }); // -> idle (epoch unchanged)
+    expect(m.phase.name).toBe("idle");
+    m = reduce(m, { type: "ATTACH_LIVE", epoch }); // late 2xx, same epoch
+    expect(m.phase.name).toBe("idle");
+    // And a late ATTACH_NONE (not `attaching`) is a no-op too.
+    m = reduce(m, { type: "ATTACH_NONE", epoch });
+    expect(m.phase.name).toBe("idle");
+  });
+});
+
+// Review #2: every terminal transition resets ownership to local.
+describe("run-fsm — review-2: terminal transitions reset ownership to local", () => {
+  const observer = (): Machine => {
+    let m = reduce(initialMachine(), { type: "ATTACH_START", runId: "r" });
+    m = reduce(m, { type: "ATTACH_LIVE", epoch: m.ctx.epoch });
+    expect(m.ctx.ownership).toBe("observer");
+    return m;
+  };
+  it("FINISH_CLEAN resets ownership", () => {
+    const m = reduce(observer(), { type: "FINISH_CLEAN", epoch: observer().ctx.epoch });
+    expect(m.ctx.ownership).toBe("local");
+  });
+  it("FINISH_ERROR / POLL_TERMINAL / RUN_FACT(null) reset ownership", () => {
+    let o = observer();
+    expect(reduce(o, { type: "FINISH_ERROR", kind: "stream", epoch: o.ctx.epoch }).ctx.ownership).toBe("local");
+    // POLL_TERMINAL from an observer polling phase
+    let p = reduce(observer(), { type: "STREAM_INCOMPLETE", reason: "starved", epoch: observer().ctx.epoch });
+    expect(reduce(p, { type: "POLL_TERMINAL" }).ctx.ownership).toBe("local");
+    // RUN_FACT(null) from an observer attaching phase
+    let a = reduce(initialMachine(), { type: "ATTACH_START", runId: "r" });
+    expect(reduce(a, { type: "RUN_FACT", runFact: null, epoch: a.ctx.epoch }).ctx.ownership).toBe("local");
+  });
 });
 
 describe("run-fsm — ownership (I2) is context, orthogonal to phase", () => {

apps/client/src/features/ai-chat/state/run-fsm.ts

@@ -254,7 +254,9 @@ export function reduce(m: Machine, event: Event): Machine {
   // `superseding` (a successor B owns) — that is the F1 supersede drop.
   if (m.phase.name === "stopping" && isFinishEvent(event)) {
     return to(m, { name: "idle" }, {
-      ctx: { runFact: null, liveFollow: false },
+      // Reset ownership to local on this terminal transition (review #2): otherwise
+      // an observer-stop leaves ownership 'observer' and hides "Send now" forever.
+      ctx: { runFact: null, liveFollow: false, ownership: "local" },
       effects: [{ type: "disarmPoll" }, { type: "cancelReconnect" }],
     });
   }
@@ -297,9 +299,10 @@ export function reduce(m: Machine, event: Event): Machine {
     case "FINISH_CLEAN":
       // A clean terminal outcome. The run is done — clear the run-fact and go
       // idle. (The queue flush is a component concern gated by ownership; the
-      // FSM only models the phase.)
+      // FSM only models the phase.) Review #2: reset ownership to local so a
+      // just-finished observer-attach turn re-exposes "Send now" for the queue.
       return to(m, { name: "idle" }, {
-        ctx: { runFact: null, liveFollow: false },
+        ctx: { runFact: null, liveFollow: false, ownership: "local" },
         effects: [{ type: "disarmPoll" }, { type: "cancelReconnect" }],
       });
 
@@ -307,7 +310,7 @@ export function reduce(m: Machine, event: Event): Machine {
       // A user Stop / intentional abort finished. If we were stopping, the
       // terminal data has now arrived (I4) — go idle. The run-fact is cleared.
       return to(m, { name: "idle" }, {
-        ctx: { runFact: null, liveFollow: false },
+        ctx: { runFact: null, liveFollow: false, ownership: "local" },
         effects: [{ type: "disarmPoll" }, { type: "cancelReconnect" }],
       });
 
@@ -340,11 +343,13 @@ export function reduce(m: Machine, event: Event): Machine {
         });
       }
       // No run to recover: a plain disconnect. Surface the terminal notice.
-      return to(m, { name: "idle" }, { ctx: { runFact: null, liveFollow: false } });
+      return to(m, { name: "idle" }, {
+        ctx: { runFact: null, liveFollow: false, ownership: "local" },
+      });
 
     case "FINISH_ERROR":
       return to(m, { name: "error", kind: event.kind }, {
-        ctx: { runFact: null, liveFollow: false },
+        ctx: { runFact: null, liveFollow: false, ownership: "local" },
         effects: [{ type: "disarmPoll" }, { type: "cancelReconnect" }],
       });
 
@@ -364,6 +369,12 @@ export function reduce(m: Machine, event: Event): Machine {
 
     case "ATTACH_LIVE":
       // The attach GET returned a live 2xx stream — follow it as an observer.
+      // Review #1: guard by SOURCE phase. The epoch filter alone is not enough — a
+      // POLL_TERMINAL uses to() (no epoch bump) and does not abort the in-flight
+      // GET, so a slow 2xx landing after the machine already left `attaching` (e.g.
+      // the armed poll saw the terminal row -> idle) would resurrect a settled run
+      // into a phantom `streaming`. Only enter streaming FROM `attaching`.
+      if (m.phase.name !== "attaching") return stay(m);
       return to(m, { name: "streaming" });
 
     case "ATTACH_NONE":
@@ -371,6 +382,9 @@ export function reduce(m: Machine, event: Event): Machine {
       // follow the run to terminal from the DB. This is a soft-negative run-fact
       // (204 on a non-stripped path is authoritative-negative; the runtime may
       // pass a RUN_FACT null separately). Keep the run-fact as-is here.
+      // Review #1: guard by source phase for consistency (a late outcome after the
+      // machine already left `attaching` must not re-arm a poll).
+      if (m.phase.name !== "attaching") return stay(m);
       return to(m, { name: "polling", reason: "attach-none" }, {
         effects: [{ type: "armPoll", reason: "attach-none" }],
       });
@@ -391,6 +405,12 @@ export function reduce(m: Machine, event: Event): Machine {
       // attempt counter is dropped, so a LATER disconnect can start a fresh
       // ladder from attempt 1 (the old one-shot `!wasResumed` gate forbade a
       // second cycle, sending the second break to silent poll).
+      // Review #1: guard by SOURCE phase. The armed degraded poll can reach the
+      // terminal row (POLL_TERMINAL -> idle, via to(), NO epoch bump, GET not
+      // aborted) BEFORE a slow reconnect GET returns 2xx; without this guard that
+      // late RECONNECT_ATTACHED (same epoch) would resurrect a settled run into a
+      // phantom `streaming`. Only re-enter streaming FROM `reconnecting`.
+      if (m.phase.name !== "reconnecting") return stay(m);
       return to(m, { name: "streaming" }, {
         effects: [{ type: "cancelReconnect" }, { type: "disarmPoll" }],
       });
@@ -437,13 +457,24 @@ export function reduce(m: Machine, event: Event): Machine {
     case "POLL_TERMINAL":
       // The run reached a terminal row via the poll (or the reconcile merge). Go
       // idle and disarm everything (I4: this is a DATA-driven exit, incl. exit
-      // from `stopping`).
+      // from `stopping`). Review #2: reset ownership to local.
       return to(m, { name: "idle" }, {
-        ctx: { runFact: null, liveFollow: false },
+        ctx: { runFact: null, liveFollow: false, ownership: "local" },
         effects: [{ type: "disarmPoll" }, { type: "cancelReconnect" }],
       });
 
     case "POLL_IDLE_CAP":
+      // Review #4: `stopping` also arms the poll (STOP_REQUESTED) but has NO other
+      // backstop — an observer-stop with no SDK stream to fire onFinish, whose
+      // server stop never drives the run terminal, would poll the DB forever. Give
+      // it a bounded exit: cap -> idle + disarm (NOT `stalled`; Stop was already
+      // pressed, so there is nothing for the user to retry).
+      if (m.phase.name === "stopping") {
+        return to(m, { name: "idle" }, {
+          ctx: { runFact: null, liveFollow: false, ownership: "local" },
+          effects: [{ type: "disarmPoll" }, { type: "cancelReconnect" }],
+        });
+      }
       // #488 commit 4a: the poll hit the inactivity cap. Instead of going SILENT
       // (the old "forever half-done answer"), surface a stalled banner + Retry.
       if (m.phase.name !== "polling" && m.phase.name !== "reconnecting") return stay(m);
@@ -464,7 +495,8 @@ export function reduce(m: Machine, event: Event): Machine {
           m.phase.name === "stopping"
         ) {
           return to(m, { name: "idle" }, {
-            ctx: { runFact: null, liveFollow: false },
+            // Review #2: reset ownership to local on this terminal transition.
+            ctx: { runFact: null, liveFollow: false, ownership: "local" },
             effects: [{ type: "cancelReconnect" }, { type: "disarmPoll" }],
           });
         }