test(sync): preserve question bootstrap and reducer invariants (#3439)
* test(sync): preserve question bootstrap and reducer invariants Port the main-independent test assets from the superseded hybrid V2 question PRs (#3266/#3267/#3288) onto current main's V1-only architecture: - listPendingQuestions: unscoped + per-directory fan-out, merge with id-dedupe (first wins), failure throws instead of empty success, malformed-item skip, true-empty success (client.questions.test.ts) - question reducer invariants: idempotent upsert-by-id, replace-not- first-wins, removal by session/request pair, duplicate-terminal no-op, late-asked re-registration (no tombstone) - bootstrap deferred-phase question merge: signature-based replace and disappearance-deletion, in-flight-change preservation (stale guard), retry-then-merge on transient failure Tests only: zero production changes, zero V2 SDK calls, zero fallback logic, zero raw V2 event aliases. * test(questions): clarify V1 merge id-filter test title --------- Co-authored-by: bashrusakh <bashrusakh@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
co-authored by
bashrusakh
parent
5c127a1e3c
commit
c1f19b1158
@@ -0,0 +1,144 @@
|
||||
import { beforeEach, describe, expect, mock, test } from 'bun:test';
|
||||
import type { QuestionRequest } from '@/types/question';
|
||||
|
||||
type QuestionListResult = {
|
||||
data?: unknown[];
|
||||
error?: unknown;
|
||||
request?: Request;
|
||||
response?: Response;
|
||||
};
|
||||
|
||||
const makeQuestion = (id: string, overrides?: Partial<QuestionRequest>): QuestionRequest => ({
|
||||
id,
|
||||
sessionID: `ses_${id}`,
|
||||
questions: [
|
||||
{
|
||||
question: `${id}: proceed with the plan?`,
|
||||
header: 'Build',
|
||||
options: [{ label: 'Yes', description: 'Proceed' }],
|
||||
},
|
||||
],
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const makeListResult = (items: unknown[]): QuestionListResult => ({
|
||||
data: items,
|
||||
error: undefined,
|
||||
request: new Request('http://test/'),
|
||||
response: new Response(null, { status: 200 }),
|
||||
});
|
||||
|
||||
const makeListError = (status: number, message: string): QuestionListResult => ({
|
||||
data: undefined,
|
||||
error: new Error(message),
|
||||
request: new Request('http://test/'),
|
||||
response: new Response(null, { status }),
|
||||
});
|
||||
|
||||
const questionListArgs: Array<{ directory?: string } | undefined> = [];
|
||||
const questionListResults: QuestionListResult[] = [];
|
||||
|
||||
const questionListMock = mock((args?: { directory?: string }) => {
|
||||
questionListArgs.push(args);
|
||||
const result = questionListResults.shift() ?? makeListResult([]);
|
||||
return Promise.resolve(result);
|
||||
});
|
||||
|
||||
const createOpencodeClientMock = mock(() => ({
|
||||
question: {
|
||||
list: questionListMock,
|
||||
},
|
||||
}));
|
||||
|
||||
mock.module('@opencode-ai/sdk/v2', () => ({
|
||||
createOpencodeClient: createOpencodeClientMock,
|
||||
}));
|
||||
|
||||
mock.module('@/contexts/runtimeAPIRegistry', () => ({
|
||||
getRegisteredRuntimeAPIs: mock(() => null),
|
||||
}));
|
||||
|
||||
mock.module('@/lib/runtime-url', () => ({
|
||||
getRuntimeUrlResolver: mock(() => ({
|
||||
api: (path: string) => path,
|
||||
})),
|
||||
}));
|
||||
|
||||
mock.module('@/lib/runtime-switch', () => ({
|
||||
getRuntimeApiBaseUrl: mock(() => ''),
|
||||
getRuntimeKey: mock(() => 'test-runtime'),
|
||||
}));
|
||||
|
||||
mock.module('@/lib/runtime-fetch', () => ({
|
||||
runtimeFetch: mock(async () => new Response(JSON.stringify([]), {
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
})),
|
||||
}));
|
||||
|
||||
mock.module('@/lib/startupTrace', () => ({
|
||||
markStartupTrace: mock(() => undefined),
|
||||
}));
|
||||
|
||||
const { opencodeClient } = await import(`./client?cache-test-questions=${Date.now()}`);
|
||||
|
||||
beforeEach(() => {
|
||||
questionListArgs.length = 0;
|
||||
questionListResults.length = 0;
|
||||
});
|
||||
|
||||
describe('opencodeClient.listPendingQuestions', () => {
|
||||
test('merges unscoped + per-directory results with id-dedupe, first occurrence wins', async () => {
|
||||
const globalQuestion = makeQuestion('q1');
|
||||
const duplicateQuestion = makeQuestion('q1', { sessionID: 'ses_dup' });
|
||||
const scopedQuestion = makeQuestion('q2');
|
||||
const otherQuestion = makeQuestion('q3');
|
||||
|
||||
questionListResults.push(
|
||||
makeListResult([globalQuestion]),
|
||||
makeListResult([duplicateQuestion, scopedQuestion]),
|
||||
makeListResult([otherQuestion]),
|
||||
makeListResult([]),
|
||||
);
|
||||
|
||||
const result = await opencodeClient.listPendingQuestions({
|
||||
directories: ['/repo', ' /repo ', '/repo/', '/other', ' ', null, undefined, 'd:\\MyProject', 'D:/MyProject'],
|
||||
});
|
||||
|
||||
expect(result).toEqual([globalQuestion, scopedQuestion, otherQuestion]);
|
||||
expect(questionListArgs).toEqual([
|
||||
undefined,
|
||||
{ directory: '/repo' },
|
||||
{ directory: '/other' },
|
||||
{ directory: 'D:/MyProject' },
|
||||
]);
|
||||
});
|
||||
|
||||
test('rejects the whole call when question.list fails (no empty-success masquerade)', async () => {
|
||||
questionListResults.push(makeListError(500, 'boom'), makeListError(500, 'boom'));
|
||||
|
||||
await expect(
|
||||
opencodeClient.listPendingQuestions({ directories: ['/repo'] }),
|
||||
).rejects.toThrow('question.list failed');
|
||||
});
|
||||
|
||||
test('ignores entries without a usable string id during the V1 merge', async () => {
|
||||
const valid = makeQuestion('q1');
|
||||
questionListResults.push(
|
||||
makeListResult([valid, null, { sessionID: 'ses_x' }, { id: 42 }, { id: '' }, 'not-an-object']),
|
||||
makeListResult([makeQuestion('q2')]),
|
||||
);
|
||||
|
||||
const result = await opencodeClient.listPendingQuestions({ directories: ['/repo'] });
|
||||
|
||||
expect(result).toEqual([valid, makeQuestion('q2')]);
|
||||
});
|
||||
|
||||
test('returns an empty array when every list is empty (true empty success)', async () => {
|
||||
questionListResults.push(makeListResult([]), makeListResult([]));
|
||||
|
||||
const result = await opencodeClient.listPendingQuestions({ directories: ['/repo'] });
|
||||
|
||||
expect(result).toEqual([]);
|
||||
expect(questionListArgs).toEqual([undefined, { directory: '/repo' }]);
|
||||
});
|
||||
});
|
||||
@@ -355,3 +355,98 @@ describe("applyDirectoryEvent", () => {
|
||||
expect(draft.question.ses_1).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe("question reducer invariants (main contract)", () => {
|
||||
const questionRequest = (id: string, sessionID = "ses_1"): QuestionRequest => ({
|
||||
id,
|
||||
sessionID,
|
||||
questions: [],
|
||||
})
|
||||
|
||||
const askedEvent = (id: string, sessionID = "ses_1"): Event => ({
|
||||
id: `evt_${id}`,
|
||||
type: "question.asked",
|
||||
properties: questionRequest(id, sessionID),
|
||||
})
|
||||
|
||||
const repliedEvent = (requestID: string, sessionID = "ses_1"): Event => ({
|
||||
id: `evt_${requestID}`,
|
||||
type: "question.replied",
|
||||
properties: { sessionID, requestID, answers: [] },
|
||||
})
|
||||
|
||||
const rejectedEvent = (requestID: string, sessionID = "ses_1"): Event => ({
|
||||
id: `evt_${requestID}`,
|
||||
type: "question.rejected",
|
||||
properties: { sessionID, requestID },
|
||||
})
|
||||
|
||||
test("question.asked is an idempotent upsert-by-id — replaying does not duplicate", () => {
|
||||
const draft = state({ question: { ses_1: [questionRequest("ques_1")] } })
|
||||
|
||||
expect(applyDirectoryEvent(draft, askedEvent("ques_1"))).toBe(true)
|
||||
expect(applyDirectoryEvent(draft, askedEvent("ques_1"))).toBe(true)
|
||||
|
||||
expect(draft.question.ses_1).toHaveLength(1)
|
||||
expect(draft.question.ses_1[0]?.id).toBe("ques_1")
|
||||
})
|
||||
|
||||
test("question.asked replaces the stored record in place (not first-wins)", () => {
|
||||
const draft = state({ question: { ses_1: [questionRequest("ques_1")] } })
|
||||
const replacement: QuestionRequest = {
|
||||
id: "ques_1",
|
||||
sessionID: "ses_1",
|
||||
questions: [
|
||||
{ question: "updated?", header: "Build", options: [{ label: "Yes", description: "Go" }] },
|
||||
],
|
||||
}
|
||||
|
||||
expect(applyDirectoryEvent(draft, {
|
||||
id: "evt_ques_1",
|
||||
type: "question.asked",
|
||||
properties: replacement,
|
||||
})).toBe(true)
|
||||
|
||||
expect(draft.question.ses_1).toHaveLength(1)
|
||||
expect(draft.question.ses_1[0]).toEqual(replacement)
|
||||
})
|
||||
|
||||
test("question.replied and question.rejected remove exactly the matching request; unknown removal is a no-op returning false", () => {
|
||||
const draft = state({
|
||||
question: { ses_1: [questionRequest("ques_1"), questionRequest("ques_2")] },
|
||||
})
|
||||
|
||||
expect(applyDirectoryEvent(draft, repliedEvent("ques_1"))).toBe(true)
|
||||
expect(draft.question.ses_1.map((q) => q.id)).toEqual(["ques_2"])
|
||||
|
||||
expect(applyDirectoryEvent(draft, rejectedEvent("ques_2"))).toBe(true)
|
||||
expect(draft.question.ses_1).toEqual([])
|
||||
|
||||
// Removal for an unknown request is a safe no-op.
|
||||
expect(applyDirectoryEvent(draft, repliedEvent("ques_missing"))).toBe(false)
|
||||
expect(applyDirectoryEvent(draft, rejectedEvent("ques_missing"))).toBe(false)
|
||||
expect(draft.question.ses_1).toEqual([])
|
||||
})
|
||||
|
||||
test("a duplicate terminal event after removal is a safe no-op", () => {
|
||||
const draft = state({ question: { ses_1: [questionRequest("ques_1")] } })
|
||||
|
||||
expect(applyDirectoryEvent(draft, repliedEvent("ques_1"))).toBe(true)
|
||||
expect(draft.question.ses_1).toEqual([])
|
||||
|
||||
// Replayed terminal event: no error, no duplicate, no state change.
|
||||
expect(applyDirectoryEvent(draft, repliedEvent("ques_1"))).toBe(false)
|
||||
expect(draft.question.ses_1).toEqual([])
|
||||
})
|
||||
|
||||
test("a late question.asked after a terminal event re-registers the request (no tombstone)", () => {
|
||||
const draft = state({ question: { ses_1: [questionRequest("ques_1")] } })
|
||||
|
||||
expect(applyDirectoryEvent(draft, repliedEvent("ques_1"))).toBe(true)
|
||||
expect(draft.question.ses_1).toEqual([])
|
||||
|
||||
// Ordered-stream replay: a late asked re-inserts; there is no tombstone.
|
||||
expect(applyDirectoryEvent(draft, askedEvent("ques_1"))).toBe(true)
|
||||
expect(draft.question.ses_1.map((q) => q.id)).toEqual(["ques_1"])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,9 +1,13 @@
|
||||
import { describe, expect, test } from "bun:test"
|
||||
import type { OpencodeClient, Project } from "@opencode-ai/sdk/v2/client"
|
||||
import type { OpencodeClient, Project, QuestionRequest } from "@opencode-ai/sdk/v2/client"
|
||||
import { bootstrapDirectory } from "./bootstrap"
|
||||
import { INITIAL_STATE, type State } from "./types"
|
||||
|
||||
const createSdk = (options?: { commandList?: () => Promise<{ data: unknown[] }>; sessionStatus?: () => Promise<{ data: State['session_status'] }> }) => ({
|
||||
const createSdk = (options?: {
|
||||
commandList?: () => Promise<{ data: unknown[] }>
|
||||
sessionStatus?: () => Promise<{ data: State['session_status'] }>
|
||||
questionList?: () => Promise<{ data?: unknown[]; error?: unknown; response?: { status?: number } }>
|
||||
}) => ({
|
||||
project: { current: async () => ({ data: { id: "project-a" } }) },
|
||||
config: { get: async () => ({ data: {} }) },
|
||||
path: { get: async () => ({ data: { state: "", config: "", worktree: "/repo", directory: "/repo", home: "/home" } }) },
|
||||
@@ -12,7 +16,7 @@ const createSdk = (options?: { commandList?: () => Promise<{ data: unknown[] }>;
|
||||
mcp: { status: async () => ({ data: {} }) },
|
||||
lsp: { status: async () => ({ data: [] }) },
|
||||
vcs: { get: async () => ({ data: { branch: "main" } }) },
|
||||
question: { list: async () => ({ data: [] }) },
|
||||
question: { list: options?.questionList ?? (async () => ({ data: [] })) },
|
||||
permission: { list: async () => ({ data: [] }) },
|
||||
}) as unknown as OpencodeClient
|
||||
|
||||
@@ -123,4 +127,124 @@ describe("bootstrapDirectory", () => {
|
||||
expect(result).toBe('complete')
|
||||
expect(state.sessionStatusReady).toBe(undefined)
|
||||
})
|
||||
|
||||
test("deferred phase merges fetched questions by session, replacing the pre-fetch record", async () => {
|
||||
let state = createState()
|
||||
const preExisting: QuestionRequest = { id: "que_1", sessionID: "ses_1", questions: [] }
|
||||
const fetched: QuestionRequest[] = [
|
||||
{ id: "que_2", sessionID: "ses_1", questions: [] },
|
||||
{
|
||||
id: "que_1",
|
||||
sessionID: "ses_1",
|
||||
questions: [{ question: "updated?", header: "Build", options: [{ label: "Yes", description: "Go" }] }],
|
||||
},
|
||||
]
|
||||
state = { ...state, question: { ses_1: [preExisting] } }
|
||||
const sdk = createSdk({ questionList: async () => ({ data: fetched }) })
|
||||
|
||||
await bootstrapDirectory({
|
||||
directory: "/repo",
|
||||
sdk,
|
||||
getState: () => state,
|
||||
set: (patch) => { state = { ...state, ...patch } },
|
||||
global: { config: {}, projects: [project] },
|
||||
loadSessions: async () => undefined,
|
||||
})
|
||||
// Deferred phase runs on a setTimeout(0); give it a tick.
|
||||
await new Promise((resolve) => setTimeout(resolve, 20))
|
||||
|
||||
// The fetched (sorted) records replace the pre-fetch snapshot entirely.
|
||||
expect(state.question["ses_1"]?.map((q) => q.id)).toEqual(["que_1", "que_2"])
|
||||
expect(state.question["ses_1"]?.[0]?.questions).toEqual(fetched[1].questions)
|
||||
})
|
||||
|
||||
test("deferred phase deletes a session's questions when they disappear and the signature is unchanged", async () => {
|
||||
let state = createState()
|
||||
const que1: QuestionRequest = { id: "que_1", sessionID: "ses_1", questions: [] }
|
||||
const que3: QuestionRequest = { id: "que_3", sessionID: "ses_2", questions: [] }
|
||||
state = {
|
||||
...state,
|
||||
question: {
|
||||
ses_1: [que1],
|
||||
ses_2: [que3],
|
||||
},
|
||||
}
|
||||
const sdk = createSdk({ questionList: async () => ({ data: [] }) })
|
||||
|
||||
await bootstrapDirectory({
|
||||
directory: "/repo",
|
||||
sdk,
|
||||
getState: () => state,
|
||||
set: (patch) => { state = { ...state, ...patch } },
|
||||
global: { config: {}, projects: [project] },
|
||||
loadSessions: async () => undefined,
|
||||
})
|
||||
await new Promise((resolve) => setTimeout(resolve, 20))
|
||||
|
||||
// Both sessions vanished from the fetched list and nothing changed in
|
||||
// between, so the signature guard allows the delete.
|
||||
expect(state.question).toEqual({})
|
||||
})
|
||||
|
||||
test("deferred phase preserves in-flight question changes when the signature changed (stale guard)", async () => {
|
||||
let state = createState()
|
||||
const que1: QuestionRequest = { id: "que_1", sessionID: "ses_1", questions: [] }
|
||||
const que2: QuestionRequest = { id: "que_2", sessionID: "ses_1", questions: [] }
|
||||
state = { ...state, question: { ses_1: [que1] } }
|
||||
const sdk = createSdk({
|
||||
questionList: async () => {
|
||||
// Simulate an event landing while the deferred fetch is in flight:
|
||||
// the session gains a second question before the fetch resolves.
|
||||
state = { ...state, question: { ...state.question, ses_1: [que1, que2] } }
|
||||
return { data: [] }
|
||||
},
|
||||
})
|
||||
|
||||
await bootstrapDirectory({
|
||||
directory: "/repo",
|
||||
sdk,
|
||||
getState: () => state,
|
||||
set: (patch) => { state = { ...state, ...patch } },
|
||||
global: { config: {}, projects: [project] },
|
||||
loadSessions: async () => undefined,
|
||||
})
|
||||
await new Promise((resolve) => setTimeout(resolve, 20))
|
||||
|
||||
// The fetched list is empty, but the in-flight change altered the
|
||||
// signature, so the disappearance must NOT be treated as authoritative.
|
||||
expect(state.question["ses_1"]?.map((q) => q.id)).toEqual(["que_1", "que_2"])
|
||||
})
|
||||
|
||||
test("deferred phase retries a transient question.list failure and still merges", async () => {
|
||||
let state = createState()
|
||||
const que1: QuestionRequest = { id: "que_1", sessionID: "ses_1", questions: [] }
|
||||
let calls = 0
|
||||
let resolveSecondCall!: () => void
|
||||
const secondCall = new Promise<void>((resolve) => { resolveSecondCall = resolve })
|
||||
const sdk = createSdk({
|
||||
questionList: async () => {
|
||||
calls += 1
|
||||
if (calls === 1) {
|
||||
return { error: { name: "ServerError", data: { message: "boom" } }, response: new Response(null, { status: 500 }) }
|
||||
}
|
||||
resolveSecondCall()
|
||||
return { data: [que1] }
|
||||
},
|
||||
})
|
||||
|
||||
await bootstrapDirectory({
|
||||
directory: "/repo",
|
||||
sdk,
|
||||
getState: () => state,
|
||||
set: (patch) => { state = { ...state, ...patch } },
|
||||
global: { config: {}, projects: [project] },
|
||||
loadSessions: async () => undefined,
|
||||
})
|
||||
// retry() backs off 500ms before the second attempt; wait for it.
|
||||
await secondCall
|
||||
await new Promise((resolve) => setTimeout(resolve, 0))
|
||||
|
||||
expect(calls).toBeGreaterThanOrEqual(2)
|
||||
expect(state.question["ses_1"]?.map((q) => q.id)).toEqual(["que_1"])
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user