fix(ai-chat): W1 — клиент читает 409-поле activeRunId (было runId=undefined)
Ревью волны 1 (agent_vscode): сервер эмитит id текущего рана в activeRunId
на обоих 409-бранчах (SUPERSEDE_TARGET_MISMATCH, A_RUN_ALREADY_ACTIVE), а
клиентский read409 читал runId → SUPERSEDE_MISMATCH{currentRunId} всегда
undefined: быстрый supersede-хинт мёртв в проде, а клиентские тесты
ложно-зелёные (мокали поле runId, которого сервер не шлёт).
- read409 читает activeRunId (undefined-safe); оба мока — на реальную форму
+ ассерт на усвоенный currentRunId (mutation: revert→runId краснит тесты).
- S4 (groundwork): activeRunId из A_RUN_ALREADY_ACTIVE поглощается в runFact
(to(), без бампа эпохи/ownership — инварианты #488 целы). Полная обвязка
supersede чужой вкладки из фазы error требует расширения gate в sendNow —
отдельным follow-up; сейчас это безопасная заготовка, не рабочая фича.
- spec.md: 2 строки контракт-таблицы приведены к activeRunId.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -414,7 +414,9 @@ describe("ChatThread — send now", () => {
|
||||
"fetch",
|
||||
vi.fn().mockResolvedValue(
|
||||
new Response(
|
||||
JSON.stringify({ code: "SUPERSEDE_TARGET_MISMATCH", runId: "run-x" }),
|
||||
// REAL server body shape: the current run id is `activeRunId`, NOT `runId`
|
||||
// (see ai-chat.controller.ts SUPERSEDE_TARGET_MISMATCH branch).
|
||||
JSON.stringify({ code: "SUPERSEDE_TARGET_MISMATCH", activeRunId: "run-x" }),
|
||||
{ status: 409 },
|
||||
),
|
||||
),
|
||||
@@ -426,6 +428,84 @@ describe("ChatThread — send now", () => {
|
||||
expect(h.state.getRun).toHaveBeenCalledWith("c1");
|
||||
});
|
||||
|
||||
it("#497/W1: the mismatch ABSORBS the server's activeRunId into runFact (fast hint is live) — a follow-up Send now CAS-targets it", async () => {
|
||||
// The 409 body's current run id is `activeRunId`; read409 must feed THAT into
|
||||
// SUPERSEDE_MISMATCH{currentRunId} -> runFact, else the fast hint is undefined.
|
||||
// Observe the absorbed fact via the NEXT CAS supersede body. Keep the verify
|
||||
// getRun PENDING so it cannot overwrite the absorbed fact with its own result.
|
||||
h.state.getRun.mockReturnValue(new Promise(() => {})); // verify never resolves
|
||||
startLocalStreamWithRun(); // runFact run-1, sending, local, autonomous
|
||||
fireEvent.click(screen.getByTestId("queue-btn")); // X
|
||||
fireEvent.click(screen.getByLabelText("Send now")); // -> superseding (target run-1)
|
||||
// A's onFinish sends B and CLEARS the pending-supersede text (no-overlap).
|
||||
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 -> 409 SUPERSEDE_TARGET_MISMATCH with the REAL field `activeRunId`.
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn().mockResolvedValue(
|
||||
new Response(
|
||||
JSON.stringify({ code: "SUPERSEDE_TARGET_MISMATCH", activeRunId: "run-x" }),
|
||||
{ status: 409 },
|
||||
),
|
||||
),
|
||||
);
|
||||
await act(async () => {
|
||||
await h.state.transport!.fetch!("http://x", { method: "POST", body: "{}" });
|
||||
});
|
||||
// runFact is now the absorbed "run-x". SEND_LOCAL preserves it; a fresh Send now
|
||||
// then CAS-supersedes THAT run — surfacing runFact through the supersede body.
|
||||
fireEvent.click(screen.getByTestId("send-btn")); // SEND_LOCAL -> sending, local
|
||||
fireEvent.click(screen.getByTestId("queue-btn")); // Y
|
||||
fireEvent.click(screen.getByLabelText("Send now")); // CAS -> arms pendingSupersede
|
||||
const { body } = h.state.transport!.prepareSendMessagesRequest!({
|
||||
messages: [],
|
||||
body: {},
|
||||
});
|
||||
// MUTATION-VERIFY: revert read409 to `runId` -> currentRunId undefined -> runFact
|
||||
// stays "run-1" -> the CAS targets "run-1" -> this assertion reddens.
|
||||
expect((body.supersede as { runId?: string } | undefined)?.runId).toBe("run-x");
|
||||
});
|
||||
|
||||
it("#497/S4: a plain 409 A_RUN_ALREADY_ACTIVE absorbs activeRunId into runFact so Send now CAS-targets the foreign run", async () => {
|
||||
startLocalStreamWithRun(); // sending, local, autonomous, runFact run-1
|
||||
// A plain (non-supersede) POST hits the one-active-run gate. The FSM must adopt
|
||||
// the server's activeRunId as the run-fact — NOT stay blind.
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn().mockResolvedValue(
|
||||
new Response(
|
||||
JSON.stringify({ code: "A_RUN_ALREADY_ACTIVE", activeRunId: "run-foreign" }),
|
||||
{ status: 409 },
|
||||
),
|
||||
),
|
||||
);
|
||||
// Drive a NON-supersede POST (phase is `sending`, not `superseding`).
|
||||
await act(async () => {
|
||||
await h.state.transport!.fetch!("http://x", { method: "POST", body: "{}" });
|
||||
});
|
||||
// runFact is now "run-foreign". SEND_LOCAL preserves it; Send now CAS-targets it.
|
||||
fireEvent.click(screen.getByTestId("send-btn")); // SEND_LOCAL -> sending, local
|
||||
fireEvent.click(screen.getByTestId("queue-btn")); // Y
|
||||
fireEvent.click(screen.getByLabelText("Send now")); // CAS -> arms pendingSupersede
|
||||
const { body } = h.state.transport!.prepareSendMessagesRequest!({
|
||||
messages: [],
|
||||
body: {},
|
||||
});
|
||||
// MUTATION-VERIFY: drop the activeRunId threading (or read the wrong field) ->
|
||||
// runFact stays "run-1" -> the CAS targets "run-1" -> this assertion reddens.
|
||||
expect((body.supersede as { runId?: string } | undefined)?.runId).toBe(
|
||||
"run-foreign",
|
||||
);
|
||||
});
|
||||
|
||||
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.
|
||||
|
||||
@@ -87,15 +87,19 @@ function isActiveRunStatus(status: string | null | undefined): boolean {
|
||||
return status === "pending" || status === "running";
|
||||
}
|
||||
|
||||
/** Read the `{ code, runId }` off a JSON error response WITHOUT consuming the
|
||||
* original body (reads a clone), so the caller can still return the Response
|
||||
* untouched to the SDK. Any parse failure => empty. */
|
||||
async function read409(response: Response): Promise<{ code?: string; runId?: string }> {
|
||||
/** Read the `{ code, activeRunId }` off a JSON error response WITHOUT consuming
|
||||
* the original body (reads a clone), so the caller can still return the Response
|
||||
* untouched to the SDK. `activeRunId` is the server's field for the run currently
|
||||
* active on the chat — it is the name emitted on BOTH 409 branches
|
||||
* (SUPERSEDE_TARGET_MISMATCH and A_RUN_ALREADY_ACTIVE, see ai-chat.controller.ts).
|
||||
* Reading `runId` here (the field the server never sends) silently yields
|
||||
* `undefined`. Any parse failure => empty. */
|
||||
async function read409(response: Response): Promise<{ code?: string; activeRunId?: string }> {
|
||||
try {
|
||||
const b = (await response.clone().json()) as { code?: unknown; runId?: unknown };
|
||||
const b = (await response.clone().json()) as { code?: unknown; activeRunId?: unknown };
|
||||
return {
|
||||
code: typeof b?.code === "string" ? b.code : undefined,
|
||||
runId: typeof b?.runId === "string" ? b.runId : undefined,
|
||||
activeRunId: typeof b?.activeRunId === "string" ? b.activeRunId : undefined,
|
||||
};
|
||||
} catch {
|
||||
return {};
|
||||
@@ -486,11 +490,11 @@ export default function ChatThread({
|
||||
if (response.ok) {
|
||||
dispatchRef.current({ type: "SUPERSEDE_READY", epoch: ep });
|
||||
} else if (response.status === 409) {
|
||||
const { code, runId } = await read409(response);
|
||||
const { code, activeRunId } = await read409(response);
|
||||
if (code === "SUPERSEDE_TARGET_MISMATCH")
|
||||
dispatchRef.current({
|
||||
type: "SUPERSEDE_MISMATCH",
|
||||
currentRunId: runId,
|
||||
currentRunId: activeRunId,
|
||||
epoch: ep,
|
||||
});
|
||||
else if (code === "SUPERSEDE_TIMEOUT")
|
||||
@@ -499,9 +503,12 @@ export default function ChatThread({
|
||||
dispatchRef.current({ type: "SUPERSEDE_INVALID", epoch: ep });
|
||||
}
|
||||
} else if (response.status === 409) {
|
||||
const { code } = await read409(response);
|
||||
const { code, activeRunId } = await read409(response);
|
||||
if (code === "A_RUN_ALREADY_ACTIVE")
|
||||
dispatchRef.current({ type: "RUN_ALREADY_ACTIVE" });
|
||||
// S4: thread the server's activeRunId into the event so the FSM can
|
||||
// adopt it as the run-fact — a later "Send now" then CAS-supersedes
|
||||
// that (possibly foreign-tab) run instead of a blind promote+abort.
|
||||
dispatchRef.current({ type: "RUN_ALREADY_ACTIVE", activeRunId });
|
||||
}
|
||||
return response;
|
||||
}
|
||||
|
||||
@@ -56,7 +56,7 @@ Legend: **†** = command-transition (bumps `epoch`, I1). Effects in `[…]`.
|
||||
| `SUPERSEDE_MISMATCH{currentRunId}` (409 SUPERSEDE_TARGET_MISMATCH) | superseding | error(supersede-mismatch) | `[postRun(verify)]`, runFact←currentRunId |
|
||||
| `SUPERSEDE_TIMEOUT` (409 SUPERSEDE_TIMEOUT) | superseding | error(supersede-timeout) | — (composer keeps text; no auto-retry) |
|
||||
| `SUPERSEDE_INVALID` (409 SUPERSEDE_INVALID) | superseding | error(supersede-invalid) | — |
|
||||
| `RUN_ALREADY_ACTIVE` (409 A_RUN_ALREADY_ACTIVE, plain POST) | sending | error(run-already-active) | — (composer offers supersede; NO auto-retry) |
|
||||
| `RUN_ALREADY_ACTIVE{activeRunId}` (409 A_RUN_ALREADY_ACTIVE, plain POST) | sending | error(run-already-active) | runFact←activeRunId (composer offers supersede; NO auto-retry) |
|
||||
| `DISPOSE` (unmount) | any | idle **†** | `[abortAttach, cancelReconnect, disarmPoll]` (I1/I5 — epoch++ kills late callbacks) |
|
||||
|
||||
**`stopping` honors any finish (re-review MEDIUM):** BEFORE the epoch filter, a
|
||||
@@ -90,8 +90,8 @@ share exactly the dispatched event set.
|
||||
|
||||
| Server response | Event dispatched | error kind → banner |
|
||||
|---|---|---|
|
||||
| 409 `A_RUN_ALREADY_ACTIVE` (plain POST) | `RUN_ALREADY_ACTIVE` | run-already-active → "already answering / interrupt & send" |
|
||||
| 409 `SUPERSEDE_TARGET_MISMATCH` (+ body.runId) | `SUPERSEDE_MISMATCH{currentRunId}` | supersede-mismatch → verify via /run |
|
||||
| 409 `A_RUN_ALREADY_ACTIVE` (+ body.activeRunId) | `RUN_ALREADY_ACTIVE{activeRunId}` | run-already-active → "already answering / interrupt & send" |
|
||||
| 409 `SUPERSEDE_TARGET_MISMATCH` (+ body.activeRunId) | `SUPERSEDE_MISMATCH{currentRunId}` | supersede-mismatch → verify via /run |
|
||||
| 409 `SUPERSEDE_TIMEOUT` | `SUPERSEDE_TIMEOUT` | supersede-timeout → "couldn't interrupt in time, resend" |
|
||||
| 409 `SUPERSEDE_INVALID` | `SUPERSEDE_INVALID` | supersede-invalid → "couldn't interrupt this run" |
|
||||
| 503 `A_RUN_BEGIN_FAILED` | `FINISH_ERROR{begin-failed}` | begin-failed → "could not start, temporary" |
|
||||
|
||||
@@ -309,6 +309,27 @@ describe("run-fsm — commit 5: supersede CAS + error classification", () => {
|
||||
expect(m.phase).toEqual({ name: "error", kind: "run-already-active" });
|
||||
expect(m.effects).toEqual([]);
|
||||
});
|
||||
|
||||
it("#497/S4: RUN_ALREADY_ACTIVE{activeRunId} ADOPTS the server's active run as the run-fact", () => {
|
||||
// The server sends `activeRunId` so a later supersede can TARGET that run
|
||||
// instead of a blind promote+abort. Absorb it into runFact.
|
||||
const m = reduce(run(initialMachine(), { type: "SEND_LOCAL" }), {
|
||||
type: "RUN_ALREADY_ACTIVE",
|
||||
activeRunId: "run-foreign",
|
||||
});
|
||||
expect(m.phase).toEqual({ name: "error", kind: "run-already-active" });
|
||||
expect(m.ctx.runFact).toEqual({ runId: "run-foreign" });
|
||||
expect(m.effects).toEqual([]);
|
||||
});
|
||||
|
||||
it("#497/S4: RUN_ALREADY_ACTIVE without an activeRunId keeps the prior run-fact", () => {
|
||||
const seeded = reduce(run(initialMachine(), { type: "SEND_LOCAL" }), {
|
||||
type: "RUN_FACT",
|
||||
runFact: { runId: "run-prior" },
|
||||
});
|
||||
const m = reduce(seeded, { type: "RUN_ALREADY_ACTIVE" });
|
||||
expect(m.ctx.runFact).toEqual({ runId: "run-prior" });
|
||||
});
|
||||
});
|
||||
|
||||
// #488 F2 — a late mount `getRun → ATTACH_START` must not hijack a local turn.
|
||||
|
||||
@@ -171,7 +171,7 @@ export type Event =
|
||||
| { type: "SUPERSEDE_MISMATCH"; currentRunId?: string; epoch?: number }
|
||||
| { type: "SUPERSEDE_TIMEOUT"; epoch?: number }
|
||||
| { type: "SUPERSEDE_INVALID"; epoch?: number }
|
||||
| { type: "RUN_ALREADY_ACTIVE" }
|
||||
| { type: "RUN_ALREADY_ACTIVE"; activeRunId?: string }
|
||||
// -- lifecycle --
|
||||
| { type: "DISPOSE" };
|
||||
|
||||
@@ -567,8 +567,13 @@ export function reduce(m: Machine, event: Event): Machine {
|
||||
|
||||
case "RUN_ALREADY_ACTIVE":
|
||||
// A plain POST hit the one-active-run gate. NO auto-retry — the composer
|
||||
// offers "interrupt and send" (supersede) instead.
|
||||
return to(m, { name: "error", kind: "run-already-active" });
|
||||
// offers "interrupt and send" (supersede) instead. #497/S4: adopt the
|
||||
// server's activeRunId as the run-fact so that supersede can TARGET the
|
||||
// (possibly foreign-tab) active run via the CAS, rather than a blind
|
||||
// promote+abort that just 409s again. A stale/absent id keeps the prior fact.
|
||||
return to(m, { name: "error", kind: "run-already-active" }, {
|
||||
ctx: { runFact: event.activeRunId ? { runId: event.activeRunId } : m.ctx.runFact },
|
||||
});
|
||||
|
||||
// ---- lifecycle -----------------------------------------------------
|
||||
case "DISPOSE":
|
||||
|
||||
@@ -56,8 +56,10 @@ describe("describeChatError", () => {
|
||||
});
|
||||
|
||||
it("classifies SUPERSEDE_TARGET_MISMATCH (409) as run-changed", () => {
|
||||
// Real server body shape: the current run id is `activeRunId` (NOT `runId`) —
|
||||
// see ai-chat.controller.ts. describeChatError classifies off `code` only.
|
||||
const body =
|
||||
'{"message":"active run does not match the supersede target","code":"SUPERSEDE_TARGET_MISMATCH","runId":"run-x","statusCode":409}';
|
||||
'{"message":"active run does not match the supersede target","code":"SUPERSEDE_TARGET_MISMATCH","activeRunId":"run-x","statusCode":409}';
|
||||
expect(describeChatError(body, t).title).toBe(
|
||||
"Couldn't interrupt — the run changed",
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user