fix: retire stale composer recovery drafts before retry
A failed-response Retry submits no composer text. Retire the earlier accepted text send's recovery fallback synchronously so a Retry model/key refusal cannot restore it over the current unsent draft. Never clear a later ordinary send's fallback at async settlement. Add key/model503, superseding-draft and ordinary-refusal coverage. Verified 183 focused tests, typecheck, full ESLint, production build, and a red/green live production regression plus ordinary-draft recovery; no external provider calls.
This commit is contained in:
+13
-1
@@ -162,6 +162,18 @@ export function parseProviderKeyRefusal(message: string | undefined): boolean {
|
||||
}
|
||||
}
|
||||
|
||||
/** Retry sends no composer text. Its refusal must not resurrect the draft
|
||||
* from an earlier, already-durable text send over today's unsent composer.
|
||||
* Retire that fallback synchronously, never when the async Retry settles:
|
||||
* a later ordinary send may own a new recovery draft by then. */
|
||||
export function retryFailedResponse(
|
||||
send: (input: ChatInput) => void,
|
||||
submittedDraft: { current: string | null },
|
||||
): void {
|
||||
submittedDraft.current = null;
|
||||
void send({ type: "retry" });
|
||||
}
|
||||
|
||||
/** useLayoutEffect that is silent during SSR (client components still render on the server in Next). */
|
||||
const useIsoLayoutEffect = typeof window === "undefined" ? useEffect : useLayoutEffect;
|
||||
|
||||
@@ -676,7 +688,7 @@ export function Chat({
|
||||
// "error" too: a dropped stream leaves the model there, and send()
|
||||
// recovers from it. Later compaction requires new input instead.
|
||||
const retryable = (status === "ready" || status === "error") && !busy && !compacting && endsInFailedTurn(nativeMessages);
|
||||
const retryFailedTurn = () => void send({ type: "retry" });
|
||||
const retryFailedTurn = () => retryFailedResponse(send, draftRef);
|
||||
const canSend =
|
||||
(text.trim().length > 0 || dictation.active) && !pending && !activeProblem && !compacting;
|
||||
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { describe, it } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { parseModelsRefusal, parseProviderKeyRefusal } from "../components/chat.tsx";
|
||||
import { parseModelsRefusal, parseProviderKeyRefusal, retryFailedResponse } from "../components/chat.tsx";
|
||||
import { SessionModel } from "../components/session-core.ts";
|
||||
import { fauxAssistantMessage } from "@earendil-works/pi-ai";
|
||||
import { newUserMessage, type LhMessage } from "../src/core/native-messages.ts";
|
||||
import { prefillFromSlots, prefillLevelFromPin, setupStage } from "../components/setup-dialog.tsx";
|
||||
import {
|
||||
isModelsRefusal,
|
||||
@@ -39,6 +42,64 @@ describe("parseModelsRefusal — the send refusal detector", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("Retry owns no composer recovery draft", () => {
|
||||
for (const envelope of [providerKeyMissing("openrouter", ["strong"]), modelsNotConfigured(["strong"])]) {
|
||||
it(`accepted A / failed model / unsent B / Retry ${envelope.code} retires only A's fallback`, async () => {
|
||||
const durable: LhMessage[] = [
|
||||
newUserMessage("Accepted text A"),
|
||||
{ ...fauxAssistantMessage("Partial reply", { stopReason: "error", errorMessage: "Model failed" }), id: "failed-A" },
|
||||
];
|
||||
const submittedDraft: { current: string | null } = { current: "Accepted text A" };
|
||||
let composer = "Unsent text B";
|
||||
const body = JSON.stringify(envelope);
|
||||
const model = new SessionModel({
|
||||
chatId: "fixture", initialMessages: durable,
|
||||
fetch: async (_url, init) => {
|
||||
assert.equal(submittedDraft.current, null, "retire A before Retry can reach a refusal");
|
||||
assert.deepEqual(JSON.parse(String(init?.body)).input, { type: "retry" });
|
||||
return new Response(body, { status: 503 });
|
||||
},
|
||||
});
|
||||
let request!: Promise<void>;
|
||||
retryFailedResponse((input) => request = model.send(input), submittedDraft);
|
||||
await request;
|
||||
assert.equal(model.getState().error?.message, body, "503 stays raw for existing setup/picker routing");
|
||||
assert.ok(parseModelsRefusal(body) || parseProviderKeyRefusal(body));
|
||||
// The existing refusal effect restores only an owned submitted
|
||||
// draft. Retry must leave this slot empty, not replace B with A.
|
||||
if (submittedDraft.current !== null) composer = submittedDraft.current;
|
||||
assert.equal(composer, "Unsent text B");
|
||||
assert.deepEqual(model.getState().nativeMessages, durable, "Retry adds no optimistic learner message");
|
||||
});
|
||||
}
|
||||
|
||||
it("does not clear a later ordinary send's recovery draft when Retry settles", async () => {
|
||||
const submittedDraft: { current: string | null } = { current: "Accepted text A" };
|
||||
let finish!: () => void;
|
||||
const pending = new Promise<void>((resolve) => { finish = resolve; });
|
||||
retryFailedResponse(async () => pending, submittedDraft);
|
||||
assert.equal(submittedDraft.current, null);
|
||||
submittedDraft.current = "Later ordinary text C";
|
||||
finish();
|
||||
await pending;
|
||||
await Promise.resolve();
|
||||
assert.equal(submittedDraft.current, "Later ordinary text C", "no asynchronous cleanup may retire a newer owner");
|
||||
});
|
||||
|
||||
it("ordinary text refusals still keep their submitted draft for recovery", async () => {
|
||||
const submittedDraft: { current: string | null } = { current: "Ordinary text C" };
|
||||
const body = JSON.stringify(providerKeyMissing("openrouter", ["strong"]));
|
||||
const model = new SessionModel({
|
||||
chatId: "fixture", initialMessages: [],
|
||||
fetch: async () => new Response(body, { status: 503 }),
|
||||
});
|
||||
await model.send({ type: "text", text: submittedDraft.current! });
|
||||
assert.equal(submittedDraft.current, "Ordinary text C");
|
||||
assert.equal(model.getState().error?.message, body);
|
||||
assert.equal(parseProviderKeyRefusal(model.getState().error?.message), true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isModelsRefusal — the job-record matcher", () => {
|
||||
it("matches the runner's composed wording, not real failures", () => {
|
||||
assert.equal(isModelsRefusal(`${modelsNotConfigured(["vision"]).error} (missing: vision)`), true);
|
||||
|
||||
Reference in New Issue
Block a user