mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-18 05:04:45 +00:00
test(acp): make exit plan mode test tolerant to LLM timeout (#8881)
* test(acp): make exit plan mode test tolerant to LLM timeout
The test sends a prompt asking the LLM to call exit_plan_mode, but LLM
behavior is non-deterministic — it may take too long or never call the
tool. Previously the test failed hard on timeout. Now it catches the
timeout and proceeds to verify whatever notifications were received,
matching the existing permissive stance already documented in the test.
* fix(test): re-throw non-timeout errors in exit plan mode test
The catch-all try/catch was suppressing JSON-RPC errors alongside
intended timeout tolerance. Guard the catch so only the harness
timeout ('Request … timed out') is swallowed; all other errors are
rethrown to surface real failures.
* fix(test): remove dead assert, polling wait, and stderr noise
- Remove stderr dump from the expected timeout path (noise on normal
tolerated path; outer catch still dumps on real failures).
- Replace fixed delay(1000) with bounded polling (5 s) for
mode_update after switch_mode to prevent new flake on slow-LLM
runs.
- Move expect(promptResult).toBeDefined() back into try (the
non-timeout guard now re-throws its AssertionError, so it is
no longer dead code).
* fix(test): address review findings for exit plan mode test
- Expose agent from setupAcpTest so callers can check for crashes
- Detect dead agent in catch block before swallowing timeout
(R1-1: agent crash was indistinguishable from slow LLM)
- Replace hand-rolled polling loop with rig.poll()
(R1-2: duplicate of existing TestRig helper)
* fix(test): tighten timeout discriminator and log swallowed timeouts
- Match the exact harness timeout shape (Request N (session/prompt) timed out)
instead of a substring to avoid swallowing JSON-RPC errors whose message
happens to contain 'timed out' (e.g. MCP request timed out).
- Check for 'response' property to distinguish client-side timeouts from
JSON-RPC error responses.
- Log swallowed timeouts so maintainers can tell which path executed.
This commit is contained in:
parent
3aa5458bdc
commit
634567b138
1 changed files with 55 additions and 15 deletions
|
|
@ -310,6 +310,7 @@ function setupAcpTest(
|
|||
stderr,
|
||||
sessionUpdates,
|
||||
permissionRequests,
|
||||
agent,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -722,7 +723,7 @@ function setupAcpTest(
|
|||
// Track which permission requests we've seen
|
||||
const planModeRequests: PermissionRequest[] = [];
|
||||
|
||||
const { sendRequest, cleanup, stderr, sessionUpdates, permissionRequests } =
|
||||
const { sendRequest, cleanup, stderr, sessionUpdates, permissionRequests, agent } =
|
||||
setupAcpTest(rig, {
|
||||
permissionHandler: (request) => {
|
||||
// Track all permission requests for later verification
|
||||
|
|
@ -762,21 +763,60 @@ function setupAcpTest(
|
|||
})) as unknown;
|
||||
expect(setModeResult).toEqual({});
|
||||
|
||||
// Send a prompt that should trigger the LLM to call exit_plan_mode
|
||||
// The prompt is designed to trigger planning behavior
|
||||
const promptResult = await sendRequest('session/prompt', {
|
||||
sessionId: newSession.sessionId,
|
||||
prompt: [
|
||||
{
|
||||
type: 'text',
|
||||
text: 'Create a simple hello world function in Python. Make a brief plan and when ready, use the exit_plan_mode tool to present it for approval.',
|
||||
},
|
||||
],
|
||||
});
|
||||
expect(promptResult).toBeDefined();
|
||||
// Send a prompt that should trigger the LLM to call exit_plan_mode.
|
||||
// The prompt is designed to trigger planning behavior, but LLM
|
||||
// behavior is non-deterministic — it may take too long or never call
|
||||
// exit_plan_mode. Catch timeouts so the test can still verify any
|
||||
// notifications that were received.
|
||||
try {
|
||||
const promptResult = await sendRequest('session/prompt', {
|
||||
sessionId: newSession.sessionId,
|
||||
prompt: [
|
||||
{
|
||||
type: 'text',
|
||||
text: 'Create a simple hello world function in Python. Make a brief plan and when ready, use the exit_plan_mode tool to present it for approval.',
|
||||
},
|
||||
],
|
||||
});
|
||||
expect(promptResult).toBeDefined();
|
||||
} catch (e) {
|
||||
// Only the harness's own 60s request timeout is acceptable — LLM
|
||||
// behavior is non-deterministic. JSON-RPC errors (errors with a
|
||||
// `response` property) indicate a real problem and must be surfaced.
|
||||
if (
|
||||
!(e instanceof Error) ||
|
||||
'response' in e ||
|
||||
!/^Request \d+ \(session\/prompt\) timed out$/.test(e.message)
|
||||
) {
|
||||
throw e;
|
||||
}
|
||||
// A dead agent also manifests as a timeout. Surface the crash instead
|
||||
// of swallowing it as an acceptable slow-LLM path.
|
||||
if (agent.exitCode !== null || agent.signalCode !== null) {
|
||||
throw e;
|
||||
}
|
||||
console.error(
|
||||
'session/prompt did not complete (continuing with partial verification):',
|
||||
e,
|
||||
);
|
||||
}
|
||||
|
||||
// Give time for all notifications to be processed
|
||||
await delay(1000);
|
||||
// Poll for mode_update notification after switch_mode, bounded at 5 s.
|
||||
// A fixed delay races the slow-LLM path: switch_mode can arrive just
|
||||
// after the timeout, and mode_update may land after the wait window.
|
||||
await rig.poll(
|
||||
() => {
|
||||
const hasSwitchMode = permissionRequests.some(
|
||||
(req) => req.toolCall?.kind === 'switch_mode',
|
||||
);
|
||||
if (!hasSwitchMode) return false;
|
||||
return sessionUpdates.some(
|
||||
(update) => update.update?.sessionUpdate === 'current_mode_update',
|
||||
);
|
||||
},
|
||||
5000,
|
||||
250,
|
||||
);
|
||||
|
||||
// Verify: If exit_plan_mode was called, we should have received:
|
||||
// 1. A permission request with kind: "switch_mode"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue