fix(sync): honor expectedRuntimeKey in archive actions
`useSessionUIStore.archiveSessions` declared an `options` parameter and discarded it, so any caller passing a captured runtime key got a silent no-op. The archive path also never rechecked the runtime, letting a response produced by a previous runtime reconcile the live and global session stores of the runtime the user had switched to. Move the batch to a canonical `archiveSessions()` action, add an optional `expectedRuntimeKey` to `archiveSession()`, `patchSessionMetadata()`, and `cleanupReviewMetadataBeforeDelete()`, and recheck that key before every store reconciliation. A guarded batch stops at the first observed runtime change: server-confirmed sessions stay in `archivedIds` and every unconfirmed ID is returned in `failedIds`, so existing partial-failure feedback stays truthful. Callers that pass no key keep prior behavior. Type the store option as `ArchiveSessionsOptions` instead of `Record<string, unknown>`, since the loose type allowed the drop.
This commit is contained in:
@@ -213,10 +213,20 @@ Examples of global-store updates performed in `session-actions.ts`:
|
||||
- `createSession()` -> `upsertSession(session)`
|
||||
- `updateSessionTitle()` -> `upsertSession(result.data)`
|
||||
- `shareSession()` / `unshareSession()` -> `upsertSession(result.data)`
|
||||
- `archiveSession()` -> waits for server confirmation, then upserts the archived session
|
||||
- `archiveSession()` / `archiveSessions()` -> wait for server confirmation, then upsert each archived session
|
||||
- `deleteSession()` -> waits for server confirmation or `404`, then removes the session and its persisted state
|
||||
- `moveSessionToDirectory()` -> move the session between directory stores and update the global directory index
|
||||
|
||||
Archive callers whose confirmation spans asynchronous SDK calls may capture the
|
||||
runtime key at operation start and pass it as `expectedRuntimeKey`. The guarded
|
||||
action rechecks that key before every store reconciliation, so a response
|
||||
produced by the previous runtime is rejected instead of mutating the current
|
||||
runtime's live or global session state. A guarded batch stops at the first
|
||||
observed runtime change: sessions the server already confirmed remain archived
|
||||
and stay in `archivedIds`, while every ID not confirmed on the captured runtime
|
||||
is returned in `failedIds` so existing partial-failure feedback stays truthful.
|
||||
Callers that pass no runtime key keep the previous unguarded behavior.
|
||||
|
||||
## The golden rule
|
||||
|
||||
When creating a draft in `handleDirectoryEvent`, **only clone the state fields the event will mutate**. Never spread all fields eagerly.
|
||||
|
||||
@@ -14,6 +14,7 @@ let sessionShareResult: { data?: unknown; error?: unknown; response?: { status?:
|
||||
let sessionUpdateResult: { data?: unknown; error?: unknown; response?: { status?: number } } = {}
|
||||
let sessionMessagesResult: { data?: unknown; error?: unknown; response?: { status?: number } } = { data: [] }
|
||||
let sessionDeleteError: unknown | null = null
|
||||
let beforeSessionUpdateResolve: ((sessionId: string) => void) | null = null
|
||||
const globalUpsertedSessions: unknown[] = []
|
||||
const globalRemovedSessionIds: string[] = []
|
||||
const deletedCleanupIdentities: Array<{ runtimeKey: string; directory: string; sessionId: string }> = []
|
||||
@@ -145,6 +146,9 @@ mock.module("@/lib/opencode/client", () => ({
|
||||
}),
|
||||
updateSession: mock((sessionId: string, changes: Record<string, unknown>, directory?: string | null) => {
|
||||
replyCalls.push({ method: "session.update", params: { sessionID: sessionId, ...changes, directory } })
|
||||
// Lets a test mutate global runtime state while the SDK call is in flight,
|
||||
// so the action observes the switch only after awaiting the response.
|
||||
beforeSessionUpdateResolve?.(sessionId)
|
||||
return Promise.resolve(sessionUpdateResult.data)
|
||||
}),
|
||||
deleteSession: mock((sessionId: string, directory?: string | null) => {
|
||||
@@ -368,6 +372,7 @@ describe("confirmed session removal", () => {
|
||||
deletedCleanupIdentities.length = 0
|
||||
sessionDeleteError = null
|
||||
sessionUpdateResult = {}
|
||||
beforeSessionUpdateResolve = null
|
||||
})
|
||||
|
||||
test("does not remove live or persisted state when delete fails", async () => {
|
||||
@@ -427,6 +432,86 @@ describe("confirmed session removal", () => {
|
||||
expect(source.getState().session).toEqual([])
|
||||
expect((globalUpsertedSessions[0] as Session)?.time?.archived).toBe(2)
|
||||
})
|
||||
|
||||
test("rejects an archive response that arrives after a runtime switch", async () => {
|
||||
sessionUpdateResult = {
|
||||
data: { id: "session-a", directory: "/test/project", time: { created: 1, archived: 2 } } as Session,
|
||||
}
|
||||
const source = createStore({}, {
|
||||
session: [{ id: "session-a", directory: "/test/project", time: { created: 1 } } as Session],
|
||||
})
|
||||
const { getRuntimeKey, switchRuntimeEndpoint } = await import("../lib/runtime-switch")
|
||||
switchRuntimeEndpoint({ apiBaseUrl: "http://archive-runtime-a.test", runtimeKey: "archive-runtime-a" })
|
||||
beforeSessionUpdateResolve = () => {
|
||||
switchRuntimeEndpoint({ apiBaseUrl: "http://archive-runtime-b.test", runtimeKey: "archive-runtime-b" })
|
||||
}
|
||||
const { archiveSession, setActionRefs } = await import("./session-actions")
|
||||
setActionRefs(mockSdk as unknown as OpencodeClient, createChildStores([["/test/project", source]]), () => "/test/project")
|
||||
|
||||
expect(await archiveSession("session-a", "archive-runtime-a")).toBe(false)
|
||||
expect(getRuntimeKey()).toBe("archive-runtime-b")
|
||||
// The stale response must not reconcile the runtime the user switched to.
|
||||
expect(source.getState().session.map((item) => item.id)).toEqual(["session-a"])
|
||||
expect(globalUpsertedSessions).toEqual([])
|
||||
})
|
||||
|
||||
test("keeps confirmed sessions and fails the rest when the runtime changes mid-batch", async () => {
|
||||
sessionUpdateResult = {
|
||||
data: { id: "session-a", directory: "/test/project", time: { created: 1, archived: 2 } } as Session,
|
||||
}
|
||||
const source = createStore({}, {
|
||||
session: [
|
||||
{ id: "session-a", directory: "/test/project", time: { created: 1 } } as Session,
|
||||
{ id: "session-b", directory: "/test/project", time: { created: 1 } } as Session,
|
||||
{ id: "session-c", directory: "/test/project", time: { created: 1 } } as Session,
|
||||
],
|
||||
})
|
||||
const { switchRuntimeEndpoint } = await import("../lib/runtime-switch")
|
||||
switchRuntimeEndpoint({ apiBaseUrl: "http://archive-batch-a.test", runtimeKey: "archive-batch-a" })
|
||||
beforeSessionUpdateResolve = (sessionId) => {
|
||||
if (sessionId === "session-b") {
|
||||
switchRuntimeEndpoint({ apiBaseUrl: "http://archive-batch-b.test", runtimeKey: "archive-batch-b" })
|
||||
}
|
||||
}
|
||||
const { archiveSessions, setActionRefs } = await import("./session-actions")
|
||||
setActionRefs(mockSdk as unknown as OpencodeClient, createChildStores([["/test/project", source]]), () => "/test/project")
|
||||
|
||||
const result = await archiveSessions(["session-a", "session-b", "session-c"], {
|
||||
expectedRuntimeKey: "archive-batch-a",
|
||||
})
|
||||
|
||||
// session-a was confirmed before the switch and stays archived; session-b's
|
||||
// response is stale and session-c is never attempted, so both are reported
|
||||
// as failures instead of being silently dropped.
|
||||
expect(result).toEqual({ archivedIds: ["session-a"], failedIds: ["session-b", "session-c"] })
|
||||
expect(source.getState().session.map((item) => item.id)).toEqual(["session-b", "session-c"])
|
||||
expect(globalUpsertedSessions).toHaveLength(1)
|
||||
// session-c must not reach the SDK after the runtime changed.
|
||||
expect(replyCalls.filter((call) => call.method === "session.update").map((call) => call.params.sessionID))
|
||||
.toEqual(["session-a", "session-b"])
|
||||
})
|
||||
|
||||
test("archives every session when the runtime stays stable", async () => {
|
||||
sessionUpdateResult = {
|
||||
data: { id: "session-a", directory: "/test/project", time: { created: 1, archived: 2 } } as Session,
|
||||
}
|
||||
const source = createStore({}, {
|
||||
session: [
|
||||
{ id: "session-a", directory: "/test/project", time: { created: 1 } } as Session,
|
||||
{ id: "session-b", directory: "/test/project", time: { created: 1 } } as Session,
|
||||
],
|
||||
})
|
||||
const { getRuntimeKey } = await import("../lib/runtime-switch")
|
||||
const { archiveSessions, setActionRefs } = await import("./session-actions")
|
||||
setActionRefs(mockSdk as unknown as OpencodeClient, createChildStores([["/test/project", source]]), () => "/test/project")
|
||||
|
||||
const result = await archiveSessions(["session-a", "session-b"], {
|
||||
expectedRuntimeKey: getRuntimeKey(),
|
||||
})
|
||||
|
||||
expect(result).toEqual({ archivedIds: ["session-a", "session-b"], failedIds: [] })
|
||||
expect(source.getState().session).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe("fetchMessagesForSession startup race", () => {
|
||||
|
||||
@@ -654,15 +654,29 @@ export async function createSession(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* True when a caller captured a runtime key before an asynchronous mutation and
|
||||
* that runtime is no longer the active one. Callers pass `undefined` when they
|
||||
* do not participate in runtime-scoped guarding, which keeps the previous
|
||||
* unguarded behavior.
|
||||
*/
|
||||
function isStaleRuntime(expectedRuntimeKey: string | undefined): boolean {
|
||||
return expectedRuntimeKey !== undefined && getRuntimeKey() !== expectedRuntimeKey
|
||||
}
|
||||
|
||||
export async function patchSessionMetadata(
|
||||
sessionId: string,
|
||||
directory: string | null | undefined,
|
||||
updater: (metadata: SessionMetadataRecord) => SessionMetadataRecord,
|
||||
expectedRuntimeKey?: string,
|
||||
): Promise<Session> {
|
||||
if (isStaleRuntime(expectedRuntimeKey)) throw new Error("runtime changed")
|
||||
const targetDirectory = directory ?? getSessionDirectory(sessionId)
|
||||
const current = await opencodeClient.getSession(sessionId, targetDirectory)
|
||||
if (isStaleRuntime(expectedRuntimeKey)) throw new Error("runtime changed")
|
||||
const nextMetadata = updater(getSessionMetadata(current))
|
||||
const updated = await opencodeClient.updateSession(sessionId, { metadata: nextMetadata }, targetDirectory)
|
||||
if (isStaleRuntime(expectedRuntimeKey)) throw new Error("runtime changed")
|
||||
useGlobalSessionsStore.getState().upsertSession(updated)
|
||||
const sessionDirectory = (updated as { directory?: string | null }).directory ?? targetDirectory
|
||||
if (sessionDirectory) registerSessionDirectory(updated.id, sessionDirectory)
|
||||
@@ -682,19 +696,26 @@ export async function setContextObligatoryMessage(
|
||||
return updated
|
||||
}
|
||||
|
||||
async function cleanupReviewMetadataBeforeDelete(sessionId: string, directory?: string | null): Promise<void> {
|
||||
async function cleanupReviewMetadataBeforeDelete(
|
||||
sessionId: string,
|
||||
directory?: string | null,
|
||||
expectedRuntimeKey?: string,
|
||||
): Promise<void> {
|
||||
if (isStaleRuntime(expectedRuntimeKey)) return
|
||||
let session: Session
|
||||
try {
|
||||
session = await opencodeClient.getSession(sessionId, directory ?? getSessionDirectory(sessionId))
|
||||
} catch {
|
||||
return
|
||||
}
|
||||
if (isStaleRuntime(expectedRuntimeKey)) return
|
||||
if (!isReviewSession(session)) return
|
||||
const originalSessionID = getOriginalSessionID(session)
|
||||
if (!originalSessionID) return
|
||||
try {
|
||||
await patchSessionMetadata(originalSessionID, directory ?? getSessionDirectory(originalSessionID), (metadata) =>
|
||||
withoutReviewSessionLink(metadata, sessionId),
|
||||
expectedRuntimeKey,
|
||||
)
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : String(error)
|
||||
@@ -803,12 +824,26 @@ export async function deleteSessionInDirectory(sessionId: string, directory: str
|
||||
}
|
||||
}
|
||||
|
||||
export async function archiveSession(sessionId: string): Promise<boolean> {
|
||||
/**
|
||||
* Archive one session.
|
||||
*
|
||||
* `expectedRuntimeKey` is the runtime key the caller captured when the user
|
||||
* confirmed the operation. When it is supplied and the runtime changes, the
|
||||
* action stops and returns `false` without reconciling any store, so a response
|
||||
* produced by the previous runtime cannot mutate the current runtime's live or
|
||||
* global session state. A session the server already archived before the switch
|
||||
* stays archived on that runtime and is re-read from the server the next time
|
||||
* the runtime is loaded.
|
||||
*/
|
||||
export async function archiveSession(sessionId: string, expectedRuntimeKey?: string): Promise<boolean> {
|
||||
if (isStaleRuntime(expectedRuntimeKey)) return false
|
||||
const sessionDirectory = getSessionDirectory(sessionId)
|
||||
const archivedAt = Date.now()
|
||||
try {
|
||||
await cleanupReviewMetadataBeforeDelete(sessionId, sessionDirectory)
|
||||
await cleanupReviewMetadataBeforeDelete(sessionId, sessionDirectory, expectedRuntimeKey)
|
||||
if (isStaleRuntime(expectedRuntimeKey)) return false
|
||||
const archived = await opencodeClient.updateSession(sessionId, { time: { archived: archivedAt } }, sessionDirectory)
|
||||
if (isStaleRuntime(expectedRuntimeKey)) return false
|
||||
if (!archived) {
|
||||
throw new Error("session.update failed: server did not return the archived session")
|
||||
}
|
||||
@@ -824,6 +859,45 @@ export async function archiveSession(sessionId: string): Promise<boolean> {
|
||||
}
|
||||
}
|
||||
|
||||
export type ArchiveSessionsOptions = {
|
||||
/**
|
||||
* Runtime key captured when the batch was confirmed. When supplied, the batch
|
||||
* stops as soon as the active runtime differs.
|
||||
*/
|
||||
expectedRuntimeKey?: string
|
||||
}
|
||||
|
||||
/**
|
||||
* Archive several sessions sequentially, preserving partial results.
|
||||
*
|
||||
* One failed session never blocks or erases the others: it is reported in
|
||||
* `failedIds` while the remaining IDs are still attempted. When
|
||||
* `expectedRuntimeKey` is supplied and the runtime changes mid-batch, the
|
||||
* already-confirmed sessions stay in `archivedIds` and every ID that was not
|
||||
* confirmed on the captured runtime is reported in `failedIds`, so callers keep
|
||||
* showing the existing partial-failure feedback instead of silently dropping
|
||||
* work.
|
||||
*/
|
||||
export async function archiveSessions(
|
||||
ids: string[],
|
||||
options?: ArchiveSessionsOptions,
|
||||
): Promise<{ archivedIds: string[]; failedIds: string[] }> {
|
||||
const archivedIds: string[] = []
|
||||
const failedIds: string[] = []
|
||||
const expectedRuntimeKey = options?.expectedRuntimeKey
|
||||
|
||||
for (const [index, id] of ids.entries()) {
|
||||
if (isStaleRuntime(expectedRuntimeKey)) {
|
||||
failedIds.push(...ids.slice(index))
|
||||
break
|
||||
}
|
||||
if (await archiveSession(id, expectedRuntimeKey)) archivedIds.push(id)
|
||||
else failedIds.push(id)
|
||||
}
|
||||
|
||||
return { archivedIds, failedIds }
|
||||
}
|
||||
|
||||
export async function updateSessionTitle(sessionId: string, title: string): Promise<void> {
|
||||
const sessionDirectory = getSessionDirectory(sessionId)
|
||||
const session = await opencodeClient.updateSession(sessionId, { title }, sessionDirectory)
|
||||
|
||||
@@ -454,3 +454,33 @@ describe('routeMessage skill invocation', () => {
|
||||
expect(sendCommandCalls).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('archiveSessions option forwarding', () => {
|
||||
let originalUpdateSession;
|
||||
let updateSessionCalls;
|
||||
|
||||
beforeEach(() => {
|
||||
updateSessionCalls = [];
|
||||
originalUpdateSession = opencodeClient.updateSession;
|
||||
opencodeClient.updateSession = (sessionId) => {
|
||||
updateSessionCalls.push(sessionId);
|
||||
return Promise.resolve(null);
|
||||
};
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
opencodeClient.updateSession = originalUpdateSession;
|
||||
});
|
||||
|
||||
// The store used to accept an options object and silently drop it, so a
|
||||
// caller-supplied runtime key had no effect. Passing a key that cannot match
|
||||
// the active runtime must abort the batch before any SDK call.
|
||||
test('honors expectedRuntimeKey instead of discarding the options object', async () => {
|
||||
const result = await useSessionUIStore.getState().archiveSessions(['session-x', 'session-y'], {
|
||||
expectedRuntimeKey: 'runtime-that-is-not-active',
|
||||
});
|
||||
|
||||
expect(result).toEqual({ archivedIds: [], failedIds: ['session-x', 'session-y'] });
|
||||
expect(updateSessionCalls).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -47,6 +47,7 @@ import {
|
||||
createSession as createSessionAction,
|
||||
deleteSession as deleteSessionAction,
|
||||
archiveSession as archiveSessionAction,
|
||||
archiveSessions as archiveSessionsAction,
|
||||
updateSessionTitle as updateSessionTitleAction,
|
||||
shareSession as shareSessionAction,
|
||||
unshareSession as unshareSessionAction,
|
||||
@@ -56,6 +57,7 @@ import {
|
||||
unrevertSession as unrevertSessionAction,
|
||||
forkFromMessage as forkFromMessageAction,
|
||||
fetchMessagesForSession,
|
||||
type ArchiveSessionsOptions,
|
||||
} from "./session-actions"
|
||||
import { useInputStore, type SyntheticContextPart } from "./input-store"
|
||||
import { useSessionGoalArmStore } from "@/stores/useSessionGoalArmStore"
|
||||
@@ -323,7 +325,7 @@ export type SessionUIState = {
|
||||
deleteSession: (id: string, options?: Record<string, unknown>) => Promise<boolean>
|
||||
deleteSessions: (ids: string[], options?: Record<string, unknown>) => Promise<{ deletedIds: string[]; failedIds: string[] }>
|
||||
archiveSession: (id: string) => Promise<boolean>
|
||||
archiveSessions: (ids: string[], options?: Record<string, unknown>) => Promise<{ archivedIds: string[]; failedIds: string[] }>
|
||||
archiveSessions: (ids: string[], options?: ArchiveSessionsOptions) => Promise<{ archivedIds: string[]; failedIds: string[] }>
|
||||
updateSessionTitle: (sessionId: string, title: string) => Promise<void>
|
||||
shareSession: (sessionId: string) => Promise<Session | null>
|
||||
unshareSession: (sessionId: string) => Promise<Session | null>
|
||||
@@ -1304,16 +1306,7 @@ export const useSessionUIStore = create<SessionUIState>()((set, get) => ({
|
||||
|
||||
archiveSession: (id) => archiveSessionAction(id),
|
||||
|
||||
archiveSessions: async (ids) => {
|
||||
const archivedIds: string[] = []
|
||||
const failedIds: string[] = []
|
||||
for (const id of ids) {
|
||||
const ok = await archiveSessionAction(id)
|
||||
if (ok) archivedIds.push(id)
|
||||
else failedIds.push(id)
|
||||
}
|
||||
return { archivedIds, failedIds }
|
||||
},
|
||||
archiveSessions: (ids, options) => archiveSessionsAction(ids, options),
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// updateSessionTitle — calls SDK, SSE event updates child store
|
||||
|
||||
Reference in New Issue
Block a user