Move the AI SDK to provider-v4 and delete @google/genai (Fixes #2761) - #3380
Merged
Conversation
* Harden dependencies and ZIP extraction (Fixes #3324) Replace extract-zip with staged, resource-bounded yauzl extraction that rejects symlinks, traversal, path aliases, and publication collisions. Raise vulnerable direct and transitive dependency floors in both lockfiles and preserve the reviewed CodeQL false-positive dispositions. * Make ZIP collision test portable Assert the exact preexisting directory entry instead of reopening it with alternate casing, which is not valid on case-sensitive Linux filesystems. * Verify streamed ZIP entry sizes * Record security review completion
…etention (#3335 #3339 #3340 #3341) (#3346) * Plan: bound runaway model output across four retention defects (#3335 #3339 #3340 #3341) * Bound aggregate subagent output with a MAX_OUTPUT terminate mode (#3335) A per-response cap does not constrain a loop. Telemetry from the incident shows a subagent reaching turn 253 of the 1000-turn default while emitting the full 16,384-token ceiling on consecutive turns, entirely inside every existing bound. Adds SubagentTerminateMode.MAX_OUTPUT, RunConfig.max_output_tokens_total, and a subagent-max-output-tokens-total ephemeral. Cumulative output is tracked from provider usage metadata, falling back to a character estimate so providers that omit usage cannot make the budget unenforceable. The derived default is clamped: turns times the model output ceiling reproduces the very bound that failed here, so it is capped at 2M aggregate output tokens. * Document the subagent aggregate output budget (#3335) Explains why a turn cap alone does not bound a looping subagent, and why the derived default is clamped rather than left as max_turns times the model output ceiling. * Count generated characters without JSON.stringify, and include reasoning (#3335) The token estimate ran JSON.stringify over every chunk's blocks, adding a per-delta allocation to the exact hot path this change exists to make cheaper, and inflating the count with JSON syntax. Sums text, thinking and tool-call argument lengths directly instead. Reasoning is counted because it is generated output: the profile behind this issue ran high reasoning effort, where reasoning dwarfs visible text, so counting only visible text would leave the budget unenforceable for that exact shape. * Draft the PR body for the runaway-output work (#3335) * Record review findings and open checks for the runaway-output branch (#3335) * Measure the debugResponses cap regression: 5661x slower than a ring buffer (#3339) The landed cap front-splices and rebuilds the streamId index on every chunk once at the cap. At the cap turn.ts actually uses (1024), 200k chunks cost 13.2s and 199M reindex operations versus 2ms for a ring buffer. That converts the memory blowup into a CPU stall, which is a worse failure mode: the runaway hangs rather than crashes, and it costs this on the normal path too. * Amortise the debugResponses trim instead of splicing every chunk (#3339) Trimming on every chunk past the cap costs an O(cap) front-splice plus an O(cap) index rebuild per chunk. At the cap in use that measured 12.2s and 407M operations for 200k chunks, converting the memory blowup this bound exists to prevent into a CPU stall that a runaway would hit as a hang rather than a crash. Letting retention reach twice the cap and dropping a full cap in one batch amortises both to O(1) per chunk: the same stream costs 23ms and 397k operations, 544x faster for 1026x fewer operations, still bounded and still retaining the newest chunks. Sizes the runaway test above the high-water mark so it actually exercises the bound, asserts the absolute bound rather than only relative to stream length, and exports the cap so the test cannot drift from the real value. * Record CLI review findings: control assertions good, bound assertion weak (#3340) * Record provider review: caps sized well, one quadratic byte check (#3341) Three of four byte-accounting sites measure the delta and accumulate, which is correct. The vercel SSE line check re-measures the whole accumulated buffer on every read, making the guard quadratic in exactly the pathological case it exists to catch. Records the O(1) length pre-check remedy. * Make the retention guards scale-invariant and prove it (#3340 #3341) The vercel SSE guard re-measured the whole accumulated buffer on every read, making it O(n^2) in the no-newline case it exists to catch. Adds an O(1) length pre-check: UTF-8 length is bounded by three times String.length, so the cheap comparison settles the common case without scanning. The CLI bound test asserted retention landed under the limit for one fixture size, which would still pass if retention scaled with the stream. Adds a test that doubles the input and asserts retention does not materially move, which is the property that actually matters, and drops an assertion on the fixture's own length. Exports the fence constants so tests bind to real values, and documents why the 512 KiB threshold sits where it does relative to the largest response any catalog model can produce. * Bound provider stream parsers and the pending-response buffer (#3339 #3340 #3341) Provider parsers accumulate untrusted network data, so these get hard byte caps that raise a typed ProviderStreamProtocolError naming the limit: tool-call arguments (OpenAI Responses, OpenAI Chat collector, Anthropic), SSE incomplete-line buffers, reasoning capture, and the qwen text buffer. Caps sit at 8-16 MiB against a ~512 KiB largest legitimate response, so ordinary traffic cannot reach them. The error is non-retryable and non-failover: a malformed stream would be malformed on the next backend too. Replaces OpenAIStreamProcessor.allChunks, which retained every raw SDK chunk so three log lines could read .length, with a counter. Counts the Kimi section tokens incrementally instead of re-matching the whole buffer on every delta, which was quadratic while a section stayed open. Wires an AbortSignal through createReasoningCaptureFetch into the detached vercel reasoning parser, which previously stopped only on end-of-stream or a read error and kept accumulating after cancellation. Forces a size-based split when an unclosed code fence would otherwise pin the pending-response split point, synthesizing the opening fence and language on the retained tail so it renders as a code-block continuation. No Ink or renderer change was needed: MarkdownDisplay already flushes an open code block at end of input, so the committed half was already correct. * Fix two regressions the budget work introduced (#3335) Both were caught by the full suite and confirmed against main, where the same tests pass. The interactive loop ran the whole termination check right after counting a turn's output, which re-evaluated max_turns mid-turn. A subagent on its last allowed turn then stopped before its pending tool calls were handled, so tool results were silently dropped. Only the output budget needs a mid-turn check: it exists to stop a runaway before another request goes out. Splits checkOutputBudget out and calls only that after the turn, leaving the turn and time limits at the top of the loop where they were. Run-config resolution had been moved after runtime assembly so it could read maxOutputTokens off the isolated SettingsService. A launch that failed during assembly then dereferenced a runtime that did not exist, masking the real error. The profile already carries the value, so this reads it from there and restores the original ordering rather than adding a guard for a runtime that should never have been required this early. Adds tests pinning that the mid-turn check ignores an exhausted turn budget, that the top-of-loop check still enforces it, and that the budget itself still trips. * Extract the retention and text-buffer concerns to satisfy lint (#3339 #3341) Both files had grown past the 800-line ceiling and both had a function past the 80-line ceiling, because the bounds were bolted onto classes that were already at their limits. Moves the diagnostic-retention bound, the thinking-block collapse, and the index that makes the collapse cheap out of Turn into TurnDebugResponses, which owns that state rather than leaving Turn to carry the bookkeeping. Turn re-exports MAX_DEBUG_RESPONSE_CHUNKS so existing importers are unaffected. Moves the Kimi section counting and the bounded text append out of OpenAIStreamProcessor into openaiTextBuffer. Drops a dead undefined-check on a retained chunk: the thinking index is rebuilt whenever chunks are dropped, so a recorded location always addresses a live chunk, and the check was both unreachable and the reason the surrounding loop nested four deep. * Record the ripgrep flake as load-induced, with the evidence * Bring the PR body up to date with the decisions made since drafting * Measure the budget against the incident's own telemetry (#3335) 8,138 responses from the affected session: p50 122 output tokens, mean 339, ceiling 16,384. The budget is ~16,000 turns of typical output, so max_turns binds thousands of turns earlier and the budget cannot degrade normal runs. Against the incident it trips at turn 122 versus the 253 actually reached, since every sampled response for the runaway subagent sat at the ceiling. But a runaway made of typical 122-token turns would use 1.5% of the budget over the same 253 turns and would never trip it. Records that limitation rather than implying the budget is a general runaway guard: the retention work is what bounds memory in that case. * Correct the budget docs with measured figures and state what it misses Replaces the estimated 500-token-per-turn figure with the measured distribution (p50 122, mean 339 across 8,138 responses), which puts the budget at ~16,000 turns of ordinary work rather than ~4,000. Adds the limitation: the budget measures volume, so a loop of small responses never approaches it, and the retention limits are what bound memory there. Points at #3344. Documents that reasoning counts, and that the budget is checked mid-turn while the turn and time limits are checked at the start of a turn, so a subagent on its last allowed turn still runs the tool calls it requested. * Record the test-audit gate result: no new findings on touched files * Record the full verification result: suite green apart from the known flake * Probe fence handling across backtick counts and record the blind spot (#3340) Measured retention for an unclosed fence followed by 800 KiB of body, on this branch and on main. 3, 4 and 5 backticks all drop from retaining the whole response to a bounded 64 KiB tail, and the longer fences recover their marker and language correctly. Six or more backticks are not seen as opening a block: scanFence matches exactly three, so the next three read as a close and parity flips straight back. main measures identically, so this is pre-existing rather than introduced. Retention is 0 so there is no leak, but such a block renders as prose. Fixing it means tracking fence run-length instead of parity, which is unrelated to the memory bound this PR is about. * Fix the review findings on the new stream bounds (#3339 #3340 #3341) The detached vercel reasoning parser now throws where it previously swallowed everything, and its promise was stored with no rejection handler. Nothing guarantees the consumer reaches its await: the SDK stream can throw first, the signal can abort, or the generator can go un-iterated. That made an unobserved rejection, which is a process-level crash. The outcome is captured on the buffer and rethrown by the code that awaits it, where a caller exists to handle it. The SSE byte guard only measured the trailing incomplete remainder, so appending a newline bypassed it entirely: the line then arrived complete and went straight to JSON.parse unmeasured. Every line is now checked. Adds a test whose only difference from the existing one is the terminating newline. TurnDebugResponses rebuilt a replaced chunk by spreading the *incoming* chunk, which overwrote the retained chunk's finishReason, usage and metadata with values belonging to a later position in the stream. It now spreads the chunk being replaced. The forced-split guard only avoided landing on a low surrogate. Landing on a high surrogate tears the pair the other way, leaving an unpaired half at the end of the committed text. Both directions are handled. * Extract the SSE line-limit loop to keep nesting under the ceiling * Record the real fence run instead of reconstructing it (#3340) Two review findings had the same root: the scanner matched exactly three backticks and then tried to recover the true fence from the header with a regex. The regex restricted the info string to word characters, but CommonMark allows any run of non-backticks, so c++, objective-c and c# failed to match. On failure the continuation fell back to three backticks with no language, and a literal triple backtick inside the retained tail would then close it early and invert fence parity for the rest of the stream. Header capture also ran before scanFence could defer. When a delta ended on a backtick, scanPos stayed put and the same character was captured again on the next delta, corrupting the header and triggering the same fallback. The scanner now measures the whole backtick run at detection time and defers while the run is still arriving, so only the info string is derived from a pattern. Header capture happens after the consume decision. This also fixes a blind spot that predates this branch: a six-backtick fence read as open-then-close, so the block was never seen as open. Measured, it now bounds like the others, retaining 65,536 rather than the whole response. * Record the real fence run instead of reconstructing it (#3340) Two review findings had the same root: the scanner matched exactly three backticks and then tried to recover the true fence from the header with a regex. The regex restricted the info string to word characters, but CommonMark allows any run of non-backticks, so c++, objective-c and c# failed to match. On failure the continuation fell back to three backticks with no language, and a literal triple backtick inside the retained tail would then close it early and invert fence parity for the rest of the stream. Header capture also ran before scanFence could defer. When a delta ended on a backtick, scanPos stayed put and the same character was captured again on the next delta, corrupting the header and triggering the same fallback. The scanner now measures the whole backtick run at detection time and defers while the run is still arriving, so only the info string is derived from a pattern. Header capture happens after the consume decision. This also fixes a blind spot that predates this branch: a six-backtick fence read as open-then-close, so the block was never seen as open. Measured, it now bounds like the others, retaining 65,536 rather than the whole response. * Close two more bypasses found in review (#3335 #3341) The Responses tool-call cap was enforced on the initial arguments and on each delta, but the terminal event could replace the accumulated value wholesale without a check. A provider that sends the whole payload only in function_call_arguments.done therefore skipped the per-call limit entirely, left bounded only by the much larger SSE line limit. The terminal payload is now measured too. The orchestrator gated the budget on `> 0`, which discarded a deliberate 0 and produced no budget at all: the opposite of what 0 asks for. Only the unlimited sentinel should omit the budget. That sentinel was also written as a bare -1 in two files that have to agree, so it is now a single exported constant with the reasoning attached, and both sites use it. Adds tests that 0 stops immediately, that the sentinel does not enforce even at a billion tokens, and keeps the existing exceeded-budget case. * Move the terminal tool-call guard into the limits module Keeps parseResponsesStream under the file-length ceiling and puts the guard with the other byte limits, where the next parser that needs it will find it. * Close the review findings on bypasses and boundaries (#3335 #3339 #3341) The SSE byte cap was fixed in the vercel parser but not in the OpenAI Responses parser, which had the identical defect: it split on newlines and measured only the unfinished remainder, so a complete oversized line ending in a newline was parsed unmeasured. A probe delivered 8,388,609 bytes in one line without error. Both parsers now share one guard in the limits module, so the next one cannot drift from it. The aggregate budget stopped only once the total exceeded the budget, so a run landing exactly on it was allowed one more request. At the incident's response size that overshoot is a whole extra maximum-length response. It now stops on reaching the budget. Diagnostic retention could lose the newest state of a thinking span. A continued span is replaced at its recorded position and trimming runs immediately after, so when that position sat in the half about to be dropped, the update was written straight into the discarded region while its sibling text survived. Replacement is now skipped when the recorded home is inside the pending drop, and the span is re-appended into the retained window instead. Adds a regression test crossing the trim boundary with a continued span, which the existing tests could not catch because they exercise collapse and trimming separately. * Cover the complete-line SSE bypass that the tests were blind to The existing test only exercised an oversized unfinished line, which was the one branch the guard already checked. Adds the completed-line case, whose only difference is a terminating newline. * Do not synthesize a code fence the renderer never opened (#3340) The scanner toggles fence state on any backtick run, including one inline in prose, while the renderer only honours a fence that begins a line. Before the forced split that mismatch only affected where text was committed. With the split it became visible: the retained tail was reopened with a synthesized fence, so ordinary prose containing inline backticks rendered as a code block. Line-anchoring the scanner itself was the wrong fix. Its split points are required to match the batch helper exactly, and there is a test enforcing that equivalence which the change broke. The scanner keeps its behaviour; it now also records whether the opening run was one the renderer would recognise, and the forced split synthesizes a continuation fence only when it was. An inline run still bounds retention, it just does not reopen a block that never existed. Measured: an inline run in prose retains 65,536 rather than the whole response and no longer gains a synthetic opening. A real fence is unchanged. * Stop miscounting reasoning and zero usage reports (#3335) Both defects push the budget away from the truth in opposite directions. Counting summed every thinking block. Providers that carry a streamId re-emit the entire accumulated thought on each delta, so an N-character thought was counted roughly N squared over two times. On a high-reasoning profile, which is the family the incident came from, that inflates the total enough to stop legitimate work early. Spans with a streamId are now tracked by their latest length and contribute once. Thinking without a streamId is a true increment and still sums, so incremental providers are not undercounted. A usage report of zero was treated as authoritative. Some providers normalise a missing completion count to zero, which made the run stop being counted at all: the one way this accounting can fail open. A zero report alongside output that plainly exists now falls back to the character estimate. An accurate report is still trusted outright. Taking the larger of report and estimate was the first attempt and it was wrong: real tokenizers pack code and JSON far tighter than four characters per token, so the estimate would routinely override a correct provider count and stop runs early. Existing budget tests caught that. * Simplify the usage-report guard to satisfy strict null checks * Record the review findings deliberately left for follow-up * Bring the PR body up to date with what review changed * Record the fake-timer cross-file failures as pre-existing Identical 23 pass / 11 fail on this branch and on main when the two files run together; the term file passes 12/12 alone. * Record that all four suite failures vanish at directory scale 903 pass / 0 fail on this branch and on main for the directory containing every one of them. * Read completion tokens from the neutral event only (#3335) The interactive path read candidatesTokenCount off the UsageMetadata event. That is a Gemini-shaped key, and the agents package is required to stay provider-neutral, which the agents-neutral gate enforces in CI. The Finished event already carries the same figure as neutral UsageStats, so the non-neutral branch is removed rather than special-cased. A provider that reports usage only through the other event now falls back to the character estimate, which is the intended behaviour for an absent report. Caught by CI, not locally: this gate is npm run gate:agents-neutral and is not part of npm run lint. * Fix three defects found in PR review (#3335 #3339 #3340) The interactive path summed cumulative reasoning. The stateful counter added earlier only covered the non-interactive loop, so the interactive one still counted an N-character span about N squared over two times. That inflates the aggregate budget and stops legitimate work on a high-reasoning profile, which is the same failure the non-interactive fix was for. Thoughts carrying a subject are now tracked by latest length; thoughts without one still sum. pendingDropCount disagreed with trim. It guarded on `<` where trim uses `<=`, and added a spurious `+ 1`. At exactly CAP * 2 it claimed 1,025 pending drops while trim discarded none, so tryReplaceThinkingBlock treated live chunks as doomed, dropped valid stream ids from the index, and appended thinking spans instead of replacing them. That defeats the linear-space collapse this class exists to provide. The guard and the arithmetic now match trim exactly. The surrogate guard tore the pair before the one it protected. A high surrogate at the split point already keeps its low partner, because the partner sits at candidate + 1 inside the retained tail. Moving to candidate - 1 pushes the boundary into the previous pair whenever that character is a low surrogate, splitting it and rendering a replacement character on both sides. Handling both halves was the wrong instinct; only the low-surrogate case needs adjusting. Not changed: the claim that the detached parser rejection is swallowed. The handler awaits parsePromise and rethrows captureBuffer.parseError immediately after, which is the surfacing path that finding asks for. * Close two more budget bypasses found in review (#3335 #3339) A non-finite explicit budget disabled enforcement while looking configured. checkOutputBudget tests total >= budget, which is false for both Infinity and NaN, and Math.floor followed by Math.max(0, ...) preserves both. Either value therefore turned the aggregate budget off completely. Non-finite input now falls back to the derived default, so a bad explicit value cannot be more permissive than supplying none. Code blocks were not counted. The counter handled text, thinking and tool calls, but code is model-generated and a runaway can emit it exclusively, which would have skipped the budget entirely. Tool responses and media stay excluded on purpose: they carry tool results and inputs the model did not produce, and charging a large tool result against a budget meant to stop runaway generation would stop healthy runs. That reasoning is now recorded next to the code. Also tied the retention test's stream length to MAX_DEBUG_RESPONSE_CHUNKS rather than a hardcoded 3000, so raising the constant cannot silently stop the test exercising the trim path, and covered the empty-report-with-empty-output case. * Cancel the abandoned tee branch instead of just unlocking it (#3341) The reasoning parser reads one branch of a tee. Releasing its reader without cancelling leaves that branch live, so as the SDK drains the other branch the tee queues every subsequent chunk for the abandoned one. On the byte-limit path that retains the remainder of the response, which is the opposite of what the limit exists to do: the guard would bound the parser's own buffer and then leak the same data somewhere else. Cleanup order also changed. finalized is set first so a consumer waiting on it is released even if a later cleanup step throws. Also corrected two comments in openaiTextBuffer: appendBufferedText only appends, it does not emit, and the invariant that textBufferBytes must track textBuffer exactly is now stated where someone changing the buffer will see it. Both were wrong in ways that invite a future change to bypass the cap. * Stop discarding tool calls from the last allowed turn (#3335) The non-interactive loop re-checked the turn and time limits between receiving a response and dispatching its tool calls. A subagent on its final allowed turn would emit tool calls and have them silently thrown away. The interactive loop was corrected earlier in this branch and the two had been left disagreeing about when a run ends, which is worse than either behaviour on its own. Both now re-check only the output budget at that point. The loop head still enforces turn and time, so this costs one dispatch and keeps the result instead of discarding it. Both reviewers raised this independently. It was recorded as pre-existing, which was true and beside the point: fixing one path and not the other made the inconsistency mine. The test drives the real non-interactive loop with max_turns 1 and a self_emitvalue call on that turn, then asserts the emitted variable arrived. Verified against the previous implementation, where it fails. * Do not trade the tee leak for a deadlock (#3341 #3339) Awaiting the tee-branch cancellation was wrong. cancel() on one branch can stay pending while the sibling is still open, and the SDK may leave its stream open after an abort, so awaiting here would keep parsePromise pending and hang vercelStreamHandler, which awaits it. Releasing the lock is what has to happen synchronously; the cancellation lands whenever the tee allows. This is the same mistake as the earlier retention cap, which stopped a crash by introducing a hang: a bound that blocks is not a bound. pendingDropCount also has to project the appended chunk. The replacement runs before push appends, so judging pending drops from the current length is one short: at exactly the high-water mark it reported nothing doomed, the append tipped the total over, and trim discarded the chunk that had just been updated in place, losing the newest thinking value. No regression test accompanies the second fix. The one I wrote failed against both the old and the new arithmetic, so it was evidence of nothing, and a test that looks like proof without being proof is worse than none. It is removed and the gap is recorded in REVIEW-NOTES. * Cover the trim-boundary case properly (#3339) The earlier attempt at this test failed against both the old and the new arithmetic, so it was removed and the gap recorded rather than shipped as false evidence. The cause was mine: it drove the whole Turn, where the chunk shape did not reach the code under test the way I assumed. Driving TurnDebugResponses directly makes the boundary exact. At the high-water mark nothing is doomed yet, and the final chunk carries both a continuation and a sibling, so the sibling is appended and trim runs inside the same push. Measured both ways: previous arithmetic thoughts retained = [] projected arithmetic thoughts retained = ["NEWEST"] The previous code did not merely mislocate the thought, it lost it entirely. The test fails against that implementation, so it pins the fix. The coverage-gap note is removed because the gap is closed. * Bound retained tool-call fragments, not just their bytes (#3341) A byte budget does not bound object count. Empty and one-byte deltas cost almost nothing against 16 MiB while each still costs a retained object and lengthens the duplicate scan, which is linear per fragment. A peer emitting them indefinitely grows memory and CPU without ever tripping the byte cap, which is the same shape as the original incident: a limit that the pathological case walks straight past. Two changes. A fragment carrying no identity and no payload is not stored at all; that is not a limit, just refusing to record noise. And retained fragments per call are capped at 500,000, reported through the same error type as the byte limits so callers cannot tell them apart. Streaming a maximum-length tool call one token at a time is on the order of 128,000 fragments, so the cap sits far above any legitimate call and only catches the degenerate case. Coalescing adjacent fragments, the other half of the review finding, is not done here: it changes how calls are reconstructed and deserves its own change. The count bound removes the unbounded growth either way. Directory-scale run measured with and without this change: 2,084 failures both times, so the providers isolation issue is unrelated. Pass count moves 4348 to 4352, which is these four tests.
…sues (Fixes #3064) (#3352) * Stamp milestone, ci/cd label, and Bug type on auto-created failure issues (Fixes #3064) Five workflows open an issue when something fails, and they stamped triage metadata inconsistently, so the issues fell out of the release view and had to be filtered by hand. Only nightly and evals-nightly carried a milestone (from #3149); release and the OCR infrastructure notifier carried just the ci/cd label; smoke-test carried nothing at all. None of the five set an issue type. Every site now creates its issue with the ci/cd label, the open milestone whose title matches the version in main's package.json, and issue type Bug. Where a workflow reuses a long-lived tracking issue instead of creating one (nightly, evals-nightly, the OCR notifier), it re-applies milestone and type to that issue rather than only stamping on first creation. gh issue create has no --type flag, so the type is applied over REST after creation via `gh api -X PATCH repos/OWNER/REPO/issues/N -f type=Bug`, parsing the issue number out of the URL create prints. apply_issue_type always returns 0 and warns on failure: the nightly notifier deliberately bans both `| true` and `|| true`, and metadata must never sink a failure notification. The milestone and type helpers are inlined per workflow rather than sourced from a shared script. Three of these notify jobs have no checkout step, which is why the existing resolve_milestone reads package.json over the API instead of from disk; sharing would mean adding five pinned sparse checkouts plus a contents: read grant to the privileged workflow_run-triggered OCR notifier. The function bodies are byte-identical across all five sites. Guard every array expansion with the `${ARR[@]+"${ARR[@]}"}` alternate form. Expanding an empty array as "${ARR[@]}" under `set -u` is an unbound-variable error on bash 3.2, still the /bin/bash on macOS, so the milestone fail-soft path shipped in #3149 aborted the notifier there. It survived only because CI runs bash 5. Tests execute the real `run:` script extracted from each workflow against a stateful fake gh on PATH, asserting recorded argv and fake-API state rather than workflow source text. Sixty cases cover, per site, the happy path, milestone resolution boundaries (no match, fetch failure, missing version, exact-title-only, second-page pagination), type-application boundaries (failed PATCH, unparseable create output), and the recurring-issue path. The nightly shell scanner learns the guarded expansion so its repository- targeting assertions keep covering the guarded call sites instead of silently degrading, with tests for both the resolving and fail-closed cases. Verification: typecheck, lint, build, actionlint with CI's ignore set, and the affected test files are green. The batch of workflow-related suites shows an identical 101 failures / 2 errors before and after this change; those are a pre-existing cross-file interference in the repo suite. Plan in project-plans/issue3064-auto-issue-metadata.md. * Fix milestone resolution, evals reuse path, and test-harness fidelity (Refs #3064) Review of the first commit found the milestone resolver could never work. Every site ran 'gh api --paginate --slurp ... --jq ...', which gh rejects outright: 'the --slurp option is not supported with --jq or --template'. The resolver fails soft, so all five workflows carried on and created their issue with no milestone at all. That defect shipped in #3149 and this branch had copied it to three more workflows. Resolution now pipes the slurped pages to a real jq and passes the version with --arg, so the value no longer has to survive a round of shell quoting. Verified live against this repository. The evals notifier could also reuse an issue without stamping it. Its inner create_issue_once race check returns the number of an issue that appeared after the outer search, in which case CREATE_ARGS never applied. Both reuse paths now go through one annotate_issue helper, which validates the reference before calling the API so an unparseable one cannot burn four retries and three sleeps. The tests passed against all of this, so the harness was the larger problem. The fake gh now models the option validation real gh performs (--slurp with --jq, and --slurp without --paginate, both rejected with gh's exit status), emits the real --slurp page-of-pages shape instead of running jq itself, requires a title and a body on issue create, records the PATCH fields, and accepts an issue URL wherever gh accepts one. The harness substitutes GitHub expressions in the run body rather than only in env, throws on an expression it does not model instead of silently yielding an empty string, and invokes bash with the runner's actual flags per the step's shell. Assertions now require the whole PATCH contract rather than just the issue number. Two mutation checks confirm the suite is no longer vacuous: reintroducing the --slurp/--jq combination fails 5 tests, and changing type=Bug to type=Task fails 2. Both previously passed. A new test compiles the embedded Python so an escaping slip in the fake surfaces directly instead of as a retried gh error. 251 tests pass across the new suite and every modified pre-existing suite; typecheck, eslint, prettier, YAML parse and actionlint with CI's ignore set are clean. * Guarantee the ci/cd label and harden the fake gh (Refs #3064) Open code review round one findings. smoke-test.yml and release.yml passed --label ci/cd to gh issue create without first guaranteeing the label exists. gh hard-fails an unknown label, and both steps run under the runner's default bash -e, so a missing ci/cd label would have lost the entire failure notification and skipped the type stamp with it. Both now use the same ensure_label helper as nightly and evals-nightly, which creates the label or verifies it and degrades to an unlabelled issue rather than to no issue. All five notifiers now share one label strategy: create-or- verify in four, create-then-retry-without-labels in the OCR notifier. The fake gh returned an empty object and exit 0 for any endpoint or subcommand it did not model, so a workflow could call something unmodelled and still go green. Unmodelled api endpoints, issue subcommands, label subcommands and top-level commands now fail loudly; gh label create is modelled properly, including the already-exists failure ensure_label depends on. The harness now asserts python3 and jq are present and names the missing one, rather than surfacing their absence as a blank assertion after the notifier retried past a non-zero gh. Spawn errors are folded into stderr instead of being flattened to status 1 with no output. The || fallback resolver folds over every operand instead of destructuring the first two and dropping the tail of a || b || c. evals-nightly's annotate_issue said it was skipping the type when it was also skipping the milestone. The OCR notifier's annotate_existing_issue now wraps its issue edit in retry_gh like every other issue mutation in these scripts. resolveExpression moved its fixed context lookups into a table and split out the fallback chain, bringing complexity back under the limit rather than raising the threshold. Rejected: the suggestion to share the helpers via a script fetched at runtime for release.yml and smoke-test.yml. Those two do have checkouts, but the other three notifier jobs do not, so it would leave two divergent mechanisms for the same logic instead of one duplicated one. Mutation checks re-run after the refactor: mutating smoke-test to type=Task and to the rejected --slurp/--jq form fails 5 tests. 303 tests pass across the new suite and every modified pre-existing suite; typecheck, eslint, prettier, actionlint with CI's ignore set, and the build are clean. * Record the review outcomes and the corrected milestone resolution in the plan (Refs #3064) * Close the duplicate-issue window and harden release interpolation (Refs #3064) PR review round: CodeRabbit and the PR-side open code review. nightly.yml retried gh issue create directly. Creation is not idempotent, so a request that reached GitHub but reported failure to the client would have the retry open a second 'Nightly workflow failed' issue. It now uses the same create_issue_once guard evals-nightly already had, which re-searches for the title before each attempt. Both nightly reuse paths -- the outer search and the inner race check -- now run through one annotate_issue helper, so an issue that won the race still receives the milestone CREATE_ARGS never applied to it. release.yml interpolated GitHub expressions straight into the run body. They now travel through env, so a value carrying shell metacharacters cannot alter the script. smoke-test.yml took REF from github.event.inputs.ref alone, which is empty on push runs and titled the issue 'Smoke test failed on @ <date>' with no revision. It now uses the same || github.sha fallback as the checkout step. The fake gh accepted gh label create and returned success without recording the label, so a later label list would not see it. It now persists, and a new issuesVisibleAfterListCalls fixture models an issue opened by a concurrent run: invisible to the first N searches, visible afterwards. That drives a new test, registered only for the two sites that actually guard the window, asserting no duplicate create and that the winning issue still gets milestone and type. Removing the guard from nightly fails it. release-process-b.test.ts asserted retry_gh gh issue create. Updated to the guarded form rather than dropped: it now requires retry_gh create_issue_once, requires the inner gh issue create, and forbids the unguarded retry. 305 tests pass across the new suite and every modified pre-existing suite, 3 skipped for sites without a race guard; typecheck, eslint, prettier, YAML parse, actionlint with CI's ignore set, and the build are clean. * Fail closed when the duplicate lookup itself fails (Refs #3064) CodeRabbit follow-up on the create_issue_once guard. The lookup swallowed its own failure with '|| found=""', so a failed gh issue list was indistinguishable from a confirmed absence and fell through to create. That reopens exactly the duplicate window the guard exists to close: if a create reached GitHub but reported failure, and the recheck then errored, the retry would open a second issue. The lookup now returns failure instead, so retry_gh rechecks before creating. Exhausting the attempts aborts without creating, which matches the policy the outer search already applies ('aborting to avoid duplicates'): a missed notification is preferable to duplicate tracking issues. Applied to evals-nightly as well, which had the same swallow. The guard bodies stay byte-identical between the two workflows. Covered by a new test, registered only for the two guarded sites, driving a new issue-list failure fixture in the fake gh: no create is attempted and the step exits non-zero. It carries an explicit 30s budget because retry_gh burns three 5s sleeps exhausting its attempts. 307 tests pass across the new suite and every modified pre-existing suite, 6 skipped for sites without a race guard; typecheck, eslint, prettier, YAML parse, actionlint with CI's ignore set, and the build are clean. * Scope the fake gh label-list failure key to label listing (Refs #3064) CodeRabbit follow-up. The issue-list failure knob added in the previous commit was applied to both handle_issue and handle_label, because a blind string replace matched the identical list-branch head in each. A failOn rule for issue listing therefore also failed label listing, and no rule could target label listing on its own, so the two failure paths could not be isolated in a test. The label branch now keys off label/list with its own message.
* Run CI for pull requests based on dev/** branches (Fixes #3358) A pull request targeting a dev/** branch ran no CI. ci.yml, e2e.yml and interactive-ui.yml filtered pull_request to main and release/** only, so the only workflows that fired were the pull_request_target ones: review bots, OCR, auto-label and the mergeability gate. The PR showed a full green board that said nothing about whether the code compiled or the tests passed. Observed on #3342 after retargeting it to dev/0.12.0. Querying the API for its head SHA returned three runs, all pull_request_target; LLxprt Code CI and Testing: E2E were absent. Adds dev/** to the pull_request branch filter in the three workflows. Leaves the push filters alone. dev/** branches are integration branches rather than release lines, and the pull_request trigger is what gates merges. * Update the pinned CI event-coverage list to include dev/** scripts/tests/ci-codeql-latency.bun.test.ts pins ci.yml's pull_request branch list so a CodeQL latency change (issue #3187) cannot alter event coverage by accident. Adding dev/** is a deliberate coverage change, so the pin moves with it rather than being deleted, and the comment records why.
Upgrades ai 5.0.206 to 7.0.83 and @ai-sdk/openai 2.0.109 to 4.0.50, which moves both onto @ai-sdk/provider 4.0.8, and adds @ai-sdk/google 4.0.54 on the same protocol so the Gemini adapter can follow without mixing protocol majors in one tree. The v2 line carries GHSA-866g-f22w-33x8 against @ai-sdk/provider-utils, and npm resolves it only by moving to v4. Measured on an isolated install, the v4 generation including Google is 13 packages with no advisories, against 17 packages and four low advisories today. Four type-level changes were required: - ai 7 dropped the deprecated CoreMessage and CoreToolMessage aliases. Renamed to ModelMessage and ToolModelMessage, which this code already imported elsewhere. - ToolContent widened to Array<ToolResultPart | ToolApprovalResponse>, so a test helper that returned a tool message's content as ToolResultPart[] now selects the tool-result parts instead of assuming them. - GenerateTextResult.reasoning widened to (ReasoningOutput | ReasoningFileOutput)[]. The file variant shares no properties with the text variant, so an optional-text object type is rejected as a weak type. The parameter now accepts the array loosely and narrows each entry, which is what the runtime code already did. - wrapLanguageModel proxies the model to normalize spec versions, so a mock returning the bare string 'mock-model' now throws. The mock returns an object declaring specificationVersion v4. packages/core declared ai and @ai-sdk/openai and imported neither; both declarations are removed. Verified by searching packages/core/src for any @ai-sdk or 'ai' import, which returns nothing. Repository typecheck passes with no errors.
The Gemini provider imported seventeen symbols from @google/genai, but only
one of them was ever a runtime value: Type.OBJECT, read once when injecting a
default schema type. Everything else was type-only. A 26-package dependency
was supplying type declarations.
geminiWireTypes.ts now declares those shapes directly. They describe the
Gemini REST wire format, which is what the provider already builds and parses,
so nothing about the request or response path changes.
Three symbols needed care rather than a plain interface:
- ApiError is constructed and thrown, so it stays a class. Its `status` is
genuinely polymorphic: REST returns an HTTP number while gRPC-shaped
payloads return strings such as RESOURCE_EXHAUSTED, and both are classified
downstream.
- Outcome and Language are read in value position by tests, so each is
exported as a const object alongside its type.
- GroundingMetadata needed real chunk and support shapes, because the
converter walks groundingChunks[].web and groundingSupports[].segment.
Client construction moves to geminiClientFactory.ts behind a two-method seam,
generateContent and generateContentStream, which is all the provider ever
used. The second call site already declared that structural shape inline. The
factory returns the client without reading .models, keeping construction lazy;
reading it eagerly broke tests whose fakes populate the property later.
One test defect surfaced. Three suites mocked the schema-type constant as
{ OBJECT: 'object' }, but the real value is 'OBJECT', and an assertion checked
for the lowercase form. The assertion was verifying the mock rather than the
value sent to the API. Mocks and assertion now use the real constant.
Verified against a worktree of the parent commit: 270 pass, 18 fail before and
after, so the pre-existing failures are unchanged and no new ones appear.
Providers typecheck is clean.
@google/genai is now reached from exactly one file.
…ixes #2761) @google/genai is gone from the dependency tree. `npm ls @google/genai` reports empty, and the 26-package subtree it carried goes with it, including the google-auth-library 10.9.0 duplicate and the gaxios 7 path to node-fetch 3 that kept the deprecated node-domexception chain alive alongside the first-party usage #3370 removes. The AI SDK now owns transport: HTTP, auth headers, base URL resolution, SSE framing and retries. No hand-written fetch, no hand-parsed SSE. What remains is shape translation in geminiAiSdkConverters.ts, because this provider builds and reads the Gemini generateContent wire format directly. Two things come out better than the path they replace: - Finish reason. V4 returns { unified, raw }, and raw is the literal Gemini string. STOP now survives without reaching into a response body, which is the gap the spike recorded for the incumbent SDK. - Usage. V4 carries usage.raw, the provider's own usageMetadata, so it is passed through intact. That preserves serviceTier and promptTokensDetails, fields the AI SDK does not model and the previous mapping dropped. Tool-call input arrives from the AI SDK as a JSON string and is parsed into `args` before becoming a functionCall. Replaying the stringified form makes the API reject the turn with INVALID_ARGUMENT on function_call.args. Six suites mocked @google/genai. They already faked the same two-method seam the factory returns, so they now mock geminiClientFactory instead. That points them at our own code rather than at a particular SDK's constructor, which is what those assertions were always about: GeminiProvider.auth checks the client options the provider builds, and the stateless, thinkingLevel, userMemory and mediaBlock suites check the request that reaches the seam. Verified live against the API, not only at type level: generateContent -> parts [{"text":"OK"}], finishReason STOP, usage {thoughtsTokenCount 53, promptTokenCount 10, candidatesTokenCount 1, totalTokenCount 64, serviceTier standard, promptTokensDetails} tool call -> {"functionCall":{"name":"get_weather","args":{"city":"Paris"}}} with args an object, not a string streaming -> 2 chunks, text "1, 2, 3." packages/providers: typecheck clean, gemini suite 270 pass / 18 fail, which is exactly the pre-change baseline with zero new failures and no assertion weakened. Repository typecheck and lint both clean.
…er (Fixes #2761) The provider-agnostic naming guard forbids the GeminiClient identifier, which was renamed to AgentClient across the repository. The new transport seam reused that exact name, and createGeminiClient contained it as a substring, so the guard failed. Renamed to GeminiApiClient, GeminiApiClientOptions and createGeminiApiClient, with geminiClientFactory.ts becoming geminiApiClientFactory.ts. The name is also more accurate: this is the Gemini HTTP API client, not the agent client the guard is protecting. Providers typecheck clean; gemini suite unchanged at 270 pass / 18 fail.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
maintainer:e2e:ok
…2761) The guard required @google/genai to be declared by two sanctioned workspaces, so removing the SDK made it fail: 'missing from dependencies — sanctioned workspace must declare it at the exact version.' It encoded the old invariant, that the SDK is contained. The new invariant is stronger: it is gone, and any reappearance anywhere is a violation. GENAI_DEPENDENCY_MANIFESTS is now empty, so no workspace may declare the SDK. GENAI_IMPORT_ENCLAVES stays. Emptying it looked right at first and was wrong: that list does double duty, also governing which directories may export Gemini-named symbols, and clearing it made the guard reject toolDeclarationsToGemini in the provider's own directory. Its justification now records that the enclave no longer imports an SDK. The allowlist test asserted the two sanctioned directories. It now asserts the allowlist is empty, which is a stronger claim rather than a weaker one. The enclave assertion is unchanged. Guard passes; genai-enclave-guard-allowlist 6 pass / 0 fail.
Three CI guards still described the world where @google/genai existed. bun.lock was stale: package.json changed but only npm install had been run, so the Bun package-manager contract test found the root and workspace dependency graphs out of sync. Regenerated with bun install; the SDK is absent from it now. dev-docs/genai-import-baseline.md tracked which files import the SDK and drifted when the provider moved to local wire types. Regenerated: 2 importers. The manifest guard tests asserted the sanctioned behaviour directly, so they had to be inverted rather than adjusted: - declaring the SDK at the root, previously the allowed packaging bridge, is now a violation - declaring it in packages/providers, previously its sanctioned home, is now a violation - omitting it, previously a violation, is now the required state The shared writeRequiredManifests helper also wrote manifests that declared the SDK, which made the post-removal case fail for the wrong reason. One design correction: REQUIRED_MANIFEST_DIRS was derived from the dependency allowlist, so emptying that allowlist silently disabled the F4 fail-closed rule that root and packages/providers manifests must be readable. A deleted manifest would then have passed. The required set is now its own constant, REQUIRED_MANIFEST_WORKSPACE_DIRS, independent of what may be declared: the guard still has to READ those manifests to prove the SDK is absent from them. genai-enclave guard passes; guard suites 32 pass / 0 fail.
…rs (Fixes #2761) writeRequiredManifests stopped declaring @google/genai, which left SANCTIONED_VERSION and its import unused and failed lint. Removed both, and corrected the doc comment that still described writing 'correct @google/genai dependency declarations' for three manifests when it writes two without any.
…nt (Fixes #2761) The last nine failures were F1, F9 and F10 unit tests built on the idea that packages/providers and the root are sanctioned homes for @google/genai. With the SDK removed and the dependency allowlist empty, those expectations are backwards. F10 is now a single coherent block: declaring the SDK is rejected in any workspace whatever the version, and declaring it nowhere is accepted. The version-mismatch cases collapse into one loop, since with no sanctioned version every version is equally wrong. F1's case was really about an unrelated npm alias not being mistaken for an SDK alias. It no longer needs an SDK declaration to make that point. F9 needed the most care, because those tests are about duplicate and wrong-section detection rather than about whether the SDK may be declared at all. Keeping them meaningful: - a single declaration must still produce no DUPLICATE violation, even though the declaration itself is now a violation - the both-sections case asserts the duplicate and wrong-section rules fired, rather than an exact count of two, because the declaration violation now adds a third - the devDependencies-only case asserted the message contained 'dependencies', which passed only because the old wrong-section message named the required section in lower case. It now asserts the message names the package and the section it was found in, which holds whichever rule fires. All four guard suites: 81 pass / 0 fail.
The packaging bridge existed so npm would install @google/genai for the published root artifact even though root source never imported it. With the Gemini provider on @ai-sdk/google there is nothing to bridge, so the test now asserts the SDK is absent from the root and providers manifests rather than present at an exact version. The no-drift case went with it: two versions cannot drift when neither exists. REQUIRED_VERSION and its import are removed with it. All five genai guard suites: 85 pass / 0 fail.
…ool args (Fixes #2761) Two converter bugs, both found by replaying a signed tool call against the real API rather than by reading types. Thought signatures were dropped on function calls. Gemini 3 puts the signature on the functionCall part, and the API rejects a replayed turn whose signature is missing; P06 in the spike established that. The converter only propagated signatures for reasoning parts, so a signed call from history lost its signature on the way out. Signatures now travel in both directions, through providerOptions.google on the prompt side and providerMetadata on the result side, which are the two names the AI SDK uses for the same payload. Tool input was double-encoded. doGenerate RETURNS input as a JSON string, and I carried that asymmetry into the prompt side, where input is the parsed value. The API rejected the replay with: Invalid value at 'contents[1].parts[0].function_call.args' (type.googleapis.com/google.protobuf.Struct), "{"city":"Paris"}" which is the same INVALID_ARGUMENT failure the spike documented, arriving from the opposite direction: parse on the way in, do not stringify on the way out. Live verification of the full loop: signature captured on functionCall: true signed replay ACCEPTED, reply: "The weather in Paris is currently 18°C." On thoughtSignatures.ts: @ai-sdk/google injects the same skip_thought_signature_validator sentinel, tracking unsigned function calls while converting messages. The llxprt module is therefore duplicated work, but it runs earlier on Gemini contents and is harmless. Removing it is left as its own change with its own verification. Providers typecheck clean; gemini suite 270 pass / 18 fail, zero new failures.
web_search and web_fetch were silently broken. They declare server tools as
bare wire markers, `{ googleSearch: {} }` and `{ urlContext: {} }`, but the
converter only walked functionDeclarations. Markers carry no declarations, so
every one was dropped and the request reached Gemini with no tools at all. No
error, no warning, just ungrounded answers.
googleSearch, urlContext and codeExecution now map to the AI SDK's provider
tools, which is why the SDK exposes them:
{ googleSearch: {} } -> { type: 'provider', id: 'google.google_search', ... }
{ urlContext: {} } -> { type: 'provider', id: 'google.url_context', ... }
Verified live: the urlContext call is accepted and comes back carrying
urlContextMetadata.
Also stopped passing grounding and url-context metadata through when the
provider reports the key as null rather than absent. Spreading null would
fabricate empty metadata for callers that check presence.
Providers typecheck clean; gemini suite 270 pass / 18 fail, zero new failures.
…ixes #2761) Every unit test passed and the provider could not answer a single prompt. Both faults are the same mistake: handing @ai-sdk/google data already shaped for @google/genai. The SDK converts JSON Schema to the Gemini wire format itself, so anything pre-converted gets converted twice. Tool schemas were rejected outright: Google does not support this JSON Schema enum. Enum values must share one supported primitive type and match the schema type. Tool authors here write schemas with Gemini's own constants, Type.STRING and Type.OBJECT, which are uppercase because that IS the wire form. @google/genai took them verbatim. The AI SDK expects lowercase JSON Schema, so type 'STRING' against string enum values fails its enum check and it refuses the request. The enum is only where it fails loudly: all 18 uppercase types across the tool set were wrong, enum or not. Schema types are now normalised at this boundary. Then every request 404'd. llxprt carries the bare origin as its base URL because @google/genai appended the API version itself. The AI SDK defaults to <origin>/v1beta and joins the model path onto whatever it is given, so the bare origin produced <origin>/models/... and a miss. This is the asymmetry P12 in the spike recorded, arriving in production: the version suffix is added when absent. Verified through the real CLI against a live key, not through mocks: --prompt 'Reply with exactly: WORKS' -> WORKS list_directory -> 'Listed 51 item(s)', 54 two sequential read_file calls plus todo_write -> Version: 0.11.0, Name: @vybestack/llxprt-code-providers The multi-turn case matters most: it exercises five tool calls with signed history, which is where dropped thought signatures or mis-encoded args would show up. Providers lint clean, typecheck 0, gemini suite 270 pass / 18 fail, zero new.
…ixes #2761) The gemini suites mock the client seam, so a converter emitting well-typed nonsense looks identical to a correct one, and 'STRING' versus 'string' is invisible to the compiler. Five defects shipped through a clean typecheck and a green suite, and every one was found by hand against the live API. That is a hole in the tests, not luck. This suite runs the real factory and the real @ai-sdk/google conversion against a local node:http server, then asserts on the bytes that arrive. No network, no mocked seam. Each test was verified by reverting the fix it covers and confirming it fails: revert base-URL adaptation -> 1 fail (path is /models/..., not /v1beta/models/...) revert schema type normalisation -> 3 fail, reproducing the production error verbatim: 'Google does not support this JSON Schema enum.' revert server tool mapping -> 2 fail (markers dropped, no tools sent) restore double-encoded tool args -> 1 fail (args arrive as a JSON string) drop function-call signatures -> 1 fail (signature missing from history) node:http rather than Bun.serve because the providers tsconfig does not declare Bun globals, and adding @types/bun to satisfy a test would be the wrong trade. One correction to earlier reporting. I had been quoting a gemini baseline of 270 pass / 18 fail. Those 18 are not real: six sibling suites register a process-wide vi.mock on the factory, which leaks when the directory runs in one process. Run per file, the way CI does, the gemini directory is 296 pass / 0 fail including these 8. The mock leakage is worth fixing on its own, but it is a test-harness fault rather than a product one. Lint clean, typecheck 0, no test-audit findings.
The gemini directory reported 270 pass / 26 fail in one process and 296 pass / 0 fail per file. The difference was entirely cross-file mock leakage, and I had been quoting the leaked number as a baseline for several commits. Two independent leaks. Six suites called vi.mock on geminiApiClientFactory. Those registrations are process-wide and bun hoists them ahead of every test in the run, so the stub reached any suite loaded alongside them, including the new wire tests that need the real factory. Adding afterAll restoration does not help: hoisting means the mock is already installed before the first test executes. The fix is injection. GeminiProvider takes its client factory as a constructor argument defaulting to the real one, and the suites pass their fake in. No module mock, so nothing to leak. Separately, two suites spread the real settings module but overrode SETTINGS_REGISTRY with an empty array. gemini.issue3255 constructs a real SettingsService and reads reasoning defaults out of that registry, so when it ran alongside those suites the registry was empty and its thinkingConfig expectations failed. Nothing needed the override; both suites still pass with the real registry. The gemini directory is now 296 pass / 0 fail whether run as a batch or per file, so the two numbers finally agree and the batch number is trustworthy. Note the wider providers package still shows heavy cross-file interference when its whole src tree runs in one process. That is the same class of problem in suites outside this change, and CI runs files in isolation, so it is untouched here rather than fixed. Lint clean, typecheck 0.
Interactive sessions failed on the first message with:
function_declarations[1].parameters.any_of[0].required:
only allowed for OBJECT type
function_declarations[1].parameters.any_of[0].required[0]:
property is not defined
apply_patch declares anyOf: [{ required: ['absolute_path'] },
{ required: ['file_path'] }], the ordinary JSON Schema way to say 'at least one
of these'. Gemini will not accept a branch that marks a property required
without being an object schema that defines it.
Each such branch now gets type: 'object' and the referenced property
definitions copied down from the parent, which preserves the either/or meaning.
Branches that already declare a type are untouched.
This is not a regression from the SDK migration. The same schema is rejected by
the raw REST endpoint, so @google/genai sent an equally invalid request; the
tool is only registered in interactive sessions, which is why --prompt runs
never hit it. Confirmed by posting both forms directly: the original returns
400 INVALID_ARGUMENT, the repaired one 200.
The repair lives at the client seam rather than in cleanGeminiSchema so there is
one implementation covering every caller that reaches the SDK, and so the wire
tests can see it. Reverting it turns the new test red.
Reproduced and verified through the tmux harness against a real key: the anyOf
error is gone from the interactive path.
Gemini suites 298 pass / 0 fail, typecheck 0, lint clean.
The default was gemini-2.5-pro, which now answers 'no longer available to new users' on a fresh key. A first run against Gemini failed before reaching any of the provider code, and the suggested replacement moves with each generation, so the default needed to move too. DEFAULT_GEMINI_MODEL and GeminiProvider.getDefaultModel both point at gemini-3.7-flash. The fallback model list, used when the models endpoint cannot be reached, now offers 3.7 and 3.6 flash ahead of the 2.5 entries rather than leading with a model new keys cannot call. Verified reachable before changing anything (HTTP 200 for 3.7 and 3.6), and verified afterwards through the CLI with no --model flag: [cli-args:gemini-3.7-flash] DEFAULT OK Full suite passes unchanged. The 43 test files mentioning gemini-2.5-pro use it as an arbitrary model string rather than asserting the default, so none needed touching; the 2.5 entries stay in the fallback list for anyone whose key still has access.
The tracked importer list drifted as the converter, factory, wire types and neutral converters changed across this branch. Regenerated: 4 importers, all inside the gemini enclave. Local lint passes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
@google/genaiis gone.npm ls @google/genaireports empty. The Gemini provider now reaches the API through@ai-sdk/google, andai/@ai-sdk/openai/@ai-sdk/googleall sit on@ai-sdk/provider@4.0.8.Stacked on #3351. This branch is based on
issue2623, which removespackages/core/src/code_assist/. That had to land first: Code Assist used@google/genaidirectly and the AI SDK cannot serve it.Dive Deeper
Why the dependency could go
The Gemini provider imported seventeen symbols from
@google/genai. Exactly one was ever a runtime value:Type.OBJECT, read once when injecting a default schema type. The other sixteen were types. A 26-package dependency was supplying type declarations.geminiWireTypes.tsdeclares those shapes directly. They describe the Gemini REST wire format, which this provider already builds and parses.Transport is the SDK's job, not ours
@ai-sdk/googleowns HTTP, auth headers, base URL resolution, SSE framing and retries. There is no hand-writtenfetchand no hand-parsed SSE in this change. What remains is shape translation ingeminiAiSdkConverters.ts, because the provider speaks the Gemini wire format on both sides.Two things improve on the path they replace:
{ unified, raw }, andrawis the literal Gemini string.STOPnow survives without reaching into a response body, which the spike had recorded as a gap for the incumbent SDK.usage.raw, the provider's ownusageMetadata, so it passes through intact. That preservesserviceTierandpromptTokensDetails, fields the AI SDK does not model and the previous mapping dropped.Five bugs that a green suite did not catch
The gemini suites mock the client seam. A converter that emits well-typed nonsense is indistinguishable from a correct one from behind a mock, and
'STRING'versus'string'is invisible to the compiler. Every one of these passed typecheck and the full suite, and every one was found by calling the real API:functionCallpart and rejects a replayed turn without it. Only reasoning parts were propagating theirs.doGeneratereturnsinputas a JSON string; the prompt side takes the parsed value. Stringifying producedINVALID_ARGUMENTonfunction_call.args.web_searchandweb_fetchdeclare{ googleSearch: {} }and{ urlContext: {} }, which carry nofunctionDeclarations. The converter walked only declarations, so those requests reached Gemini with no tools at all. No error, just ungrounded answers.Type.STRINGandType.OBJECT, which is the Gemini wire spelling that@google/genaitook verbatim.@ai-sdk/googledoes its own JSON Schema conversion and expects lowercase, so every request was rejected with "Google does not support this JSON Schema enum."@google/genaiappended the version itself. The AI SDK defaults to<origin>/v1betaand joins the model path onto whatever it is given, so every request 404'd.geminiApiClientFactory.wire.test.tsnow covers all five. It runs the real factory and the real@ai-sdk/googleconversion against a localnode:httpserver and asserts on the bytes that arrive: no network, no mocked seam. Each test was verified by reverting the fix it covers and confirming it goes red.One pre-existing bug fixed
Interactive sessions failed on the first message with
any_of[0].required: only allowed for OBJECT type.apply_patchdeclaresanyOf: [{ required: ['absolute_path'] }, { required: ['file_path'] }], and Gemini will not accept a branch that marks a property required without being an object schema that defines it.Not a migration regression: the raw REST endpoint rejects the identical schema, so
@google/genaiwas sending an equally invalid request. The tool is only registered in interactive sessions, which is why--promptruns never hit it. Each branch now getstype: 'object'and the referenced definitions copied down from the parent.Default model
gemini-2.5-proanswers "no longer available to new users" on a fresh key, so a first Gemini run failed before reaching any provider code. The default is nowgemini-3.7-flash, and the offline fallback list leads with the current generation.Test isolation
The gemini directory reported 270 pass / 26 fail as a batch and 296 pass / 0 fail per file. The gap was entirely cross-file mock leakage, from two sources: six suites called
vi.mockon the client factory, which bun hoists ahead of the whole run, and two suites overrodeSETTINGS_REGISTRYwith an empty array thatgemini.issue3255reads reasoning defaults out of.The factory is now injected into
GeminiProviderrather than module-mocked, so there is nothing to leak. Both numbers agree at 298 pass / 0 fail.Supply chain
ai@5+@ai-sdk/openai@2+@ai-sdk/provider-utils@2.2.8The 4 low findings were GHSA-866g-f22w-33x8 against
@ai-sdk/provider-utils, which npm resolves only by moving to the v4 line. Removing@google/genaialso drops thegoogle-auth-library@10.9.0duplicate and thegaxios@7path tonode-fetch@3, which is what kept the deprecatednode-domexceptionchain alive alongside the first-party usage #3370 removes. Neither change alone clears that chain; both together do.packages/corealso declaredaiand@ai-sdk/openaiand imported neither. Both declarations are removed.The genai guard
check-genai-enclaverequired@google/genaito be declared by two sanctioned workspaces, so removing the SDK made it fail. The dependency allowlist is now empty, which is the stronger invariant: the SDK is gone and any reappearance anywhere is a violation.REQUIRED_MANIFEST_WORKSPACE_DIRSis a separate constant, because deriving the required-manifest set from an empty allowlist silently disabled the fail-closed rule that those manifests must be readable.No assertion was weakened to make anything pass. Where a guard test asserted the old behaviour, it was inverted to assert the new one.
Reviewer Test Plan
Live verification against the real API:
The multi-turn cases matter most: they exercise signed history, which is where dropped thought signatures or mis-encoded args show up.
Testing Matrix
Known follow-ups, not in this PR
thoughtSignatures.tsinjection is duplicated. The AI SDK injects the sameskip_thought_signature_validatorsentinel itself, confirmed in its shipped source. Harmless, but likely dead weight; removing it deserves its own change with its own verification.src/gemini. The wider providers package still shows heavy interference when its wholesrctree runs in one process. Same class of problem, different suites. CI runs files in isolation so it is not biting today.OpenAIVercelProvider.test.tsdrives the realgenerateTextagainst a stubbedfetchwhile sibling suites register a process-widevi.mock('ai'). Pre-existing isolation weakness, not a v4 regression.Linked issues / bugs
Fixes #2761
Depends on #3351