fix(chat): pin existing-session sends to captured target (#2424)
* fix(chat): pin sends to captured session * fix(chat): handle runtime cancellation consistently
This commit is contained in:
@@ -244,8 +244,9 @@ Rules:
|
||||
2. If an action targets a session by ID, resolve the **session's own directory**. Do not assume the current directory is correct.
|
||||
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.
|
||||
5. Composer and queued sends carry their captured runtime, directory, and session through asynchronous preparation. A runtime change cancels the send instead of re-resolving it against the new runtime.
|
||||
6. 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.
|
||||
7. 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`:
|
||||
|
||||
|
||||
@@ -1027,7 +1027,7 @@ describe("optimisticSend target directory", () => {
|
||||
expect(targetStore.getState().part.msg_2).toEqual([revertedPart])
|
||||
})
|
||||
|
||||
test("allows callers to block final send when runtime changes after optimistic insert", async () => {
|
||||
test("rolls back a captured send when the runtime changes after optimistic insert", async () => {
|
||||
const targetStore = createStore({})
|
||||
const childStores = createChildStores([["/target/project", targetStore]])
|
||||
let optimisticAdd: OptimisticAddCall | null = null
|
||||
@@ -1052,15 +1052,15 @@ describe("optimisticSend target directory", () => {
|
||||
await optimisticSend({
|
||||
sessionId: "session-race",
|
||||
directory: "/target/project",
|
||||
runtimeKey: "runtime-a",
|
||||
content: "hello",
|
||||
providerID: "provider",
|
||||
modelID: "model",
|
||||
beforeOptimisticInsert: () => {
|
||||
onOptimisticInsert: () => {
|
||||
expect(getRuntimeKey()).toBe("runtime-a")
|
||||
switchRuntimeEndpoint({ apiBaseUrl: "http://runtime-b.test", runtimeKey: "runtime-b" })
|
||||
},
|
||||
send: async () => {
|
||||
switchRuntimeEndpoint({ apiBaseUrl: "http://runtime-b.test", runtimeKey: "runtime-b" })
|
||||
if (getRuntimeKey() !== "runtime-a") throw new Error("Auto-review stopped because the runtime changed.")
|
||||
finalSendCalled = true
|
||||
},
|
||||
})
|
||||
|
||||
@@ -1257,6 +1257,7 @@ function ascendingId(prefix: string): string {
|
||||
* handles deduplication when the server echoes back the real message.
|
||||
*/
|
||||
export async function optimisticSend(input: {
|
||||
runtimeKey?: string
|
||||
sessionId: string
|
||||
content: string
|
||||
providerID: string
|
||||
@@ -1273,9 +1274,20 @@ export async function optimisticSend(input: {
|
||||
if (!_optimisticAdd || !_optimisticRemove) {
|
||||
throw new Error("Optimistic refs not set — is useSync() mounted?")
|
||||
}
|
||||
const optimisticAdd = _optimisticAdd
|
||||
const optimisticRemove = _optimisticRemove
|
||||
const optimisticConfirm = _optimisticConfirm
|
||||
|
||||
const assertRuntimeUnchanged = () => {
|
||||
if (input.runtimeKey && input.runtimeKey !== getRuntimeKey()) {
|
||||
throw new Error("Message was not sent because the runtime changed.")
|
||||
}
|
||||
}
|
||||
|
||||
assertRuntimeUnchanged()
|
||||
await waitForConnectionOrThrow()
|
||||
input.beforeOptimisticInsert?.()
|
||||
assertRuntimeUnchanged()
|
||||
|
||||
const targetDirectory = input.directory ?? dir()
|
||||
const store = targetDirectory ? dirStoreForDirectory(targetDirectory) : dirStore()
|
||||
@@ -1341,7 +1353,7 @@ export async function optimisticSend(input: {
|
||||
} as unknown as Message
|
||||
|
||||
// Insert into store + register in shadow Map (for mergeOptimisticPage cleanup)
|
||||
_optimisticAdd({
|
||||
optimisticAdd({
|
||||
sessionID: input.sessionId,
|
||||
directory: targetDirectory,
|
||||
message: optimisticMessage,
|
||||
@@ -1359,6 +1371,7 @@ export async function optimisticSend(input: {
|
||||
})
|
||||
|
||||
try {
|
||||
assertRuntimeUnchanged()
|
||||
await input.send(messageID)
|
||||
} catch (error) {
|
||||
const status = getErrorStatus(error)
|
||||
@@ -1369,7 +1382,7 @@ export async function optimisticSend(input: {
|
||||
|
||||
if (acceptedRecords) {
|
||||
materializeConfirmedSendRecords(store, input.sessionId, messageID, acceptedRecords)
|
||||
_optimisticConfirm?.({
|
||||
optimisticConfirm?.({
|
||||
sessionID: input.sessionId,
|
||||
directory: targetDirectory,
|
||||
messageID,
|
||||
@@ -1396,7 +1409,7 @@ export async function optimisticSend(input: {
|
||||
console.warn("[session-actions] prompt send rejected; rolling back optimistic message", failureRecord)
|
||||
|
||||
// Rollback via optimistic infrastructure
|
||||
_optimisticRemove({
|
||||
optimisticRemove({
|
||||
sessionID: input.sessionId,
|
||||
directory: targetDirectory,
|
||||
messageID,
|
||||
|
||||
@@ -8,6 +8,7 @@ import { setActionRefs, setOptimisticRefs } from './session-actions';
|
||||
import { useSkillsStore } from '@/stores/useSkillsStore';
|
||||
import { useCommandsStore } from '@/stores/useCommandsStore';
|
||||
import { useConfigStore } from '@/stores/useConfigStore';
|
||||
import { getRuntimeKey } from '@/lib/runtime-switch';
|
||||
|
||||
/**
|
||||
* Unit tests for session worktree routing through the authoritative store.
|
||||
@@ -228,6 +229,85 @@ describe('routeMessage directory scoping', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('sendMessage captured target', () => {
|
||||
let originalSendMessage;
|
||||
const calls = [];
|
||||
|
||||
beforeEach(() => {
|
||||
calls.length = 0;
|
||||
const childStore = {
|
||||
getState: () => ({ session: [], message: {}, part: {}, session_status: {} }),
|
||||
setState: () => {},
|
||||
};
|
||||
const childStores = {
|
||||
children: new Map(),
|
||||
ensureChild: () => childStore,
|
||||
getChild: () => childStore,
|
||||
};
|
||||
setActionRefs(opencodeClient, childStores, () => '/current/project');
|
||||
setOptimisticRefs(() => {}, () => {});
|
||||
useConfigStore.setState({ isConnected: true });
|
||||
useSessionUIStore.setState({
|
||||
currentSessionId: 'session-current',
|
||||
currentSessionDirectory: '/current/project',
|
||||
newSessionDraft: { open: false, directoryOverride: null, parentID: null },
|
||||
});
|
||||
|
||||
originalSendMessage = opencodeClient.sendMessage;
|
||||
opencodeClient.sendMessage = async (params) => {
|
||||
calls.push(params);
|
||||
return 'msg';
|
||||
};
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
opencodeClient.sendMessage = originalSendMessage;
|
||||
});
|
||||
|
||||
const sendToTarget = (target) => useSessionUIStore.getState().sendMessage(
|
||||
'queued message',
|
||||
'provider-a',
|
||||
'model-a',
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
'normal',
|
||||
{ target },
|
||||
);
|
||||
|
||||
test('uses the target captured before the active session changes', async () => {
|
||||
await sendToTarget({
|
||||
runtimeKey: getRuntimeKey(),
|
||||
sessionId: 'session-captured',
|
||||
directory: '/captured/project',
|
||||
});
|
||||
|
||||
expect(calls).toHaveLength(1);
|
||||
expect(calls[0].runtimeKey).toBe(getRuntimeKey());
|
||||
expect(calls[0].id).toBe('session-captured');
|
||||
expect(calls[0].directory).toBe('/captured/project');
|
||||
});
|
||||
|
||||
test('does not send a captured target through a different runtime', async () => {
|
||||
let error = null;
|
||||
try {
|
||||
await sendToTarget({
|
||||
runtimeKey: `${getRuntimeKey()}-stale`,
|
||||
sessionId: 'session-captured',
|
||||
directory: '/captured/project',
|
||||
});
|
||||
} catch (caught) {
|
||||
error = caught;
|
||||
}
|
||||
|
||||
expect(error).toBeInstanceOf(Error);
|
||||
expect(error.message).toContain('runtime changed');
|
||||
expect(calls).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('slash-command goal objectives', () => {
|
||||
test('expands every $ARGUMENTS reference from the authoritative command template', () => {
|
||||
expect(expandSlashCommandGoalObjective('/issue--to-pr LIN-123 --draft', [{
|
||||
@@ -370,7 +450,12 @@ describe('routeMessage skill invocation', () => {
|
||||
|
||||
// Minimal optimistic + connection machinery so routeMessage can dispatch.
|
||||
const childStore = {
|
||||
getState: () => ({ session_status: {} }),
|
||||
getState: () => ({
|
||||
session: [],
|
||||
message: {},
|
||||
part: {},
|
||||
session_status: {},
|
||||
}),
|
||||
setState: () => {},
|
||||
};
|
||||
const childStores = {
|
||||
|
||||
@@ -122,6 +122,7 @@ export function expandSlashCommandGoalObjective(content: string, commands: GoalC
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
export function routeMessage(params: {
|
||||
runtimeKey?: string
|
||||
sessionId: string
|
||||
directory?: string | null
|
||||
content: string
|
||||
@@ -138,6 +139,7 @@ export function routeMessage(params: {
|
||||
const requestDirectory = params.directory ?? undefined
|
||||
if (params.inputMode === "shell") {
|
||||
return opencodeClient.shellSession({
|
||||
runtimeKey: params.runtimeKey,
|
||||
sessionId: params.sessionId,
|
||||
directory: requestDirectory,
|
||||
agent: params.agent ?? "",
|
||||
@@ -166,6 +168,7 @@ export function routeMessage(params: {
|
||||
|
||||
if (isCommand) {
|
||||
return optimisticSend({
|
||||
runtimeKey: params.runtimeKey,
|
||||
sessionId: params.sessionId,
|
||||
content: params.content,
|
||||
providerID: params.providerID,
|
||||
@@ -174,6 +177,7 @@ export function routeMessage(params: {
|
||||
directory: requestDirectory,
|
||||
files: params.files,
|
||||
send: (messageID) => opencodeClient.sendCommand({
|
||||
runtimeKey: params.runtimeKey,
|
||||
id: params.sessionId,
|
||||
providerID: params.providerID,
|
||||
modelID: params.modelID,
|
||||
@@ -191,6 +195,7 @@ export function routeMessage(params: {
|
||||
|
||||
// Normal prompt — optimistic insert so message appears instantly
|
||||
return optimisticSend({
|
||||
runtimeKey: params.runtimeKey,
|
||||
sessionId: params.sessionId,
|
||||
content: params.content,
|
||||
providerID: params.providerID,
|
||||
@@ -199,6 +204,7 @@ export function routeMessage(params: {
|
||||
directory: requestDirectory,
|
||||
files: params.files,
|
||||
send: (messageID) => opencodeClient.sendMessage({
|
||||
runtimeKey: params.runtimeKey,
|
||||
id: params.sessionId,
|
||||
providerID: params.providerID,
|
||||
modelID: params.modelID,
|
||||
@@ -215,7 +221,14 @@ export function routeMessage(params: {
|
||||
})
|
||||
}
|
||||
|
||||
type CapturedSendTarget = {
|
||||
runtimeKey: string
|
||||
sessionId: string
|
||||
directory: string
|
||||
}
|
||||
|
||||
type SendMessageOptions = {
|
||||
target?: CapturedSendTarget
|
||||
sessionId?: string
|
||||
directory?: string
|
||||
delivery?: 'steer'
|
||||
@@ -1198,8 +1211,13 @@ export const useSessionUIStore = create<SessionUIState>()((set, get) => ({
|
||||
inputMode?: "normal" | "shell",
|
||||
options?: SendMessageOptions,
|
||||
) => {
|
||||
const capturedTarget = options?.target
|
||||
if (capturedTarget && capturedTarget.runtimeKey !== getRuntimeKey()) {
|
||||
throw new Error("Message was not sent because the runtime changed.")
|
||||
}
|
||||
|
||||
// Clear non-Git changed-files bar on new user message for current session
|
||||
const sid = options?.sessionId ?? get().currentSessionId;
|
||||
const sid = capturedTarget?.sessionId ?? options?.sessionId ?? get().currentSessionId;
|
||||
if (sid) {
|
||||
const map = new Map(get().pendingChangesBarDismissed);
|
||||
map.delete(sid);
|
||||
@@ -1258,7 +1276,7 @@ export const useSessionUIStore = create<SessionUIState>()((set, get) => ({
|
||||
}
|
||||
|
||||
// ---- New session from draft ----
|
||||
if (!options?.sessionId && draft?.open) {
|
||||
if (!capturedTarget && !options?.sessionId && draft?.open) {
|
||||
const createdDraftSession = await materializeOpenDraftSession({
|
||||
providerID,
|
||||
modelID,
|
||||
@@ -1310,7 +1328,7 @@ export const useSessionUIStore = create<SessionUIState>()((set, get) => ({
|
||||
}
|
||||
|
||||
// ---- Existing session ----
|
||||
const targetSessionId = options?.sessionId ?? get().currentSessionId
|
||||
const targetSessionId = capturedTarget?.sessionId ?? options?.sessionId ?? get().currentSessionId
|
||||
const sessionAgentSelection = targetSessionId
|
||||
? useSelectionStore.getState().getSessionAgentSelection(targetSessionId)
|
||||
: null
|
||||
@@ -1345,7 +1363,7 @@ export const useSessionUIStore = create<SessionUIState>()((set, get) => ({
|
||||
}
|
||||
|
||||
const currentSessionDirectory = targetSessionId
|
||||
? normalizePath(options?.directory ?? get().getDirectoryForSession(targetSessionId))
|
||||
? normalizePath(capturedTarget?.directory ?? options?.directory ?? get().getDirectoryForSession(targetSessionId))
|
||||
: null
|
||||
if (targetSessionId) {
|
||||
notifyMessageSent(targetSessionId)
|
||||
@@ -1366,6 +1384,7 @@ export const useSessionUIStore = create<SessionUIState>()((set, get) => ({
|
||||
await applyArmedGoal(targetSessionId, currentSessionDirectory)
|
||||
}
|
||||
await routeMessage({
|
||||
runtimeKey: capturedTarget?.runtimeKey,
|
||||
sessionId: targetSessionId || "",
|
||||
directory: currentSessionDirectory,
|
||||
content,
|
||||
|
||||
Reference in New Issue
Block a user