fix(client): give the Antigravity test fixture the SessionContext field it lost - #2385
fix(client): give the Antigravity test fixture the SessionContext field it lost#2385L42y wants to merge 2 commits into
Conversation
…ld it lost `SessionContext` gained a required `noteTurnStart` in first-tree-ai#2378. The Antigravity provider (first-tree-ai#2375) was written before that field existed and merged after it, so its handler fixture builds a `SessionContext` without it and main's typecheck has been failing on the object literal ever since — every other provider fixture sets it. The tests themselves pass; nothing about the provider is wrong. This is the fixture catching up to the interface. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
yuezengwu
left a comment
There was a problem hiding this comment.
Exact-head review: 1de53b709b2b60408ec1c61aea0235ec0974a7a4. No actionable issue in this one-line fixture repair. noteTurnStart: vi.fn() satisfies the required SessionContext callback without weakening the interface, suppressing type errors, skipping tests, or changing production behavior.
Independently verified in a clean detached worktree: pnpm check passed (36 warnings, 22 infos), uncached build 5/5, uncached monorepo typecheck 9/9, and the complete Antigravity handler suite 14/14. The current CI Lint & Type Check, Test Client & Web, and both CLI test shards also pass.
CI is not fully green: run 33733656062 fails Test Server at admin-sessions-suspend-terminate.test.ts:1125, in the durable-ACK fallback when the result wake is lost (503 instead of 200; 1 failed / 3490 passed). This PR changes only the client fixture; the entire packages/server tree is identical between this head and base 38752415320c97c0bd122aa4afb13c3ff60b1b06 (tree 8788f653893bc38bc8d83981a2bcf4c1719e9bda). I reran that exact failed case in the repository's isolated PostgreSQL 17 Testcontainers setup on this head: 1 passed, 48 intentionally skipped. That does not establish the CI failure's cause, prove flakiness, or make the failed run green.
Approving the reviewed fixture change only. The independent server CI failure still needs resolution before merge; this approval does not waive checks or transfer to a successor head. No source-branch edits or merge were performed.
|
The only red checks here are Could you rerun the failed jobs so this small fixture fix can land and unblock the inherited red check on other open PRs? |
|
@L42y I investigated the failed CI at The test contains a reproducible timing race. At lines 1109–1123 it sets the RPC deadline to 50 ms, waits for I checked this in an isolated worktree and disposable PostgreSQL containers:
Please fix the test's ordering so it deterministically exercises a persisted acknowledgement with a lost wake: synchronize the durable write before triggering the timeout/fallback, and assert the store succeeded. Keep coverage for a genuinely missing acknowledgement. Simply rerunning or slightly increasing the short timeout leaves this race in place. Please fix or incorporate a fix for the flaky test and obtain a passing Test Server result. This is an inherited test-code issue, also present on current main |
bestony
left a comment
There was a problem hiding this comment.
Reviewed latest head 1ef1cda7846a1e6d6a724a8c35099d5ca8963.
The new server-test stabilization deterministically waits for the command frame, asserts the durable ack write succeeds, then advances the timeout; the existing no-ack coverage remains unchanged. The Antigravity fixture change remains correct. Biome and client typecheck pass locally; GitHub CI is fully green, including Test Server, Test Client & Web, Lint & Type Check, CLI shards, and aggregate Test. No actionable findings.
yuezengwu
left a comment
There was a problem hiding this comment.
Approved at 1ef1cda7846a1a6e17d6a724a8c35099d5ca8963 after a fresh review of both changed files. The Antigravity fixture supplies the required noteTurnStart field. The server regression now waits directly for the command frame, asserts that the durable acknowledgement was stored, and only then advances the controlled timeout. It sends no NOTIFY, retains the existing missing-ack coverage, and restores real timers in finally. This addresses the inherited CI race raised in my earlier comment.
Validation in a clean detached worktree: all 49 suspend/terminate tests and 14 Antigravity handler tests pass; repository Biome check and Client/Server typechecks pass. Current-head GitHub CI is green, including Test Server. A fresh merge-tree check against current main cb8d386c69d0b9b5a3ff3f67d1526d46fa280545 is conflict-free; the executed tests above are on the PR head. No actionable findings.
What
Adds the required
noteTurnStartto the Antigravity handler test'sSessionContextfixture.Why
SessionContextgained a requirednoteTurnStartin #2378 (fb40bc24d). The Antigravity provider (#2375,387524153) was written before that field existed and merged after it, so its fixture builds aSessionContextobject literal without the field — and387524153is currentmain, so main's client typecheck has been failing on it since that merge. Every other provider fixture (e.g.providers/claude/__tests__/provider-error-result.test.ts:213) already sets it.Nothing about the provider itself is wrong: the fixture is catching up to the interface.
Verification
vitest run src/providers/antigravity/__tests__/handler.test.tsonmain— 14/14 pass before this change, which is why the failure surfaces only in typecheck, not in the test run.Found while investigating CI on #2384, where this inherited failure appears in the merge result and is unrelated to that PR's changes.
🤖 Generated with Claude Code