test(desktop): cover the one-entry batch clarify shape end to end
Since the questions[]-only schema (#95907) every single question is a one-entry batch on both the tool args and the gateway wire, yet no test exercised that shape (called out in #98645). Lock the behavior down at both layers: - unit: a one-entry batch renders the batch card (not a blank/spinner single card) both with the wire already parked and when the request lands after the tool row, and answers with the qid-keyed lock - e2e: a SINGLE_BATCH trigger drives the real chain (composer -> gateway -> agent -> clarify tool -> clarify.request -> renderer) through mount, pick, confirm, and settle for questions.length === 1 The e2e mock's trigger routing also learns to scope its has-tool-result guard to the answering turn's own question text: the existing any-tool- result check would false-positive once a second scripted clarify shares the conversation history. Adapted to main: the mock server now lives in tests-js/scripts, and a batch confirm answers with clarify.lock.
This commit is contained in:
@@ -15,7 +15,12 @@
|
||||
import { expect, test } from './test'
|
||||
|
||||
import { type MockBackendFixture, setupMockBackend, waitForAppReady } from './fixtures'
|
||||
import { BATCH_CLARIFY_QUESTIONS, BATCH_CLARIFY_TRIGGER } from '../../../tests-js/scripts/mock-server'
|
||||
import {
|
||||
BATCH_CLARIFY_QUESTIONS,
|
||||
BATCH_CLARIFY_TRIGGER,
|
||||
SINGLE_BATCH_CLARIFY_QUESTIONS,
|
||||
SINGLE_BATCH_CLARIFY_TRIGGER
|
||||
} from '../../../tests-js/scripts/mock-server'
|
||||
|
||||
let fixture: MockBackendFixture | null = null
|
||||
|
||||
@@ -82,4 +87,40 @@ test.describe('batch clarify card', () => {
|
||||
// And still no duplicate live card lingering after settle.
|
||||
await expect(page.locator('form[data-clarify-batch]')).toHaveCount(0)
|
||||
})
|
||||
|
||||
// #98645: since the questions[]-only schema (#95907) a single question is a
|
||||
// one-entry batch on both the tool args and the gateway wire. The card must
|
||||
// mount and answer exactly like the 2+ case — a blank or spinner-only
|
||||
// section that eats the 10-minute tool timeout is the reported failure.
|
||||
test('renders and answers a one-entry batch like the 2+ case', async () => {
|
||||
const page = fixture!.page
|
||||
const composer = page.locator('[contenteditable="true"]').first()
|
||||
await composer.waitFor({ state: 'visible', timeout: 10_000 })
|
||||
|
||||
await composer.click()
|
||||
await composer.type(SINGLE_BATCH_CLARIFY_TRIGGER, { delay: 20 })
|
||||
await page.keyboard.press('Enter')
|
||||
|
||||
const batchCard = page.locator('form[data-clarify-batch]')
|
||||
await batchCard.first().waitFor({ state: 'visible', timeout: 60_000 })
|
||||
|
||||
await expect(batchCard).toHaveCount(1)
|
||||
await expect(batchCard).toHaveAttribute('data-clarify-batch', '1')
|
||||
|
||||
await expect(batchCard.getByText(SINGLE_BATCH_CLARIFY_QUESTIONS[0]!.question)).toHaveCount(1)
|
||||
|
||||
const confirmButton = batchCard.locator('button[type="submit"]')
|
||||
await expect(confirmButton).toBeDisabled()
|
||||
|
||||
await batchCard.getByRole('button', { name: /Espresso/ }).click()
|
||||
await expect(confirmButton).toBeEnabled()
|
||||
await confirmButton.click()
|
||||
|
||||
const settled = page.locator('[data-clarify-settled]')
|
||||
await settled.waitFor({ state: 'visible', timeout: 30_000 })
|
||||
await expect(settled.getByText(SINGLE_BATCH_CLARIFY_QUESTIONS[0]!.question)).toBeVisible()
|
||||
await expect(settled.getByText('Espresso', { exact: true })).toBeVisible()
|
||||
|
||||
await expect(page.locator('form[data-clarify-batch]')).toHaveCount(0)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -933,6 +933,107 @@ describe('ClarifyTool batch card', () => {
|
||||
expect(request).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// ─── Single-entry batch (one-question questions[]) ────────────────────────
|
||||
// #95907 made `questions[]` the only advertised shape, so a single question
|
||||
// now arrives as a one-entry batch on BOTH sides: tool args carry
|
||||
// `questions:[{question, choices}]` and the gateway wire carries
|
||||
// `questions:[{qid, question, choices}]` with no top-level question. The
|
||||
// batch card must mount for the one-entry case exactly as it does for 2+.
|
||||
|
||||
function singleBatchArgs(): { questions: { question: string; choices: string[] }[] } {
|
||||
return {
|
||||
questions: [
|
||||
{
|
||||
choices: ['Local Markdown under .scratch/', 'GitHub Issues', 'Linear', 'GitLab Issues'],
|
||||
question: 'Which issue tracker should this repository use?'
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
function liveSingleBatchProps(): ToolCallMessagePartProps {
|
||||
const args = singleBatchArgs()
|
||||
|
||||
return {
|
||||
addResult: vi.fn(),
|
||||
args,
|
||||
argsText: JSON.stringify(args),
|
||||
isError: false,
|
||||
respondToApproval: vi.fn(),
|
||||
result: undefined,
|
||||
resume: vi.fn(),
|
||||
status: { type: 'running' },
|
||||
toolCallId: 'clarify-single-batch',
|
||||
toolName: 'clarify',
|
||||
type: 'tool-call'
|
||||
}
|
||||
}
|
||||
|
||||
function singleBatchRequest() {
|
||||
return {
|
||||
choices: null,
|
||||
multiSelect: false,
|
||||
question: '',
|
||||
questions: [
|
||||
{
|
||||
choices: ['Local Markdown under .scratch/', 'GitHub Issues', 'Linear', 'GitLab Issues'],
|
||||
multiSelect: false,
|
||||
qid: 'q0',
|
||||
question: 'Which issue tracker should this repository use?'
|
||||
}
|
||||
],
|
||||
requestId: 'request-single-batch',
|
||||
sessionId: 'session-1'
|
||||
}
|
||||
}
|
||||
|
||||
it('renders a one-entry batch as the batch card, not a blank/spinner single card', () => {
|
||||
const request = vi.fn().mockResolvedValue({ ok: true, remaining: [] })
|
||||
|
||||
$activeSessionId.set('session-1')
|
||||
$gateway.set({ request } as never)
|
||||
setClarifyRequest(singleBatchRequest())
|
||||
renderClarify(<ClarifyTool {...liveSingleBatchProps()} />)
|
||||
|
||||
expect(screen.getByText('Which issue tracker should this repository use?')).toBeTruthy()
|
||||
expect(screen.getByRole('button', { name: /GitHub Issues/ })).toBeTruthy()
|
||||
expect(document.querySelector('form[data-clarify-batch]')?.getAttribute('data-clarify-batch')).toBe('1')
|
||||
})
|
||||
|
||||
it('mounts the batch card once the wire request lands after the tool row (#98645 timing)', async () => {
|
||||
const request = vi.fn().mockResolvedValue({ ok: true, remaining: [] })
|
||||
|
||||
$activeSessionId.set('session-1')
|
||||
$gateway.set({ request } as never)
|
||||
|
||||
// Tool row mounts FIRST with the model's args; the gateway wire (with the
|
||||
// qid the renderer needs) arrives a beat later — same ordering as
|
||||
// tool.start → clarify.request in a live session.
|
||||
const { rerender } = renderClarify(<ClarifyTool {...liveSingleBatchProps()} />)
|
||||
|
||||
await act(async () => {
|
||||
setClarifyRequest(singleBatchRequest())
|
||||
})
|
||||
rerender(clarifyTree(<ClarifyTool {...liveSingleBatchProps()} />))
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText('Which issue tracker should this repository use?')).toBeTruthy()
|
||||
})
|
||||
expect(screen.getByRole('button', { name: /GitHub Issues/ })).toBeTruthy()
|
||||
|
||||
// And the single pick answers with the qid-keyed lock.
|
||||
fireEvent.click(screen.getByRole('button', { name: /GitHub Issues/ }))
|
||||
fireEvent.click(screen.getByRole('button', { name: /Confirm and continue/ }))
|
||||
|
||||
await waitFor(() => {
|
||||
expect(request).toHaveBeenCalledWith('clarify.lock', {
|
||||
answer: 'GitHub Issues',
|
||||
question_id: 'q0',
|
||||
request_id: 'request-single-batch'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
it('renders the settled batch with all questions and answers', () => {
|
||||
renderClarify(
|
||||
<ClarifyTool
|
||||
|
||||
@@ -400,6 +400,22 @@ const BATCH_CLARIFY_TURN: ScriptedTurn = {
|
||||
toolCalls: [{ name: 'clarify', args: { questions: BATCH_CLARIFY_QUESTIONS } }],
|
||||
}
|
||||
|
||||
/**
|
||||
* A marker that makes the mock emit a ONE-ENTRY batch clarify — the exact
|
||||
* payload shape every single question takes since the `questions[]`-only
|
||||
* schema (#95907). Coverage gap called out in #98645: nothing exercised
|
||||
* `questions:[{question, choices}]` before this.
|
||||
*/
|
||||
export const SINGLE_BATCH_CLARIFY_TRIGGER = 'E2E_SINGLE_BATCH_CLARIFY_TRIGGER'
|
||||
export const SINGLE_BATCH_CLARIFY_QUESTIONS = [
|
||||
{ question: 'Pick a single-batch drink?', choices: ['Espresso', 'Latte'] },
|
||||
]
|
||||
|
||||
const SINGLE_BATCH_CLARIFY_TURN: ScriptedTurn = {
|
||||
text: '',
|
||||
toolCalls: [{ name: 'clarify', args: { questions: SINGLE_BATCH_CLARIFY_QUESTIONS } }],
|
||||
}
|
||||
|
||||
function includesBatchClarifyTrigger(value: unknown): boolean {
|
||||
if (typeof value === 'string') {
|
||||
return value.includes(BATCH_CLARIFY_TRIGGER)
|
||||
@@ -416,6 +432,22 @@ function includesBatchClarifyTrigger(value: unknown): boolean {
|
||||
return false
|
||||
}
|
||||
|
||||
function includesSingleBatchClarifyTrigger(value: unknown): boolean {
|
||||
if (typeof value === 'string') {
|
||||
return value.includes(SINGLE_BATCH_CLARIFY_TRIGGER)
|
||||
}
|
||||
|
||||
if (Array.isArray(value)) {
|
||||
return value.some(includesSingleBatchClarifyTrigger)
|
||||
}
|
||||
|
||||
if (value && typeof value === 'object') {
|
||||
return Object.values(value).some(includesSingleBatchClarifyTrigger)
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
/**
|
||||
* A marker that makes the mock run a recursive delete through the real
|
||||
* `terminal` tool. Under `approvals: mode: "manual"` the backend parks the
|
||||
@@ -763,6 +795,27 @@ export function startMockServer(options: MockServerOptions = {}): Promise<MockSe
|
||||
}
|
||||
}
|
||||
|
||||
if (includesSingleBatchClarifyTrigger(parsed.messages)) {
|
||||
// Unlike the 2-question trigger (fresh conversation), this one
|
||||
// shares history with it, so "any tool result" would false-positive
|
||||
// — match only THIS turn's answered result by its question text.
|
||||
const hasOwnToolResult = Array.isArray(parsed.messages)
|
||||
&& parsed.messages.some(
|
||||
(message: { role?: string }) =>
|
||||
message?.role === 'tool' && JSON.stringify(message).includes('single-batch'),
|
||||
)
|
||||
|
||||
if (!hasOwnToolResult) {
|
||||
if (stream) {
|
||||
streamScriptedTurn(res, model, SINGLE_BATCH_CLARIFY_TURN)
|
||||
} else {
|
||||
nonStreamingScriptedTurn(res, model, SINGLE_BATCH_CLARIFY_TURN)
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
if (includesBlockingClarifyTrigger(parsed.messages)) {
|
||||
if (stream) {
|
||||
streamScriptedTurn(res, model, BLOCKING_CLARIFY_TURN)
|
||||
|
||||
Reference in New Issue
Block a user