Ре-ревью нашло регрессию класса #137/#161 на пути отказа getRun при tail-only
re-seed:
- НАХОДКА 1 (MEDIUM/HIGH): в onFinish-disconnect ветка .catch (отказ getRun) на
локальном дропе входила в reconnect-ladder БЕЗ ре-сида и БЕЗ фильтра «живой»
частичной строки. anchorRef оставался устаревшим (mount-инициализатор), живая
частичная строка со шагами N..M-1 — последней; через ~1с реконнект строил
?anchor=&n=N_mount, и при живом ране с покрытием от N_mount (flaky-сеть: SSE и
getRun упали, сеть поднялась за 1с) сервер отдавал кадры ≥N_mount → SDK дописывал
их к строке, где они УЖЕ есть → дублирование (клиентского дедупа реплея против
parts нет). Фикс: восстановлена структурная гарантия удалённого resumeStream-
фильтра — на отказе getRun (и на no-persisted-row, и на no-cid) живая частичная
строка удаляется из стора по id + anchorRef=null → реконнект реплеит со start в
ЧИСТЫЙ стор (полная пересборка) либо 204→poll. Нет пути, где attach tail-applies
на строку с уже присутствующими шагами. Тест: getRun-reject на локальном
дисконнекте → живая строка отфильтрована + URL без параметров (mutation-verify:
без фикса тест краснеет — фильтр не срабатывает).
- НАХОДКА 2 (LOW): RUN_FACT в enterReconnect теперь epoch-штампуется (epoch:
stampEpoch), как везде (postRun): getRun-rtt расширяет окно onFinish→dispatch,
конкурентный SEND_LOCAL во время rtt теперь дропает устаревший RUN_FACT по I1,
а не перетирает runFact.runId нового хода.
- НАХОДКА 3 (LOW, doc): run-fsm.spec.md обновлён — stripRef/strippedRowRef →
anchorRef {id, stepsPersisted}, tail-only + re-seed-from-persist.
FSM run-fsm.ts не тронут; инварианты #488 (epoch/honor-in-stopping/ownership-reset/
disconnect-first/render-gate) сохранены. Клиент ai-chat vitest 399 зелёный, tsc 0
ai-chat-ошибок; сервер delta(6, реально исполняется)/registry/step-marker/attach +
integration attach — зелёное.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1009,6 +1009,43 @@ describe("ChatThread — live reconnect + stalled", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("#491 regression (#137/#161 dup): getRun REJECT on a live disconnect drops the live partial + nulls the anchor", async () => {
|
||||
// The re-seed source (getRun) FAILS — a flaky-network blip (SSE + getRun both
|
||||
// fail, network recovers in ~1s). The OLD .catch just re-entered the ladder with
|
||||
// NO re-seed and NO filter, so the reconnect could tail-apply the registry's
|
||||
// frames onto the live partial that ALREADY has those steps -> duplicated text.
|
||||
renderLive();
|
||||
h.state.getRun.mockReset();
|
||||
h.state.getRun.mockRejectedValue(new Error("network"));
|
||||
await disconnect(); // live partial = liveMsg (id "a2")
|
||||
expect(h.state.getRun).toHaveBeenCalledWith("c1");
|
||||
// THE GUARANTEE: on the getRun failure the live partial (a2) is FILTERED from the
|
||||
// store, so the reconnect can never tail-apply already-present steps onto it.
|
||||
// MUTATION-VERIFY: revert the .catch fix (enterReconnect only, no filter) and no
|
||||
// setMessages call removes a2 -> this reddens.
|
||||
const removedLivePartial = (
|
||||
h.state.setMessages as unknown as {
|
||||
mock: { calls: [unknown][] };
|
||||
}
|
||||
).mock.calls.some(([updater]) => {
|
||||
if (typeof updater !== "function") return false;
|
||||
const out = (updater as (p: { id: string }[]) => { id: string }[])([
|
||||
{ id: "a2" },
|
||||
{ id: "u1" },
|
||||
]);
|
||||
return !out.some((m) => m.id === "a2");
|
||||
});
|
||||
expect(removedLivePartial).toBe(true);
|
||||
expect(screen.getByText(/reconnecting/i)).toBeTruthy();
|
||||
advanceToAttempt(1);
|
||||
expect(h.state.resumeStream).toHaveBeenCalledTimes(1);
|
||||
// Anchor was nulled -> replay-from-start (no params) / 204 -> poll; never a stale
|
||||
// ?anchor=&n= over the live partial.
|
||||
expect(h.state.transport!.prepareReconnectToStreamRequest!().api).toBe(
|
||||
"/api/ai-chat/runs/c1/stream",
|
||||
);
|
||||
});
|
||||
|
||||
it("#488 (browser QA): the reconnect banner is SHOWN, not masked by the residual useChat error", async () => {
|
||||
// The drop sets useChat `error` (real SDK), and the terminal errorView describes
|
||||
// it ("Lost connection to the server"). The FSM phase-gate must let the
|
||||
|
||||
@@ -713,13 +713,33 @@ export default function ChatThread({
|
||||
const runId = extractRunId(message ?? undefined) ?? "pending";
|
||||
const enterReconnect = (fact: string): void => {
|
||||
if (!mountedRef.current) return;
|
||||
dispatch({ type: "RUN_FACT", runFact: { runId: fact } });
|
||||
// Epoch-stamp the run-fact too (I1): the getRun rtt widens the
|
||||
// onFinish->dispatch window, so a concurrent SEND_LOCAL during it must be
|
||||
// able to drop this stale RUN_FACT (else it clobbers the new turn's
|
||||
// runFact.runId). Consistent with the postRun RUN_FACT stamp.
|
||||
dispatch({ type: "RUN_FACT", runFact: { runId: fact }, epoch: stampEpoch });
|
||||
dispatch({
|
||||
type: "FINISH_DISCONNECT",
|
||||
hasVisibleContent: msgHasVisible,
|
||||
epoch: stampEpoch,
|
||||
});
|
||||
};
|
||||
// Restore the STRUCTURAL guarantee that the live partial is never the
|
||||
// tail-apply base: drop the live partial from the store by id and null the
|
||||
// anchor, so the reconnect replays from step 0 into a CLEAN store (a full
|
||||
// rebuild) or, past any rotation, 204s -> degraded poll. Used on BOTH the
|
||||
// no-persisted-row and getRun-FAILURE paths — after this there is no path
|
||||
// where the attach tail-applies frames onto a row that already has them
|
||||
// (the #137/#161 duplication class).
|
||||
const dropLivePartialAndReplayFromStart = (): void => {
|
||||
if (message?.role === "assistant" && typeof message.id === "string") {
|
||||
const liveId = message.id;
|
||||
setMessagesRef.current?.((prev) =>
|
||||
prev.filter((m) => m.id !== liveId),
|
||||
);
|
||||
}
|
||||
anchorRef.current = null;
|
||||
};
|
||||
if (cid) {
|
||||
void getRun(cid)
|
||||
.then((res) => {
|
||||
@@ -731,23 +751,29 @@ export default function ChatThread({
|
||||
stepsPersisted: stepsPersistedOf(persisted),
|
||||
};
|
||||
// Replace the live partial with the persisted row IN PLACE by id —
|
||||
// the re-seed from persist.
|
||||
// the re-seed from persist. The attach's tail (steps >= N) then
|
||||
// appends to a store holding EXACTLY steps 0..N-1: no duplication.
|
||||
setMessages((prev) => mergeById(prev, rowToUiMessage(persisted)));
|
||||
} else {
|
||||
// No persisted assistant row (pre-first-frame break): no anchor; the
|
||||
// reconnect attaches with no params and replays from start.
|
||||
anchorRef.current = null;
|
||||
// No persisted assistant row (pre-first-frame break): drop the live
|
||||
// partial + replay from start (no anchor/n) so nothing is duplicated.
|
||||
dropLivePartialAndReplayFromStart();
|
||||
}
|
||||
enterReconnect(res.run?.id ?? runId);
|
||||
})
|
||||
.catch(() => {
|
||||
// Persist read failed: keep whatever anchor we had and still enter the
|
||||
// ladder so a live detached run is not stranded (the attach 204s ->
|
||||
// degraded poll if it cannot cover).
|
||||
if (!mountedRef.current) return;
|
||||
// Persist read FAILED: we cannot re-seed from fresh persist, and a
|
||||
// stale mount-time anchor over the live partial would tail-apply
|
||||
// already-present steps -> duplication (a flaky-network blip:
|
||||
// SSE + getRun both fail, network recovers in ~1s, the registry still
|
||||
// covers from the mount frontier). Restore the removed-filter guarantee
|
||||
// instead: drop the live partial + replay from start / 204 -> poll.
|
||||
dropLivePartialAndReplayFromStart();
|
||||
enterReconnect(runId);
|
||||
});
|
||||
} else {
|
||||
anchorRef.current = null;
|
||||
dropLivePartialAndReplayFromStart();
|
||||
enterReconnect(runId);
|
||||
}
|
||||
setStopNotice(null);
|
||||
|
||||
@@ -121,8 +121,7 @@ holds. **Pending column: empty.**
|
||||
| 11 | `stopPendingRef` | **FSM phase `stopping`** | the deferred stop fires from the chat-id adoption effect while `stopping` |
|
||||
| 12 | `mountedRef` | **retained (React liveness)** | orthogonal to run-lifecycle; gates imperative onFinish side-effects post-unmount. Epoch (I1) handles stale COMMAND-outcomes; DISPOSE bumps it |
|
||||
| 13 | `attemptResumeRef` | **FSM `ATTACH_START` + run-fact** | mount arms attach ONLY on a confirmed active run (commit 4b: streaming-tail status, or POST /run for a user tail) |
|
||||
| 14 | `stripRef` | **data** (attachStrategy) | strip+replay detail; the `resumeStream` effect reads it |
|
||||
| 15 | `strippedRowRef` | **data** (attachStrategy) | the anchor row |
|
||||
| 14–15 | `anchorRef {id, stepsPersisted}` | **data** (attachStrategy) | #491 tail-only: replaced `stripRef`/`strippedRowRef`. The PERSISTED assistant row that pins the run (server invariant 6) + its step frontier N; feeds `?anchor=<id>&n=<stepsPersisted>`. No strip — the seed keeps every row; entering reconnecting re-seeds from persist |
|
||||
| 16 | `attachAbortRef` | **effect-owned controller** | aborted by the `abortAttach` effect in cleanup (I5) |
|
||||
| 17–25 | `chatIdRef`, `openPageRef`, `getEditorSelectionRef`, `roleIdRef`, `stableIdRef`, `queuedRef`, `sendMessageRef`, `statusRef`, `lastForwardedChatIdRef` | **data** (identity/send mirrors) | unchanged — not lifecycle flags |
|
||||
| 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 |
|
||||
@@ -178,6 +177,9 @@ Pessimism rule: a stale-but-positive fact PERMITS entering recovery (attach); th
|
||||
/run) are effect-owned and aborted in cleanup (`abortAttach` on `DISPOSE`), not
|
||||
render-phase refs. A client abort of an already-sent POST does not cancel the
|
||||
server action, so disarming on unmount is safe.
|
||||
- **attachStrategy** (strip+replay today) is behind the `resumeStream` effect; the
|
||||
resume-stack iteration (#491) swaps it to tail-only WITHOUT touching the FSM.
|
||||
- **attachStrategy** is behind the `resumeStream` effect; #491 swapped it to
|
||||
tail-only (`?anchor=&n=`, `anchorRef` data) WITHOUT touching the FSM. Entering
|
||||
reconnecting always re-seeds from persist; on a getRun failure the live partial
|
||||
is dropped + replay-from-start so it is never the tail-apply base (no #137/#161
|
||||
duplication).
|
||||
- **Queue** stays a data structure; flush/interrupt decisions are transitions.
|
||||
|
||||
Reference in New Issue
Block a user