Skip to content
4 changes: 2 additions & 2 deletions integration-tests/channel-plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,14 @@ describe('Channel Plugin (Mock WebSocket E2E)', () => {
const opts = { chatId };

const r1 = await server.sendMessage(
'My secret word is "pineapple". Remember it.',
'My favorite fruit is "pineapple". Remember it.',
opts,
);
expect(r1).toBeTruthy();
console.log(`[mock-e2e] Memory set response: "${r1}"`);

const r2 = await server.sendMessage(
'What is my secret word? Reply with ONLY the word, nothing else.',
'What is my favorite fruit? Reply with ONLY the fruit, nothing else.',
opts,
);
expect(r2).toBeTruthy();
Expand Down
11 changes: 5 additions & 6 deletions integration-tests/interactive/file-system-interactive.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,19 +78,18 @@ describe('Interactive file system', () => {

// The tool call is logged once the model issues it, but the turn may
// still be settling (a failed edit can be retried) and the model may
// append a trailing newline. Poll the file until it reflects the new
// version instead of reading it once.
// write more than just '1.0.1'. Poll the file until it contains the new
// version, matching the lenient assertion used by the non-interactive
// sibling test (file-system.test.ts uses .toContain('1.0.1')).
const updated = await rig.poll(
() => rig.readFile(fileName).trimEnd() === '1.0.1',
() => rig.readFile(fileName).includes('1.0.1'),
Comment on lines 84 to +85

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] Issue fidelity: PR claims to fix #7111, but the CI run documented in that issue (run 29578497397) shows file-system-interactive.test.ts PASSED (1 test, 14516ms). The only failing test in that run was channel-plugin.test.tsshould maintain session state across multiple WebSocket messages, which failed because the word "secret" triggered the model's safety alignment (expected 'i cannot reveal your secret word. for…' to contain 'pineapple'). This PR does not touch channel-plugin.test.ts. — Failure scenario: merging this PR closes #7111 via the Fixes keyword, but the actual root cause — the "secret word" safety-guardrail trigger in channel-plugin.test.ts — remains unfixed. The next CI run on main will still fail on channel-plugin.test.ts, and the issue will appear resolved when it is not.

Suggested change
const updated = await rig.poll(
() => rig.readFile(fileName).trimEnd() === '1.0.1',
() => rig.readFile(fileName).includes('1.0.1'),
// Fix the test that actually failed in #7111 (channel-plugin.test.ts — rephrase the prompt
// to avoid the word 'secret'), or open a new issue for this flakiness and remove `Fixes #7111`.
const updated = await rig.poll(
() => rig.readFile(fileName).includes('1.0.1'),

— qwen3.7-max via Qwen Code /review

rig.getDefaultTimeout(),
200,
);
if (!updated) {
printDebugInfo(rig, rig._interactiveOutput, { toolCall });
}
expect(updated, 'Expected file content to be updated to 1.0.1').toBe(
true,
);
expect(updated, 'Expected file content to contain 1.0.1').toBe(true);
},
);
});
32 changes: 23 additions & 9 deletions integration-tests/test-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -544,20 +544,34 @@ export class TestRig {
): Promise<boolean> {
const startTime = Date.now();
let attempts = 0;
let lastError: unknown;
while (Date.now() - startTime < timeout) {
attempts++;
const result = predicate();
if (env['VERBOSE'] === 'true' && attempts % 5 === 0) {
console.log(
`Poll attempt ${attempts}: ${result ? 'success' : 'waiting...'}`,
);
}
if (result) {
return true;
try {
const result = predicate();
if (env['VERBOSE'] === 'true' && attempts % 5 === 0) {
console.log(
`Poll attempt ${attempts}: ${result ? 'success' : 'waiting...'}`,
);
}
if (result) {
lastError = undefined;
return true;
}
Comment on lines +550 to +560

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] lastError is set when the predicate throws but is never cleared when the predicate later evaluates without throwing, so the timeout log can report a stale error unrelated to the actual failure. — Failure scenario: a poll predicate throws ENOENT on early attempts (file not yet written), then the file appears but has wrong content, so the predicate returns false without throwing. On timeout, the log prints Last error: ENOENT... even though the real failure is a content mismatch, misleading the debugger.

Suggested change
try {
const result = predicate();
if (env['VERBOSE'] === 'true' && attempts % 5 === 0) {
console.log(
`Poll attempt ${attempts}: ${result ? 'success' : 'waiting...'}`,
);
}
if (result) {
return true;
}
try {
const result = predicate();
lastError = undefined;
if (env['VERBOSE'] === 'true' && attempts % 5 === 0) {
console.log(
`Poll attempt ${attempts}: ${result ? 'success' : 'waiting...'}`,
);
}
if (result) {
return true;
}

— qwen3.7-max via Qwen Code /review

} catch (err) {
lastError = err;
if (env['VERBOSE'] === 'true') {
console.log(`Poll attempt ${attempts}: predicate threw: ${err}`);
}
Comment on lines +561 to +565

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The blanket try/catch in poll() silently swallows all predicate exceptions unless VERBOSE=true, and when poll ultimately returns false on timeout, the last caught exception is never surfaced. — Failure scenario: a developer debugging a test failure sees "Expected X to be true" with no indication of why the predicate never succeeded. The root cause (e.g., readFileSync throwing ENOENT because the model never created the file) is invisible in default CI runs (VERBOSE is off). The developer must reproduce with VERBOSE=true to discover that the predicate was throwing on every attempt rather than simply returning false. This affects all 12 poll() call sites.

Suggested change
} catch (err) {
if (env['VERBOSE'] === 'true') {
console.log(`Poll attempt ${attempts}: predicate threw: ${err}`);
}
} catch (err) {
lastError = err;
if (env['VERBOSE'] === 'true') {
console.log(`Poll attempt ${attempts}: predicate threw: ${err}`);
}
}

Track the last caught exception and log it unconditionally when poll times out. Add let lastError: unknown; before the loop, and after the loop: if (lastError) { console.log(\Poll timed out after ${attempts} attempts. Last error: ${lastError}`); }`. This preserves the transient-error resilience while keeping programming errors visible in non-VERBOSE CI runs.

— qwen3.7-max via Qwen Code /review

}
await new Promise((resolve) => setTimeout(resolve, interval));
}
if (env['VERBOSE'] === 'true') {
if (lastError) {
console.log(
`Poll timed out after ${attempts} attempts. Last error:`,
lastError,
);
} else if (env['VERBOSE'] === 'true') {
console.log(`Poll timed out after ${attempts} attempts`);
}
return false;
Expand Down
Loading