Merge remote-tracking branch 'origin/main' into performance-improvements
This commit is contained in:
@@ -232,6 +232,7 @@ Rules:
|
||||
3. `session-ui-store.ts` should delegate to `session-actions.ts` for these mutations instead of duplicating SDK calls.
|
||||
4. Sending after a revert commits the new branch optimistically: remove the reverted tail and marker before inserting the new message, and restore both if the send is rejected.
|
||||
5. After session creation, the directory returned by the server is authoritative over the requested draft directory. The server may canonicalize a worktree path, and the first prompt must use the same directory identity as the created session.
|
||||
6. A prompt send that fails **after** the request left the client is ambiguous, never a definite failure: the server may already be answering it. Transports tag those errors (`markAmbiguousTransportFailure` in `@/lib/relay/transport-error`; the relay tunnel tags every stream that dies with a request in flight), and `isAmbiguousSendFailure` reads the tag before falling back to status/text heuristics. An ambiguous failure waits for the connection to return, refetches recent messages, and confirms the optimistic message in place instead of rolling it back — rolling it back lets the message queue re-send a prompt the engine is already running, producing two independent AI responses for one user message.
|
||||
|
||||
Examples of global-store updates performed in `session-actions.ts`:
|
||||
|
||||
|
||||
@@ -1014,6 +1014,53 @@ describe("optimisticSend target directory", () => {
|
||||
expect(targetStore.getState().part[sentMessageID]?.[0]?.id).toBe("server-part")
|
||||
})
|
||||
|
||||
// Relay tunnel aborts carry no HTTP status and no wording the text-matching
|
||||
// heuristic recognizes. Without the transport tag they were classified as
|
||||
// definite failures, the accepted prompt was rolled back, and the queue
|
||||
// re-sent a message the engine was already answering (#2425).
|
||||
test("confirms a tunnel-tagged transport failure that no text heuristic matches", async () => {
|
||||
const targetStore = createStore({})
|
||||
const childStores = createChildStores([["/target/project", targetStore]])
|
||||
let optimisticRemove: OptimisticRemoveCall | null = null
|
||||
let optimisticConfirm: OptimisticRemoveCall | null = null
|
||||
let sentMessageID = ""
|
||||
|
||||
const { markAmbiguousTransportFailure } = await import("@/lib/relay/transport-error")
|
||||
const { optimisticSend, setActionRefs, setOptimisticRefs } = await import("./session-actions")
|
||||
setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/target/project")
|
||||
setOptimisticRefs(
|
||||
() => {},
|
||||
(input) => {
|
||||
optimisticRemove = input
|
||||
},
|
||||
(input) => {
|
||||
optimisticConfirm = input
|
||||
},
|
||||
)
|
||||
|
||||
await optimisticSend({
|
||||
sessionId: "session-tunnel",
|
||||
directory: "/target/project",
|
||||
content: "hello",
|
||||
providerID: "provider",
|
||||
modelID: "model",
|
||||
send: async (messageID) => {
|
||||
sentMessageID = messageID
|
||||
sessionMessagesResult = {
|
||||
data: [{
|
||||
info: { id: messageID, role: "user", sessionID: "session-tunnel", time: { created: 1 } } as Message,
|
||||
parts: [{ id: "server-part", type: "text", text: "hello" } as Part],
|
||||
}],
|
||||
}
|
||||
throw markAmbiguousTransportFailure(new Error("stream aborted by host"))
|
||||
},
|
||||
})
|
||||
|
||||
expect(optimisticRemove).toBe(null)
|
||||
expect((optimisticConfirm as OptimisticRemoveCall | null)?.messageID).toBe(sentMessageID)
|
||||
expect(targetStore.getState().message["session-tunnel"]?.[0]?.id).toBe(sentMessageID)
|
||||
})
|
||||
|
||||
test("rolls back an ambiguous send failure when recent messages do not contain the sent ID", async () => {
|
||||
const targetStore = createStore({})
|
||||
const childStores = createChildStores([["/target/project", targetStore]])
|
||||
|
||||
@@ -29,11 +29,21 @@ import { withContextObligatoryMessage, type ContextObligatoryMessage } from "@/l
|
||||
import { getImperativeSessionMessageLoader } from "./session-message-loader"
|
||||
import { cleanupPersistedSessionState } from "./session-deletion-cleanup"
|
||||
import { getRuntimeKey } from "@/lib/runtime-switch"
|
||||
import { isAmbiguousTransportFailure } from "@/lib/relay/transport-error"
|
||||
|
||||
const MESSAGE_REFETCH_LIMIT = 100
|
||||
const SEND_CONFIRMATION_REFETCH_LIMIT = 30
|
||||
const SEND_CONFIRMATION_REFETCH_ATTEMPTS = 2
|
||||
const SEND_CONFIRMATION_REFETCH_RETRY_MS = 150
|
||||
// A relay-tunnel send fails when the tunnel drops, and the confirming refetch
|
||||
// then has to travel over that same tunnel to answer "did my message land?".
|
||||
// Two attempts 150ms apart always answered "no" on a remote connection, so an
|
||||
// accepted prompt looked like a failed one and got re-sent — two AI responses
|
||||
// for one user message. Wait for the connection to actually come back (an
|
||||
// authoritative signal, not a blind sleep), then retry with backoff. A healthy
|
||||
// connection skips the wait and answers on the first attempt.
|
||||
const SEND_CONFIRMATION_REFETCH_ATTEMPTS = 3
|
||||
const SEND_CONFIRMATION_REFETCH_BASE_RETRY_MS = 250
|
||||
const SEND_CONFIRMATION_RECONNECT_TIMEOUT_MS = 3000
|
||||
const SEND_CONFIRMATION_RECONNECT_POLL_MS = 100
|
||||
const MESSAGE_REFETCH_SKIP_PARTS = new Set(["patch", "step-start", "step-finish"])
|
||||
const UNREVERT_REFETCH_ATTEMPTS = 3
|
||||
const UNREVERT_REFETCH_RETRY_MS = 150
|
||||
@@ -360,6 +370,13 @@ function getErrorStatus(error: unknown): number | null {
|
||||
}
|
||||
|
||||
function isAmbiguousSendFailure(error: unknown): boolean {
|
||||
// Authoritative first: the transport that lost the request says whether it
|
||||
// had already been dispatched. The text matching below only covers direct
|
||||
// fetch/HTTP failures, whose wording we do not control either — relay tunnel
|
||||
// aborts ("stream aborted by host", "relay keepalive timeout", …) match none
|
||||
// of those patterns and used to be misread as definite failures.
|
||||
if (isAmbiguousTransportFailure(error)) return true
|
||||
|
||||
const status = getErrorStatus(error)
|
||||
if (status === 503 || status === 504 || status === 408) return true
|
||||
if (error instanceof TypeError) return true
|
||||
@@ -1255,8 +1272,15 @@ async function fetchRecentSendConfirmationRecords(
|
||||
messageID: string,
|
||||
directory?: string | null,
|
||||
): Promise<Array<{ info: Message; parts?: Part[] }> | null> {
|
||||
// Bounded: a connection that never returns must still let the send fail
|
||||
// rather than hang the composer.
|
||||
const reconnectDeadline = Date.now() + SEND_CONFIRMATION_RECONNECT_TIMEOUT_MS
|
||||
while (!useConfigStore.getState().isConnected && Date.now() < reconnectDeadline) {
|
||||
await wait(SEND_CONFIRMATION_RECONNECT_POLL_MS)
|
||||
}
|
||||
|
||||
for (let attempt = 0; attempt < SEND_CONFIRMATION_REFETCH_ATTEMPTS; attempt += 1) {
|
||||
if (attempt > 0) await wait(SEND_CONFIRMATION_REFETCH_RETRY_MS)
|
||||
if (attempt > 0) await wait(SEND_CONFIRMATION_REFETCH_BASE_RETRY_MS * 2 ** (attempt - 1))
|
||||
try {
|
||||
const result = await sdk().session.messages({
|
||||
sessionID: sessionId,
|
||||
|
||||
Reference in New Issue
Block a user