Fix flaky AI assistant skill menu test (await default skill render)#4940
Fix flaky AI assistant skill menu test (await default skill render)#4940habdelra wants to merge 2 commits into
Conversation
…l menu The "Add Same Skills" acceptance test opens the skill menu twice. The first open (after copying skills) uses a waitUntil for the attached-card count to settle. The second open (after a plain new-session click that attaches only the default env skill) skipped that wait and asserted directly, racing the async default-skill attachment. Mirror the symmetric waitUntil and surface the observed attached-card list if the wait times out, so the next failure is debuggable.
Preview deploymentsHost Test Results 1 files ±0 1 suites ±0 1h 51m 53s ⏱️ + 4m 30s Results for commit 41c8493. ± Comparison against earlier commit a7a8430. Realm Server Test Results 1 files ±0 1 suites ±0 10m 22s ⏱️ + 1m 1s Results for commit 41c8493. ± Comparison against earlier commit a7a8430. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a flaky acceptance test around opening the AI assistant skill menu after creating a new session, by waiting for the default skill (derived from envSkillId) to finish rendering before asserting on attached cards.
Changes:
- Add a second
waitUntilafter the “new session” click flow to wait for exactly one attached card to appear in the skill menu. - On timeout, collect and surface the observed attached-card IDs to make failures easier to debug.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| false, | ||
| `Default skill never rendered on the new session's skill menu. Attached cards seen: ${JSON.stringify(attached)}; expected exactly [${envSkillId}].`, | ||
| ); | ||
| throw e; |
There was a problem hiding this comment.
[Claude Code 🤖] Good catch — switched the catch branch to assert.ok(false, …); return; (no rethrow), so the test ends with one diagnostic assertion failure instead of an assertion + an uncaught timeout. Fixed in 41c8493.
Drop the rethrow after assert.ok(false, …) and return instead so QUnit records a single, descriptive failure rather than an assertion plus an uncaught waitUntil timeout.
Summary
ai-assistant-test.gtsopens the skill menu twice in the "Add Same Skills" test. The first open already uses awaitUntilfor the attached-card count; the second open (after a plain new-session click) was asserting directly, racing the async default-skill (envSkillId) attachment.waitUntiland surface the observed attached-card list if the wait times out, so the next failure is debuggable.Observed failure on CS-11236 CI shard 1:
Element [data-test-skill-menu] [data-test-attached-card] does not exist(expected:exists once).Test plan
packages/hostacceptance test"Add Same Skills" copies skill configuration to new sessionlocally.